linux-kernel - Re: [3.10.4] NFS locking panic, plus persisting NFS shutdown panic from 3.9.*

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1375755693.7337.42.camel@leira.trondhjem.org>
Date:	Tue, 6 Aug 2013 02:21:35 +0000
From:	"Myklebust, Trond" <Trond.Myklebust@...app.com>
To:	Jeff Layton <jlayton@...hat.com>
CC:	Nix <nix@...eri.org.uk>,
	Toralf Förster <toralf.foerster@....de>,
	Oleg Nesterov <oleg@...hat.com>,
	NFS list <linux-nfs@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"dhowells@...hat.com" <dhowells@...hat.com>
Subject: Re: [3.10.4] NFS locking panic, plus persisting NFS shutdown panic
 from 3.9.*

On Mon, 2013-08-05 at 14:33 -0400, Jeff Layton wrote:
> On Mon, 5 Aug 2013 18:18:03 +0000
> "Myklebust, Trond" <Trond.Myklebust@...app.com> wrote:
> 
> > On Mon, 2013-08-05 at 13:37 -0400, Jeff Layton wrote:
> > > On Mon, 5 Aug 2013 16:15:01 +0000
> > > "Myklebust, Trond" <Trond.Myklebust@...app.com> wrote:
> > > 
> > > > From 3c50ba80105464a28d456d9a1e0f1d81d4af92a8 Mon Sep 17 00:00:00 2001
> > > > From: Trond Myklebust <Trond.Myklebust@...app.com>
> > > > Date: Mon, 5 Aug 2013 12:06:12 -0400
> > > > Subject: [PATCH] LOCKD: Don't call utsname()->nodename from
> > > >  nlmclnt_setlockargs
> > > > MIME-Version: 1.0
> > > > Content-Type: text/plain; charset=UTF-8
> > > > Content-Transfer-Encoding: 8bit
> > > > 
> > > > Firstly, nlmclnt_setlockargs can be called from a reclaimer thread, in
> > > > which case we're in entirely the wrong namespace.
> > > > Secondly, commit 8aac62706adaaf0fab02c4327761561c8bda9448 (move
> > > > exit_task_namespaces() outside of exit_notify()) now means that
> > > > exit_task_work() is called after exit_task_namespaces(), which
> > > > triggers an Oops when we're freeing up the locks.
> > > > 
> > > > Signed-off-by: Trond Myklebust <Trond.Myklebust@...app.com>
> > > > Cc: Toralf Förster <toralf.foerster@....de>
> > > > Cc: Oleg Nesterov <oleg@...hat.com>
> > > > Cc: Nix <nix@...eri.org.uk>
> > > > Cc: Jeff Layton <jlayton@...hat.com>
> > > > ---
> > > >  fs/lockd/clntproc.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/fs/lockd/clntproc.c b/fs/lockd/clntproc.c
> > > > index 9760ecb..acd3947 100644
> > > > --- a/fs/lockd/clntproc.c
> > > > +++ b/fs/lockd/clntproc.c
> > > > @@ -125,14 +125,15 @@ static void nlmclnt_setlockargs(struct nlm_rqst *req, struct file_lock *fl)
> > > >  {
> > > >  	struct nlm_args	*argp = &req->a_args;
> > > >  	struct nlm_lock	*lock = &argp->lock;
> > > > +	char *nodename = req->a_host->h_rpcclnt->cl_nodename;
> > > >  
> > > >  	nlmclnt_next_cookie(&argp->cookie);
> > > >  	memcpy(&lock->fh, NFS_FH(file_inode(fl->fl_file)), sizeof(struct nfs_fh));
> > > > -	lock->caller  = utsname()->nodename;
> > > > +	lock->caller  = nodename;
> > > >  	lock->oh.data = req->a_owner;
> > > >  	lock->oh.len  = snprintf(req->a_owner, sizeof(req->a_owner), "%u@%s",
> > > >  				(unsigned int)fl->fl_u.nfs_fl.owner->pid,
> > > > -				utsname()->nodename);
> > > > +				nodename);
> > > >  	lock->svid = fl->fl_u.nfs_fl.owner->pid;
> > > >  	lock->fl.fl_start = fl->fl_start;
> > > >  	lock->fl.fl_end = fl->fl_end;
> > > 
> > > Looks good to me...
> > > 
> > > Reviewed-by: Jeff Layton <jlayton@...hat.com>
> > > 
> > > Trond, any thoughts on the other oops that Nix posted? The issue there
> > > seems to be that we're trying to do the pathwalk to the rpcbind unix
> > > socket from exit_task_work(), but that's happening after we've already
> > > called exit_fs().
> > > 
> > > The trivial answer seems to be to simply call exit_task_work() before
> > > exit_fs() there, but it seems like we ought to be doing the upcall to
> > > rpcbind in a mount namespace from which we know we can reach the
> > > socket...
> > 
> > Isn't it enough to just do the same thing as we did for gss proxy? i.e.
> > set the RPC_CLNT_CREATE_NO_IDLE_TIMEOUT flag.
> > 
> > See attachment.
> 
> Yeah, that looks like a reasonable thing to do...
> 
> OTOH, Is there any other way for a unix socket to end up disconnected
> other than if we were to close it? Maybe if rpcbind stopped, the socket
> unlinked and recreated and then started again?
> 
> If so then you still could potentially end up in this situation even if
> you didn't autoclose it.

True. How about something like the following instead. Note the change to
the original patch...
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@...app.com
www.netapp.com

View attachment "0001-SUNRPC-Don-t-auto-disconnect-from-the-local-rpcbind-.patch" of type "text/x-patch" (1427 bytes)

View attachment "0002-SUNRPC-If-the-rpcbind-channel-is-disconnected-fail-t.patch" of type "text/x-patch" (6880 bytes)