lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <848c64cb6c6cf88fbbcf61624810c060f858a217.camel@kernel.org> Date: Tue, 30 May 2023 11:44:23 -0400 From: Jeff Layton <jlayton@...nel.org> To: Dan Carpenter <dan.carpenter@...aro.org>, Stanislav Kinsbursky <skinsbursky@...allels.com> Cc: Chuck Lever <chuck.lever@...cle.com>, Trond Myklebust <trond.myklebust@...merspace.com>, Anna Schumaker <anna@...nel.org>, "J. Bruce Fields" <bfields@...hat.com>, linux-nfs@...r.kernel.org, netdev@...r.kernel.org, kernel-janitors@...r.kernel.org Subject: Re: [PATCH] nfsd: fix double fget() bug in __write_ports_addfd() On Mon, 2023-05-29 at 14:35 +0300, Dan Carpenter wrote: > The bug here is that you cannot rely on getting the same socket > from multiple calls to fget() because userspace can influence > that. This is a kind of double fetch bug. > Nice catch. > The fix is to delete the svc_alien_sock() function and insted do > the checking inside the svc_addsock() function. > > Fixes: 3064639423c4 ("nfsd: check passed socket's net matches NFSd superblock's one") > Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org> > --- > Based on static analysis and untested. This goes through the NFS tree. > Inspired by CVE-2023-1838. > > include/linux/sunrpc/svcsock.h | 7 +++---- > fs/nfsd/nfsctl.c | 7 +------ > net/sunrpc/svcsock.c | 23 +++++------------------ > 3 files changed, 9 insertions(+), 28 deletions(-) > > diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h > index d16ae621782c..a7116048a4d4 100644 > --- a/include/linux/sunrpc/svcsock.h > +++ b/include/linux/sunrpc/svcsock.h > @@ -61,10 +61,9 @@ int svc_recv(struct svc_rqst *, long); > void svc_send(struct svc_rqst *rqstp); > void svc_drop(struct svc_rqst *); > void svc_sock_update_bufs(struct svc_serv *serv); > -bool svc_alien_sock(struct net *net, int fd); > -int svc_addsock(struct svc_serv *serv, const int fd, > - char *name_return, const size_t len, > - const struct cred *cred); > +int svc_addsock(struct svc_serv *serv, struct net *net, > + const int fd, char *name_return, const size_t len, > + const struct cred *cred); > void svc_init_xprt_sock(void); > void svc_cleanup_xprt_sock(void); > struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot); > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c > index e0e98b40a6e5..1489e0b703b4 100644 > --- a/fs/nfsd/nfsctl.c > +++ b/fs/nfsd/nfsctl.c > @@ -698,16 +698,11 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net, const struct cred > return -EINVAL; > trace_nfsd_ctl_ports_addfd(net, fd); > > - if (svc_alien_sock(net, fd)) { > - printk(KERN_ERR "%s: socket net is different to NFSd's one\n", __func__); > - return -EINVAL; > - } > - > err = nfsd_create_serv(net); > if (err != 0) > return err; > > - err = svc_addsock(nn->nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred); > + err = svc_addsock(nn->nfsd_serv, net, fd, buf, SIMPLE_TRANSACTION_LIMIT, cred); > > if (err >= 0 && > !nn->nfsd_serv->sv_nrthreads && !xchg(&nn->keep_active, 1)) > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c > index 46845cb6465d..e4184e40793c 100644 > --- a/net/sunrpc/svcsock.c > +++ b/net/sunrpc/svcsock.c > @@ -1474,22 +1474,6 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv, > return svsk; > } > > -bool svc_alien_sock(struct net *net, int fd) > -{ > - int err; > - struct socket *sock = sockfd_lookup(fd, &err); > - bool ret = false; > - > - if (!sock) > - goto out; > - if (sock_net(sock->sk) != net) > - ret = true; > - sockfd_put(sock); > -out: > - return ret; > -} > -EXPORT_SYMBOL_GPL(svc_alien_sock); > - > /** > * svc_addsock - add a listener socket to an RPC service > * @serv: pointer to RPC service to which to add a new listener > @@ -1502,8 +1486,8 @@ EXPORT_SYMBOL_GPL(svc_alien_sock); > * Name is terminated with '\n'. On error, returns a negative errno > * value. > */ > -int svc_addsock(struct svc_serv *serv, const int fd, char *name_return, > - const size_t len, const struct cred *cred) > +int svc_addsock(struct svc_serv *serv, struct net *net, const int fd, > + char *name_return, const size_t len, const struct cred *cred) > { > int err = 0; > struct socket *so = sockfd_lookup(fd, &err); > @@ -1514,6 +1498,9 @@ int svc_addsock(struct svc_serv *serv, const int fd, char *name_return, > > if (!so) > return err; > + err = -EINVAL; > + if (sock_net(so->sk) != net) > + goto out; > err = -EAFNOSUPPORT; > if ((so->sk->sk_family != PF_INET) && (so->sk->sk_family != PF_INET6)) > goto out; Reviewed-by: Jeff Layton <jlayton@...nel.org>
Powered by blists - more mailing lists