| Message ID | 20210807170047.68720-1-dai.ngo@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] Fix DoS vulnerability in libtirpc | expand |
> On Aug 7, 2021, at 1:00 PM, Dai Ngo <dai.ngo@oracle.com> wrote: > > Currently svc_run does not handle poll time and rendezvous_request > does not handle EMFILE error returned from accept(2 as it used to. > These two missing functionality were removed by commit b2c9430f46c4. > > The effect of not handling poll timeout allows idle TCP conections > to remain ESTABLISHED indefinitely. When the number of connections > reaches the limit of the open file descriptors (ulimit -n) then > accept(2) fails with EMFILE. Since there is no handling of EMFILE > error this causes svc_run() to get in a tight loop calling accept(2). > This resulting in the RPC service of svc_run is being down, it's > no longer able to service any requests. > > The below script, td.sh, with nc (nmap-ncat-7.80-3) can be used > to take down the RPC service: > > if [ $# -ne 3 ]; then > echo "$0: server dst_port conn_cnt" > exit > fi > server=$1 > dport=$2 > conn_cnt=$3 > echo "dport[$dport] server[$server] conn_cnt[$conn_cnt]" > > pcnt=0 > while [ $pcnt -lt $conn_cnt ] > do > nc -v --recv-only $server $dport & > pcnt=`expr $pcnt + 1` > done > > Fix by restoring code in svc_run to cleanup idle conncetions when > poll(2) returns 0 (timeout) and in rendezvous_request to handle > EMFILE error returned from accept(2). > > Fixes: b2c9430f46c4 Use poll() instead of select() in svc_run() > Signed-off-by: dai.ngo@oracle.com > --- > src/libtirpc.map | 2 +- > src/rpc_com.h | 2 ++ > src/svc.c | 2 +- > src/svc_run.c | 2 ++ > src/svc_vc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 54 insertions(+), 2 deletions(-) > > diff --git a/src/libtirpc.map b/src/libtirpc.map > index 21d60651ff57..b754110faadb 100644 > --- a/src/libtirpc.map > +++ b/src/libtirpc.map > @@ -331,5 +331,5 @@ TIRPC_PRIVATE { > global: > __libc_clntudp_bufcreate; > # private, but used by rpcbind: > - __svc_clean_idle; svc_auth_none; libtirpc_set_debug; > + __svc_destroy_idle; __svc_clean_idle; svc_auth_none; libtirpc_set_debug; As I stated yesterday, as a reviewer I agree with everything in this patch except for the addition of __svc_destroy_idle to the library's private API list. IMO the private __svc_clean_idle() API was a mistake we shouldn't make again. (That might be why Thorsten removed it years ago). Thanks, Dai, for your work nailing this down! > }; > diff --git a/src/rpc_com.h b/src/rpc_com.h > index 76badefcfe90..ede6ec8b1d4e 100644 > --- a/src/rpc_com.h > +++ b/src/rpc_com.h > @@ -55,6 +55,7 @@ struct netbuf *__rpcb_findaddr_timed(rpcprog_t, rpcvers_t, > bool_t __rpc_control(int,void *); > > bool_t __svc_clean_idle(fd_set *, int, bool_t); > +void __svc_destroy_idle(int, bool_t); > bool_t __xdrrec_setnonblock(XDR *, int); > bool_t __xdrrec_getrec(XDR *, enum xprt_stat *, bool_t); > void __xprt_unregister_unlocked(SVCXPRT *); > @@ -62,6 +63,7 @@ void __xprt_set_raddr(SVCXPRT *, const struct sockaddr_storage *); > > > extern int __svc_maxrec; > +extern SVCXPRT **__svc_xports; > > #ifdef __cplusplus > } > diff --git a/src/svc.c b/src/svc.c > index 6db164bbd76b..aa0c92591914 100644 > --- a/src/svc.c > +++ b/src/svc.c > @@ -57,7 +57,7 @@ > > #define max(a, b) (a > b ? a : b) > > -static SVCXPRT **__svc_xports; > +SVCXPRT **__svc_xports; > int __svc_maxrec; > > /* > diff --git a/src/svc_run.c b/src/svc_run.c > index f40314b9948e..4eba36174524 100644 > --- a/src/svc_run.c > +++ b/src/svc_run.c > @@ -44,6 +44,7 @@ > #include "rpc_com.h" > #include <sys/select.h> > > + > void > svc_run() > { > @@ -86,6 +87,7 @@ svc_run() > warn ("svc_run: - poll failed"); > break; > case 0: > + __svc_destroy_idle(30, FALSE); > continue; > default: > svc_getreq_poll (my_pollfd, i); > diff --git a/src/svc_vc.c b/src/svc_vc.c > index f1d9f001fcdc..4880ab5dbc26 100644 > --- a/src/svc_vc.c > +++ b/src/svc_vc.c > @@ -58,6 +58,7 @@ > > #include <rpc/rpc.h> > > +#include "debug.h" > #include "rpc_com.h" > > #include <getpeereid.h> > @@ -337,6 +338,10 @@ again: > if (sock < 0) { > if (errno == EINTR) > goto again; > + if (errno == EMFILE || errno == ENFILE) { > + /* remove least active fd */ > + __svc_destroy_idle(0, FALSE); > + } > return (FALSE); > } > /* > @@ -820,3 +825,46 @@ __svc_clean_idle(fd_set *fds, int timeout, bool_t cleanblock) > { > return FALSE; > } > + > +void > +__svc_destroy_idle(int timeout, bool_t cleanblock) > +{ > + int i; > + SVCXPRT *xprt, *least_active; > + struct timeval tv, tdiff, tmax; > + struct cf_conn *cd; > + > + gettimeofday(&tv, NULL); > + tmax.tv_sec = tmax.tv_usec = 0; > + least_active = NULL; > + rwlock_wrlock(&svc_fd_lock); > + > + for (i = 0; i <= svc_max_pollfd; i++) { > + if (svc_pollfd[i].fd == -1) > + continue; > + xprt = __svc_xports[i]; > + if (xprt == NULL || xprt->xp_ops == NULL || > + xprt->xp_ops->xp_recv != svc_vc_recv) > + continue; > + cd = (struct cf_conn *)xprt->xp_p1; > + if (!cleanblock && !cd->nonblock) > + continue; > + if (timeout == 0) { > + timersub(&tv, &cd->last_recv_time, &tdiff); > + if (timercmp(&tdiff, &tmax, >)) { > + tmax = tdiff; > + least_active = xprt; > + } > + continue; > + } > + if (tv.tv_sec - cd->last_recv_time.tv_sec > timeout) { > + __xprt_unregister_unlocked(xprt); > + __svc_vc_dodestroy(xprt); > + } > + } > + if (timeout == 0 && least_active != NULL) { > + __xprt_unregister_unlocked(least_active); > + __svc_vc_dodestroy(least_active); > + } > + rwlock_unlock(&svc_fd_lock); > +} > -- > 2.26.2 -- Chuck Lever
diff --git a/src/libtirpc.map b/src/libtirpc.map index 21d60651ff57..b754110faadb 100644 --- a/src/libtirpc.map +++ b/src/libtirpc.map @@ -331,5 +331,5 @@ TIRPC_PRIVATE { global: __libc_clntudp_bufcreate; # private, but used by rpcbind: - __svc_clean_idle; svc_auth_none; libtirpc_set_debug; + __svc_destroy_idle; __svc_clean_idle; svc_auth_none; libtirpc_set_debug; }; diff --git a/src/rpc_com.h b/src/rpc_com.h index 76badefcfe90..ede6ec8b1d4e 100644 --- a/src/rpc_com.h +++ b/src/rpc_com.h @@ -55,6 +55,7 @@ struct netbuf *__rpcb_findaddr_timed(rpcprog_t, rpcvers_t, bool_t __rpc_control(int,void *); bool_t __svc_clean_idle(fd_set *, int, bool_t); +void __svc_destroy_idle(int, bool_t); bool_t __xdrrec_setnonblock(XDR *, int); bool_t __xdrrec_getrec(XDR *, enum xprt_stat *, bool_t); void __xprt_unregister_unlocked(SVCXPRT *); @@ -62,6 +63,7 @@ void __xprt_set_raddr(SVCXPRT *, const struct sockaddr_storage *); extern int __svc_maxrec; +extern SVCXPRT **__svc_xports; #ifdef __cplusplus } diff --git a/src/svc.c b/src/svc.c index 6db164bbd76b..aa0c92591914 100644 --- a/src/svc.c +++ b/src/svc.c @@ -57,7 +57,7 @@ #define max(a, b) (a > b ? a : b) -static SVCXPRT **__svc_xports; +SVCXPRT **__svc_xports; int __svc_maxrec; /* diff --git a/src/svc_run.c b/src/svc_run.c index f40314b9948e..4eba36174524 100644 --- a/src/svc_run.c +++ b/src/svc_run.c @@ -44,6 +44,7 @@ #include "rpc_com.h" #include <sys/select.h> + void svc_run() { @@ -86,6 +87,7 @@ svc_run() warn ("svc_run: - poll failed"); break; case 0: + __svc_destroy_idle(30, FALSE); continue; default: svc_getreq_poll (my_pollfd, i); diff --git a/src/svc_vc.c b/src/svc_vc.c index f1d9f001fcdc..4880ab5dbc26 100644 --- a/src/svc_vc.c +++ b/src/svc_vc.c @@ -58,6 +58,7 @@ #include <rpc/rpc.h> +#include "debug.h" #include "rpc_com.h" #include <getpeereid.h> @@ -337,6 +338,10 @@ again: if (sock < 0) { if (errno == EINTR) goto again; + if (errno == EMFILE || errno == ENFILE) { + /* remove least active fd */ + __svc_destroy_idle(0, FALSE); + } return (FALSE); } /* @@ -820,3 +825,46 @@ __svc_clean_idle(fd_set *fds, int timeout, bool_t cleanblock) { return FALSE; } + +void +__svc_destroy_idle(int timeout, bool_t cleanblock) +{ + int i; + SVCXPRT *xprt, *least_active; + struct timeval tv, tdiff, tmax; + struct cf_conn *cd; + + gettimeofday(&tv, NULL); + tmax.tv_sec = tmax.tv_usec = 0; + least_active = NULL; + rwlock_wrlock(&svc_fd_lock); + + for (i = 0; i <= svc_max_pollfd; i++) { + if (svc_pollfd[i].fd == -1) + continue; + xprt = __svc_xports[i]; + if (xprt == NULL || xprt->xp_ops == NULL || + xprt->xp_ops->xp_recv != svc_vc_recv) + continue; + cd = (struct cf_conn *)xprt->xp_p1; + if (!cleanblock && !cd->nonblock) + continue; + if (timeout == 0) { + timersub(&tv, &cd->last_recv_time, &tdiff); + if (timercmp(&tdiff, &tmax, >)) { + tmax = tdiff; + least_active = xprt; + } + continue; + } + if (tv.tv_sec - cd->last_recv_time.tv_sec > timeout) { + __xprt_unregister_unlocked(xprt); + __svc_vc_dodestroy(xprt); + } + } + if (timeout == 0 && least_active != NULL) { + __xprt_unregister_unlocked(least_active); + __svc_vc_dodestroy(least_active); + } + rwlock_unlock(&svc_fd_lock); +}
Currently svc_run does not handle poll time and rendezvous_request does not handle EMFILE error returned from accept(2 as it used to. These two missing functionality were removed by commit b2c9430f46c4. The effect of not handling poll timeout allows idle TCP conections to remain ESTABLISHED indefinitely. When the number of connections reaches the limit of the open file descriptors (ulimit -n) then accept(2) fails with EMFILE. Since there is no handling of EMFILE error this causes svc_run() to get in a tight loop calling accept(2). This resulting in the RPC service of svc_run is being down, it's no longer able to service any requests. The below script, td.sh, with nc (nmap-ncat-7.80-3) can be used to take down the RPC service: if [ $# -ne 3 ]; then echo "$0: server dst_port conn_cnt" exit fi server=$1 dport=$2 conn_cnt=$3 echo "dport[$dport] server[$server] conn_cnt[$conn_cnt]" pcnt=0 while [ $pcnt -lt $conn_cnt ] do nc -v --recv-only $server $dport & pcnt=`expr $pcnt + 1` done Fix by restoring code in svc_run to cleanup idle conncetions when poll(2) returns 0 (timeout) and in rendezvous_request to handle EMFILE error returned from accept(2). Fixes: b2c9430f46c4 Use poll() instead of select() in svc_run() Signed-off-by: dai.ngo@oracle.com --- src/libtirpc.map | 2 +- src/rpc_com.h | 2 ++ src/svc.c | 2 +- src/svc_run.c | 2 ++ src/svc_vc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 54 insertions(+), 2 deletions(-)