Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one

[Stanislav Kinsbursky]

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.

> --b
>
> commit e1f2922c12cb59baba0f2c7726bee992a0861310
> Author: Stanislav Kinsbursky <skinsbursky@xxxxxxxxxxxxx>
> 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@xxxxxxxxxx>
>      Signed-off-by: Stanislav Kinsbursky <skinsbursky@xxxxxxxxxxxxx>
>      Cc: stable@xxxxxxxxxxxxxxx
>      Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/