All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
@ 2013-12-30 14:23 Stanislav Kinsbursky
  2014-01-03 22:22 ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2013-12-30 14:23 UTC (permalink / raw)
  To: bfields; +Cc: wengmeiling.weng, greg, linux-nfs, linux-kernel, devel

There could be a case, when NFSd file system is mounted in network, different
to socket's one, like below:

"ip netns exec" creates new network and mount namespace, which duplicates NFSd
mount point, created in init_net context. And thus NFS server stop in nested
network context leads to RPCBIND client destruction in init_net.
Then, on NFSd start in nested network context, rpc.nfsd process creates socket
in nested net and passes it into "write_ports", which leads to RPCBIND sockets
creation in init_net context because of the same reason (NFSd monut point was
created in init_net context). An attempt to register passed socket in nested
net leads to panic, because no RPCBIND client present in nexted network
namespace.

This patch add check that passed socket's net matches NFSd superblock's one.
And returns -EINVAL error to user psace otherwise.

Reported-by: Weng Meiling <wengmeiling.weng@huawei.com>
Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: stable@vger.kernel.org
---
 fs/nfsd/nfsctl.c               |    5 +++++
 include/linux/sunrpc/svcsock.h |    1 +
 net/sunrpc/svcsock.c           |   11 +++++++++++
 3 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 7f55517..f34d9de 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 to NFSd's one\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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2013-12-30 14:23 [PATCH] nfsd: check passed socket's net matches NFSd superblock's one Stanislav Kinsbursky
@ 2014-01-03 22:22 ` J. Bruce Fields
  2014-01-09  7:15   ` Stanislav Kinsbursky
  2014-02-15  1:51   ` Weng Meiling
  0 siblings, 2 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-01-03 22:22 UTC (permalink / raw)
  To: Stanislav Kinsbursky
  Cc: wengmeiling.weng, greg, linux-nfs, linux-kernel, devel

On Mon, Dec 30, 2013 at 05:23:59PM +0300, Stanislav Kinsbursky wrote:
> There could be a case, when NFSd file system is mounted in network, different
> to socket's one, like below:
> 
> "ip netns exec" creates new network and mount namespace, which duplicates NFSd
> mount point, created in init_net context. And thus NFS server stop in nested
> network context leads to RPCBIND client destruction in init_net.
> Then, on NFSd start in nested network context, rpc.nfsd process creates socket
> in nested net and passes it into "write_ports", which leads to RPCBIND sockets
> creation in init_net context because of the same reason (NFSd monut point was
> created in init_net context). An attempt to register passed socket in nested
> net leads to panic, because no RPCBIND client present in nexted network
> namespace.

So it's the attempt to use a NULL ->rpcb_local_clnt4?

Interesting, thanks--applying with a minor fix to logged message.

--b.

> 
> This patch add check that passed socket's net matches NFSd superblock's one.
> And returns -EINVAL error to user psace otherwise.
> 
> Reported-by: Weng Meiling <wengmeiling.weng@huawei.com>
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/nfsd/nfsctl.c               |    5 +++++
>  include/linux/sunrpc/svcsock.h |    1 +
>  net/sunrpc/svcsock.c           |   11 +++++++++++
>  3 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 7f55517..f34d9de 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 to NFSd's one\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
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-01-03 22:22 ` J. Bruce Fields
@ 2014-01-09  7:15   ` Stanislav Kinsbursky
  2014-02-15  1:51   ` Weng Meiling
  1 sibling, 0 replies; 15+ messages in thread
From: Stanislav Kinsbursky @ 2014-01-09  7:15 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: wengmeiling.weng, greg, linux-nfs, linux-kernel, devel

