Message ID | 20110902034138.22595.9759.stgit@ltc219.sdl.hitachi.co.jp |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On Fri, Sep 02, 2011 at 12:41:38PM +0900, Mitsuo Hayasaka wrote: > The sk_sndbuf and sk_rcvbuf fields of struct sock are sizes of send and > receive socket buffers respectively, and are defined as integer. > The sunrpc which is used in NFSD and any other applications I'd call those "kernel subsystems" or "in-kernel applications"--it needs to be clear that you're not talking about userspace applications. > can change them > via svc_sock_setbufsize(). It, however, sets them as unsigned integer and > may cause overflow of integer. This leads to a degradation of networking > capability. Tracing through the callers, actually I believe they all set this to a constant, with the single exception of nfsd, which allows the size to be configured; but write_maxlksize already limits it to NFSSVC_MAXBLKSIZE. So this patch looks unnecessary to me, unless I'm missing something. If you can argue that it would be safer to check here as well, I might consider that, but please: - do make that argument in detail; - especially, convince me that *this* is the right place for the check; and - also double-check my audit of the callers. And include all that in the changelog. Dropping for now. --b. > > This patch adds integer-overflow check into svc_sock_setbufsize() before > both fields are set, and limits their maximum sizes to INT_MAX. > > Signed-off-by: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com> > Cc: Trond Myklebust <Trond.Myklebust@netapp.com> > Cc: "J. Bruce Fields" <bfields@fieldses.org> > Cc: Neil Brown <neilb@suse.de> > Cc: "David S. Miller" <davem@davemloft.net> > --- > > net/sunrpc/svcsock.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c > index 767d494..bd66775 100644 > --- a/net/sunrpc/svcsock.c > +++ b/net/sunrpc/svcsock.c > @@ -54,6 +54,7 @@ > #include "sunrpc.h" > > #define RPCDBG_FACILITY RPCDBG_SVCXPRT > +#define MAX_SKBUFSIZ INT_MAX > > > static struct svc_sock *svc_setup_socket(struct svc_serv *, struct socket *, > @@ -435,6 +436,11 @@ static void svc_sock_setbufsize(struct socket *sock, unsigned int snd, > * on not having CAP_SYS_RESOURCE or similar, we go direct... > * DaveM said I could! > */ > + if (snd > MAX_SKBUFSIZ/2) > + snd = MAX_SKBUFSIZ/2; > + if (rcv > MAX_SKBUFSIZ/2) > + rcv = MAX_SKBUFSIZ/2; > + > lock_sock(sock->sk); > sock->sk->sk_sndbuf = snd * 2; > sock->sk->sk_rcvbuf = rcv * 2; > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index 767d494..bd66775 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -54,6 +54,7 @@ #include "sunrpc.h" #define RPCDBG_FACILITY RPCDBG_SVCXPRT +#define MAX_SKBUFSIZ INT_MAX static struct svc_sock *svc_setup_socket(struct svc_serv *, struct socket *, @@ -435,6 +436,11 @@ static void svc_sock_setbufsize(struct socket *sock, unsigned int snd, * on not having CAP_SYS_RESOURCE or similar, we go direct... * DaveM said I could! */ + if (snd > MAX_SKBUFSIZ/2) + snd = MAX_SKBUFSIZ/2; + if (rcv > MAX_SKBUFSIZ/2) + rcv = MAX_SKBUFSIZ/2; + lock_sock(sock->sk); sock->sk->sk_sndbuf = snd * 2; sock->sk->sk_rcvbuf = rcv * 2;
The sk_sndbuf and sk_rcvbuf fields of struct sock are sizes of send and receive socket buffers respectively, and are defined as integer. The sunrpc which is used in NFSD and any other applications can change them via svc_sock_setbufsize(). It, however, sets them as unsigned integer and may cause overflow of integer. This leads to a degradation of networking capability. This patch adds integer-overflow check into svc_sock_setbufsize() before both fields are set, and limits their maximum sizes to INT_MAX. Signed-off-by: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com> Cc: Trond Myklebust <Trond.Myklebust@netapp.com> Cc: "J. Bruce Fields" <bfields@fieldses.org> Cc: Neil Brown <neilb@suse.de> Cc: "David S. Miller" <davem@davemloft.net> --- net/sunrpc/svcsock.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html