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

From: Stanislav Kinsbursky
Date: Wed Feb 19 2014 - 09:57:59 EST


19.02.2014 18:50, J. Bruce Fields ÐÐÑÐÑ:
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.


Thank you, Bruce!

--b.


--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


--
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/