04.01.2014 02:22, J. Bruce Fields пишет:
> On Mon, Dec 30, 2013 at 05:23:59PM +0300, Stanislav Kinsbursky wrote:
>> There could be a case, when NFSd file system is mounted in network, different
>> to socket's one, like below:
>>
>> "ip netns exec" creates new network and mount namespace, which duplicates NFSd
>> mount point, created in init_net context. And thus NFS server stop in nested
>> network context leads to RPCBIND client destruction in init_net.
>> Then, on NFSd start in nested network context, rpc.nfsd process creates socket
>> in nested net and passes it into "write_ports", which leads to RPCBIND sockets
>> creation in init_net context because of the same reason (NFSd monut point was
>> created in init_net context). An attempt to register passed socket in nested
>> net leads to panic, because no RPCBIND client present in nexted network
>> namespace.
>
> So it's the attempt to use a NULL ->rpcb_local_clnt4?
>

Correct.

-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-01-03 22:22 ` J. Bruce Fields
  2014-01-09  7:15   ` Stanislav Kinsbursky
@ 2014-02-15  1:51   ` Weng Meiling
  2014-02-17 22:19     ` J. Bruce Fields
  1 sibling, 1 reply; 15+ messages in thread
From: Weng Meiling @ 2014-02-15  1:51 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Stanislav Kinsbursky, greg, linux-nfs, linux-kernel, devel

Hi Bruce,

The upstream has merged your git tree for-3.14, but there is no this patch?
Do you forget this patch?

Thanks!
Weng Meiling


On 2014/1/4 6:22, J. Bruce Fields wrote:
> On Mon, Dec 30, 2013 at 05:23:59PM +0300, Stanislav Kinsbursky wrote:
>> There could be a case, when NFSd file system is mounted in network, different
>> to socket's one, like below:
>>
>> "ip netns exec" creates new network and mount namespace, which duplicates NFSd
>> mount point, created in init_net context. And thus NFS server stop in nested
>> network context leads to RPCBIND client destruction in init_net.
>> Then, on NFSd start in nested network context, rpc.nfsd process creates socket
>> in nested net and passes it into "write_ports", which leads to RPCBIND sockets
>> creation in init_net context because of the same reason (NFSd monut point was
>> created in init_net context). An attempt to register passed socket in nested
>> net leads to panic, because no RPCBIND client present in nexted network
>> namespace.
> 
> So it's the attempt to use a NULL ->rpcb_local_clnt4?
> 
> Interesting, thanks--applying with a minor fix to logged message.
> 
> --b.
> 




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-15  1:51   ` Weng Meiling
@ 2014-02-17 22:19     ` J. Bruce Fields
  2014-02-18 13:06       ` Weng Meiling
  2014-02-18 15:19       ` Stanislav Kinsbursky
  0 siblings, 2 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-17 22:19 UTC (permalink / raw)
  To: Weng Meiling; +Cc: Stanislav Kinsbursky, greg, linux-nfs, linux-kernel, devel

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.

--b.

