[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140219145043.GC8401@fieldses.org>
Date: Wed, 19 Feb 2014 09:50:43 -0500
From: "J. Bruce Fields" <bfields@...ldses.org>
To: Stanislav Kinsbursky <skinsbursky@...allels.com>
Cc: Weng Meiling <wengmeiling.weng@...wei.com>, greg@...ah.com,
linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
devel@...nvz.org
Subject: Re: [PATCH] nfsd: check passed socket's net matches NFSd
superblock's one
On Wed, Feb 19, 2014 at 02:26:33PM +0400, Stanislav Kinsbursky wrote:
> 18.02.2014 19:44, J. Bruce Fields пишет:
> >On Tue, Feb 18, 2014 at 07:19:31PM +0400, Stanislav Kinsbursky wrote:
> >>18.02.2014 02:19, J. Bruce Fields пишет:
> >>>On Sat, Feb 15, 2014 at 09:51:20AM +0800, Weng Meiling wrote:
> >>>>Hi Bruce,
> >>>>
> >>>>The upstream has merged your git tree for-3.14, but there is no this patch?
> >>>>Do you forget this patch?
> >>>
> >>>Apologies, I'm not sure what happened.
> >>>
> >>>Looking back at it.... The patch causes all my pynfs reboot recovery
> >>>tests to fail. They're just doing a "systemctl restart
> >>>nfs-server.service", and "systemctl status nfs-server.service" shows in
> >>>part
> >>>
> >>> ExecStart=/usr/sbin/rpc.nfsd $RPCNFSDARGS $RPCNFSDCOUNT (code=exited, status=1/FAILURE)
> >>>
> >>>So the patch is causing rpc.nfsd to fail? No network namespaces should
> >>>be involved.
> >>>
> >>>I haven't investigated any further.
> >>>
> >>
> >>Hi Bruce,
> >>Are you sure, that exactly this patch broke your pynfs tests?
> >>BTW, systemd manipulates namespaces. Maybe the patch revealed some pynfs internal bugs?
> >>What do you think?
> >
> >It's really just "systemctl restart nfs-server.service" that the patch
> >breaks, pynfs isn't involved much.
> >
> >The patch I'm actually using follows, but I believe the only difference
> >is in the printk message?
> >
>
> Yep, looks true.
> That's strange: "systemctl restart nfs-server.service" works for me on Fedora 18 with kernel, based on your repo.
OK, I'll take a closer look and let you know what I find.
--b.
>
> >--b
> >
> >commit e1f2922c12cb59baba0f2c7726bee992a0861310
> >Author: Stanislav Kinsbursky <skinsbursky@...allels.com>
> >Date: Mon Dec 30 17:23:59 2013 +0300
> >
> > nfsd: check passed socket's net matches the NFSd superblock's
> >
> > The file descriptor written to the nfsd/portlist file could be for a
> > socket in a different network namespace from the network namespace that
> > the nfsd filesystem was mounted in, and this can cause a crash.
> >
> > For example: "ip netns exec" creates a new network and mount namespace,
> > which duplicates the nfsd mount point which was created in the init_net
> > context. Thus NFS server stop in the nested network context leads to
> > RPCBIND client destruction in init_net. Then, on nfsd start in the
> > nested network context, the rpc.nfsd process creates a socket in the
> > nested net and passes it into "write_ports", which leads to RPCBIND
> > socket creation in init_net context for the same reason (the nfsd mount
> > point was created in the init_net context). An attempt to register
> > passed socket in nested net leads to panic, because no RPCBIND client is
> > present in the nested network namespace.
> >
> > This patch adds a check that the passed the socket's net matches the
> > nfsd superblock's net, and returns -EINVAL error to user space if not.
> >
> > Reported-by: Weng Meiling <wengmeiling.weng@...wei.com>
> > Signed-off-by: Stanislav Kinsbursky <skinsbursky@...allels.com>
> > Cc: stable@...r.kernel.org
> > Signed-off-by: J. Bruce Fields <bfields@...hat.com>
> >
> >diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> >index 7f55517..1331766 100644
> >--- a/fs/nfsd/nfsctl.c
> >+++ b/fs/nfsd/nfsctl.c
> >@@ -699,6 +699,11 @@ static ssize_t __write_ports_addfd(char *buf, struct net *net)
> > if (err != 0 || fd < 0)
> > return -EINVAL;
> >
> >+ if (svc_alien_sock(net, fd)) {
> >+ printk(KERN_ERR "%s: socket net is different from NFSd's\n", __func__);
> >+ return -EINVAL;
> >+ }
> >+
> > err = nfsd_create_serv(net);
> > if (err != 0)
> > return err;
> >diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h
> >index 62fd1b7..947009e 100644
> >--- a/include/linux/sunrpc/svcsock.h
> >+++ b/include/linux/sunrpc/svcsock.h
> >@@ -56,6 +56,7 @@ int svc_recv(struct svc_rqst *, long);
> > int svc_send(struct svc_rqst *);
> > 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);
> > void svc_init_xprt_sock(void);
> >diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> >index b6e59f0..3ba5b87 100644
> >--- a/net/sunrpc/svcsock.c
> >+++ b/net/sunrpc/svcsock.c
> >@@ -1397,6 +1397,17 @@ 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);
> >+
> >+ if (sock && (sock_net(sock->sk) != net))
> >+ return true;
> >+ return false;
> >+}
> >+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
> >
>
>
> --
> Best regards,
> Stanislav Kinsbursky
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists