| Message ID | 20201204160027.3844260-1-stli@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Handle out-of-memory case in svc_tcp.c/svc_unix.c:rendezvous_request. | expand |
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
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 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 @@ -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 (); } } 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 */ 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 */
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(-)