> 
> Thanks!
> Weng Meiling
> 
> 
> On 2014/1/4 6:22, J. Bruce Fields wrote:
> > On Mon, Dec 30, 2013 at 05:23:59PM +0300, Stanislav Kinsbursky wrote:
> >> There could be a case, when NFSd file system is mounted in network, different
> >> to socket's one, like below:
> >>
> >> "ip netns exec" creates new network and mount namespace, which duplicates NFSd
> >> mount point, created in init_net context. And thus NFS server stop in nested
> >> network context leads to RPCBIND client destruction in init_net.
> >> Then, on NFSd start in nested network context, rpc.nfsd process creates socket
> >> in nested net and passes it into "write_ports", which leads to RPCBIND sockets
> >> creation in init_net context because of the same reason (NFSd monut point was
> >> created in init_net context). An attempt to register passed socket in nested
> >> net leads to panic, because no RPCBIND client present in nexted network
> >> namespace.
> > 
> > So it's the attempt to use a NULL ->rpcb_local_clnt4?
> > 
> > Interesting, thanks--applying with a minor fix to logged message.
> > 
> > --b.
> > 
> 
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-17 22:19     ` J. Bruce Fields
@ 2014-02-18 13:06       ` Weng Meiling
  2014-02-18 15:19       ` Stanislav Kinsbursky
  1 sibling, 0 replies; 15+ messages in thread
From: Weng Meiling @ 2014-02-18 13:06 UTC (permalink / raw)
  To: J. Bruce Fields, Stanislav Kinsbursky
  Cc: greg, linux-nfs, linux-kernel, devel

Hi Bruce , Stanislav
On 2014/2/18 6:19, J. Bruce Fields wrote:
> 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.
> 
the problem exists. Sorry for careless testing.
Stanislav, how do you think about this?

Thanks!
Weng Meiling
> --b.
> 
>>
>> Thanks!
>> Weng Meiling
>>
>>
>> On 2014/1/4 6:22, J. Bruce Fields wrote:
>>> On Mon, Dec 30, 2013 at 05:23:59PM +0300, Stanislav Kinsbursky wrote:
>>>> There could be a case, when NFSd file system is mounted in network, different
>>>> to socket's one, like below:
>>>>
>>>> "ip netns exec" creates new network and mount namespace, which duplicates NFSd
>>>> mount point, created in init_net context. And thus NFS server stop in nested
>>>> network context leads to RPCBIND client destruction in init_net.
>>>> Then, on NFSd start in nested network context, rpc.nfsd process creates socket
>>>> in nested net and passes it into "write_ports", which leads to RPCBIND sockets
>>>> creation in init_net context because of the same reason (NFSd monut point was
>>>> created in init_net context). An attempt to register passed socket in nested
>>>> net leads to panic, because no RPCBIND client present in nexted network
>>>> namespace.
>>>
>>> So it's the attempt to use a NULL ->rpcb_local_clnt4?
>>>
>>> Interesting, thanks--applying with a minor fix to logged message.
>>>
>>> --b.
>>>
>>
>>
>>
> 
> .
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-17 22:19     ` J. Bruce Fields
  2014-02-18 13:06       ` Weng Meiling
@ 2014-02-18 15:19       ` Stanislav Kinsbursky
  2014-02-18 15:44         ` J. Bruce Fields
  1 sibling, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2014-02-18 15:19 UTC (permalink / raw)
  To: J. Bruce Fields, Weng Meiling; +Cc: greg, linux-nfs, linux-kernel, devel

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?

> --b.
>
>>
>> Thanks!
>> Weng Meiling
>>
>>
>> On 2014/1/4 6:22, J. Bruce Fields wrote:
>>> On Mon, Dec 30, 2013 at 05:23:59PM +0300, Stanislav Kinsbursky wrote:
>>>> There could be a case, when NFSd file system is mounted in network, different
>>>> to socket's one, like below:
>>>>
>>>> "ip netns exec" creates new network and mount namespace, which duplicates NFSd
>>>> mount point, created in init_net context. And thus NFS server stop in nested
>>>> network context leads to RPCBIND client destruction in init_net.
>>>> Then, on NFSd start in nested network context, rpc.nfsd process creates socket
>>>> in nested net and passes it into "write_ports", which leads to RPCBIND sockets
>>>> creation in init_net context because of the same reason (NFSd monut point was
>>>> created in init_net context). An attempt to register passed socket in nested
>>>> net leads to panic, because no RPCBIND client present in nexted network
>>>> namespace.
>>>
>>> So it's the attempt to use a NULL ->rpcb_local_clnt4?
>>>
>>> Interesting, thanks--applying with a minor fix to logged message.
>>>
>>> --b.
>>>
>>
>>
>>


-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-18 15:19       ` Stanislav Kinsbursky
@ 2014-02-18 15:44         ` J. Bruce Fields
  2014-02-19 10:26           ` Stanislav Kinsbursky
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 15:44 UTC (permalink / raw)
  To: Stanislav Kinsbursky; +Cc: Weng Meiling, greg, linux-nfs, linux-kernel, devel

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?

