[v2] Handle out-of-memory case in svc_tcp.c/svc_unix.c:rendezvous_request.
Commit Message
If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32),
gcc 11 dumps this warning:
svc_tcp.c: In function 'rendezvous_request':
svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds]
274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
In out-of-memory case, if one of the mallocs in makefd_xprt function
returns NULL, a message is dumped, makefd_xprt returns NULL
and the subsequent memcpy would copy to NULL.
Instead of a segfaulting, we delay a bit (see also __svc_accept_failed
and Bug 14889 (CVE-2011-4609) - svc_run() produces high cpu usage when
accept() fails with EMFILE (CVE-2011-4609).
The same applies to svc_unix.c.
---
include/rpc/svc.h | 1 +
sunrpc/svc.c | 10 ++++++++--
sunrpc/svc_tcp.c | 8 ++++++++
sunrpc/svc_unix.c | 8 ++++++++
4 files changed, 25 insertions(+), 2 deletions(-)
Comments
On 04/12/2020 13:00, Stefan Liebler via Libc-alpha wrote:
> If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32),
> gcc 11 dumps this warning:
> svc_tcp.c: In function 'rendezvous_request':
> svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds]
> 274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> In out-of-memory case, if one of the mallocs in makefd_xprt function
> returns NULL, a message is dumped, makefd_xprt returns NULL
> and the subsequent memcpy would copy to NULL.
>
> Instead of a segfaulting, we delay a bit (see also __svc_accept_failed
> and Bug 14889 (CVE-2011-4609) - svc_run() produces high cpu usage when
> accept() fails with EMFILE (CVE-2011-4609).
>
> The same applies to svc_unix.c.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> include/rpc/svc.h | 1 +
> sunrpc/svc.c | 10 ++++++++--
> sunrpc/svc_tcp.c | 8 ++++++++
> sunrpc/svc_unix.c | 8 ++++++++
> 4 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/include/rpc/svc.h b/include/rpc/svc.h
> index 465bf4427d..d9c0e8fbca 100644
> --- a/include/rpc/svc.h
> +++ b/include/rpc/svc.h
> @@ -38,6 +38,7 @@ libc_hidden_proto (svc_getreq_common)
> libc_hidden_proto (svc_getreq_poll)
>
> extern void __svc_accept_failed (void) attribute_hidden;
> +extern void __svc_wait_on_error (void) attribute_hidden;
>
> # endif /* !_ISOMAC */
> #endif
Ok.
> diff --git a/sunrpc/svc.c b/sunrpc/svc.c
> index 917e9a311c..3ed6cee09e 100644
> --- a/sunrpc/svc.c
> +++ b/sunrpc/svc.c
> @@ -545,6 +545,13 @@ svc_getreq_common (const int fd)
> }
> libc_hidden_nolink_sunrpc (svc_getreq_common, GLIBC_2_2)
>
> +void
> +__svc_wait_on_error (void)
> +{
> + struct timespec ts = { .tv_sec = 0, .tv_nsec = 50000000 };
> + __nanosleep (&ts, NULL);
> +}
> +
> /* If there are no file descriptors available, then accept will fail.
> We want to delay here so the connection request can be dequeued;
> otherwise we can bounce between polling and accepting, never giving the
Ok.
> @@ -555,8 +562,7 @@ __svc_accept_failed (void)
> {
> if (errno == EMFILE)
> {
> - struct timespec ts = { .tv_sec = 0, .tv_nsec = 50000000 };
> - __nanosleep (&ts, NULL);
> + __svc_wait_on_error ();
> }
> }
>
Ok.
> diff --git a/sunrpc/svc_tcp.c b/sunrpc/svc_tcp.c
> index efbdd22548..12de60f605 100644
> --- a/sunrpc/svc_tcp.c
> +++ b/sunrpc/svc_tcp.c
> @@ -271,6 +271,14 @@ again:
> * make a new transporter (re-uses xprt)
> */
> xprt = makefd_xprt (sock, r->sendsize, r->recvsize);
> +
> + /* If we are out of memory, makefd_xprt has already dumped an error. */
> + if (xprt == NULL)
> + {
> + __svc_wait_on_error ();
> + return FALSE;
> + }
> +
> memcpy (&xprt->xp_raddr, &addr, sizeof (addr));
> xprt->xp_addrlen = len;
> return FALSE; /* there is never an rpc msg to be processed */
Ok.
> diff --git a/sunrpc/svc_unix.c b/sunrpc/svc_unix.c
> index e01afeabe6..3decea427c 100644
> --- a/sunrpc/svc_unix.c
> +++ b/sunrpc/svc_unix.c
> @@ -270,6 +270,14 @@ again:
> memset (&in_addr, '\0', sizeof (in_addr));
> in_addr.sin_family = AF_UNIX;
> xprt = makefd_xprt (sock, r->sendsize, r->recvsize);
> +
> + /* If we are out of memory, makefd_xprt has already dumped an error. */
> + if (xprt == NULL)
> + {
> + __svc_wait_on_error ();
> + return FALSE;
> + }
> +
> memcpy (&xprt->xp_raddr, &in_addr, sizeof (in_addr));
> xprt->xp_addrlen = len;
> return FALSE; /* there is never an rpc msg to be processed */
>
Ok.
On 12/9/20 9:23 PM, Adhemerval Zanella wrote:
>
>
> On 04/12/2020 13:00, Stefan Liebler via Libc-alpha wrote:
>> If glibc is build with -O3 on at least 390 (-m31) or x86 (-m32),
>> gcc 11 dumps this warning:
>> svc_tcp.c: In function 'rendezvous_request':
>> svc_tcp.c:274:3: error: 'memcpy' offset [0, 15] is out of the bounds [0, 0] [-Werror=array-bounds]
>> 274 | memcpy (&xprt->xp_raddr, &addr, sizeof (addr));
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> In out-of-memory case, if one of the mallocs in makefd_xprt function
>> returns NULL, a message is dumped, makefd_xprt returns NULL
>> and the subsequent memcpy would copy to NULL.
>>
>> Instead of a segfaulting, we delay a bit (see also __svc_accept_failed
>> and Bug 14889 (CVE-2011-4609) - svc_run() produces high cpu usage when
>> accept() fails with EMFILE (CVE-2011-4609).
>>
>> The same applies to svc_unix.c.
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Committed.
Thanks,
Stefan
@@ -38,6 +38,7 @@ libc_hidden_proto (svc_getreq_common)
libc_hidden_proto (svc_getreq_poll)
extern void __svc_accept_failed (void) attribute_hidden;
+extern void __svc_wait_on_error (void) attribute_hidden;
# endif /* !_ISOMAC */
#endif
@@ -545,6 +545,13 @@ svc_getreq_common (const int fd)
}
libc_hidden_nolink_sunrpc (svc_getreq_common, GLIBC_2_2)
+void
+__svc_wait_on_error (void)
+{
+ struct timespec ts = { .tv_sec = 0, .tv_nsec = 50000000 };
+ __nanosleep (&ts, NULL);
+}
+
/* If there are no file descriptors available, then accept will fail.
We want to delay here so the connection request can be dequeued;
otherwise we can bounce between polling and accepting, never giving the
@@ -555,8 +562,7 @@ __svc_accept_failed (void)
{
if (errno == EMFILE)
{
- struct timespec ts = { .tv_sec = 0, .tv_nsec = 50000000 };
- __nanosleep (&ts, NULL);
+ __svc_wait_on_error ();
}
}
@@ -271,6 +271,14 @@ again:
* make a new transporter (re-uses xprt)
*/
xprt = makefd_xprt (sock, r->sendsize, r->recvsize);
+
+ /* If we are out of memory, makefd_xprt has already dumped an error. */
+ if (xprt == NULL)
+ {
+ __svc_wait_on_error ();
+ return FALSE;
+ }
+
memcpy (&xprt->xp_raddr, &addr, sizeof (addr));
xprt->xp_addrlen = len;
return FALSE; /* there is never an rpc msg to be processed */
@@ -270,6 +270,14 @@ again:
memset (&in_addr, '\0', sizeof (in_addr));
in_addr.sin_family = AF_UNIX;
xprt = makefd_xprt (sock, r->sendsize, r->recvsize);
+
+ /* If we are out of memory, makefd_xprt has already dumped an error. */
+ if (xprt == NULL)
+ {
+ __svc_wait_on_error ();
+ return FALSE;
+ }
+
memcpy (&xprt->xp_raddr, &in_addr, sizeof (in_addr));
xprt->xp_addrlen = len;
return FALSE; /* there is never an rpc msg to be processed */