--b

commit e1f2922c12cb59baba0f2c7726bee992a0861310
Author: Stanislav Kinsbursky <skinsbursky@parallels.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@huawei.com>
    Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
    Cc: stable@vger.kernel.org
    Signed-off-by: J. Bruce Fields <bfields@redhat.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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-18 15:44         ` J. Bruce Fields
@ 2014-02-19 10:26           ` Stanislav Kinsbursky
  2014-02-19 14:50             ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2014-02-19 10:26 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Weng Meiling, greg, linux-nfs, linux-kernel, devel

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@parallels.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@huawei.com>
>      Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
>      Cc: stable@vger.kernel.org
>      Signed-off-by: J. Bruce Fields <bfields@redhat.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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-19 10:26           ` Stanislav Kinsbursky
@ 2014-02-19 14:50             ` J. Bruce Fields
  2014-02-19 14:57               ` Stanislav Kinsbursky
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-19 14:50 UTC (permalink / raw)
  To: Stanislav Kinsbursky; +Cc: Weng Meiling, greg, linux-nfs, linux-kernel, devel

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@parallels.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@huawei.com>
> >     Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> >     Cc: stable@vger.kernel.org
> >     Signed-off-by: J. Bruce Fields <bfields@redhat.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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-19 14:50             ` J. Bruce Fields
@ 2014-02-19 14:57               ` Stanislav Kinsbursky
  2014-02-20 21:31                 ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2014-02-19 14:57 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Weng Meiling, greg, linux-nfs, linux-kernel, devel

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@parallels.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@huawei.com>
>>>      Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
>>>      Cc: stable@vger.kernel.org
>>>      Signed-off-by: J. Bruce Fields <bfields@redhat.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


-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-19 14:57               ` Stanislav Kinsbursky
@ 2014-02-20 21:31                 ` J. Bruce Fields
  2014-02-21  9:18                   ` Stanislav Kinsbursky
  0 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-20 21:31 UTC (permalink / raw)
  To: Stanislav Kinsbursky; +Cc: Weng Meiling, greg, linux-nfs, linux-kernel, devel

On Wed, Feb 19, 2014 at 06:57:46PM +0400, Stanislav Kinsbursky wrote:
> 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!

rpc.nfsd's attempt to bind port 2049 the second time is failing with
EADDRINUSE.

svc_alien_sock isn't failing, so the only different here is that we're
running sockfd_lookup....

Does that take a reference on the fd or the sock that needs to be put?

--b.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-20 21:31                 ` J. Bruce Fields
@ 2014-02-21  9:18                   ` Stanislav Kinsbursky
  0 siblings, 0 replies; 15+ messages in thread
From: Stanislav Kinsbursky @ 2014-02-21  9:18 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Weng Meiling, greg, linux-nfs, linux-kernel, devel

21.02.2014 01:31, J. Bruce Fields пишет:
> On Wed, Feb 19, 2014 at 06:57:46PM +0400, Stanislav Kinsbursky wrote:
>> 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!
>
> rpc.nfsd's attempt to bind port 2049 the second time is failing with
> EADDRINUSE.
>
> svc_alien_sock isn't failing, so the only different here is that we're
> running sockfd_lookup....
>
> Does that take a reference on the fd or the sock that needs to be put?
>

Yes, you right. I missed it, sorry.
Thank you. Will resend.

> --b.
>


-- 
Best regards,
Stanislav Kinsbursky

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
  2014-02-26 13:50 Stanislav Kinsbursky
@ 2014-03-31 20:58 ` J. Bruce Fields
  0 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-03-31 20:58 UTC (permalink / raw)
  To: Stanislav Kinsbursky
  Cc: wengmeiling.weng, greg, linux-nfs, linux-kernel, devel

Applying, sorry for the delay.--b.

On Wed, Feb 26, 2014 at 04:50:01PM +0300, Stanislav Kinsbursky wrote:
> There could be a case, when NFSd file system is mounted in network, different
> to socket's one, like below:
> 
> "ip netns exec" creates new network and mount namespace, which duplicates NFSd
> mount point, created in init_net context. And thus NFS server stop in nested
> network context leads to RPCBIND client destruction in init_net.
> Then, on NFSd start in nested network context, rpc.nfsd process creates socket
> in nested net and passes it into "write_ports", which leads to RPCBIND sockets
> creation in init_net context because of the same reason (NFSd monut point was
> created in init_net context). An attempt to register passed socket in nested
> net leads to panic, because no RPCBIND client present in nexted network
> namespace.
> 
> This patch add check that passed socket's net matches NFSd superblock's one.
> And returns -EINVAL error to user psace otherwise.
> 
> v2: Put socket on exit.
> 
> Reported-by: Weng Meiling <wengmeiling.weng@huawei.com>
> Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
> Cc: stable@vger.kernel.org
> ---
>  fs/nfsd/nfsctl.c               |    5 +++++
>  include/linux/sunrpc/svcsock.h |    1 +
>  net/sunrpc/svcsock.c           |   16 ++++++++++++++++
>  3 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 7f55517..f34d9de 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 to NFSd's one\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..d06cb87 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1397,6 +1397,22 @@ 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
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] nfsd: check passed socket's net matches NFSd superblock's one
@ 2014-02-26 13:50 Stanislav Kinsbursky
  2014-03-31 20:58 ` J. Bruce Fields
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Kinsbursky @ 2014-02-26 13:50 UTC (permalink / raw)
  To: bfields; +Cc: wengmeiling.weng, greg, linux-nfs, linux-kernel, devel

There could be a case, when NFSd file system is mounted in network, different
to socket's one, like below:

"ip netns exec" creates new network and mount namespace, which duplicates NFSd
mount point, created in init_net context. And thus NFS server stop in nested
network context leads to RPCBIND client destruction in init_net.
Then, on NFSd start in nested network context, rpc.nfsd process creates socket
in nested net and passes it into "write_ports", which leads to RPCBIND sockets
creation in init_net context because of the same reason (NFSd monut point was
created in init_net context). An attempt to register passed socket in nested
net leads to panic, because no RPCBIND client present in nexted network
namespace.

This patch add check that passed socket's net matches NFSd superblock's one.
And returns -EINVAL error to user psace otherwise.

v2: Put socket on exit.

Reported-by: Weng Meiling <wengmeiling.weng@huawei.com>
Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
Cc: stable@vger.kernel.org
---
 fs/nfsd/nfsctl.c               |    5 +++++
 include/linux/sunrpc/svcsock.h |    1 +
 net/sunrpc/svcsock.c           |   16 ++++++++++++++++
 3 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 7f55517..f34d9de 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 to NFSd's one\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..d06cb87 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1397,6 +1397,22 @@ 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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-03-31 20:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-30 14:23 [PATCH] nfsd: check passed socket's net matches NFSd superblock's one Stanislav Kinsbursky
2014-01-03 22:22 ` J. Bruce Fields
2014-01-09  7:15   ` Stanislav Kinsbursky
2014-02-15  1:51   ` Weng Meiling
2014-02-17 22:19     ` J. Bruce Fields
2014-02-18 13:06       ` Weng Meiling
2014-02-18 15:19       ` Stanislav Kinsbursky
2014-02-18 15:44         ` J. Bruce Fields
2014-02-19 10:26           ` Stanislav Kinsbursky
2014-02-19 14:50             ` J. Bruce Fields
2014-02-19 14:57               ` Stanislav Kinsbursky
2014-02-20 21:31                 ` J. Bruce Fields
2014-02-21  9:18                   ` Stanislav Kinsbursky
2014-02-26 13:50 Stanislav Kinsbursky
2014-03-31 20:58 ` J. Bruce Fields

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.