kern/164261: [patch] fix panic with NFS served from NULLFS

[Kostik Belousov]

On Wed, Jan 18, 2012 at 12:28:53AM +0400, Eygene Ryabinkin wrote:
> 
> >Number:         164261
> >Category:       kern
> >Synopsis:       [patch] fix panic with NFS served from NULLFS
> >Confidential:   no
> >Severity:       serious
> >Priority:       medium
> >Responsible:    freebsd-bugs
> >State:          open
> >Quarter:        
> >Keywords:       
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Tue Jan 17 20:40:14 UTC 2012
> >Closed-Date:
> >Last-Modified:
> >Originator:     Eygene Ryabinkin
> >Release:        FreeBSD 10.0-CURRENT amd64
> >Organization:
> Code Labs
> >Environment:
> 
> System: FreeBSD 10.0-CURRENT, FreeBSD 9.0-STABLE
> 
> >Description:
> 
> When one exports NULLFS filesystems via NFS, he can face kernel
> panics if external clients use readdir+ feature and are accessing
> same directories simultaneously.
> 
> The example of the backtrace can be obtained at
>   http://codelabs.ru/fbsd/prs/2012-jan-nullfs-LK_SHARED/panic-backtrace.txt
> This backtrace is from 9.x as of December 2011.
> 
> The real problem is that the thread that loses the race in
> null_nodeget (/sys/fs/nullfs/null_subr.c) will put the native lock
> (vp->v_vnlock = &vp->v_lock) to the nullfs vnode that should be
> destroyed (because the thread lost the race).  And null_reclaim
> (/sys/fs/nullfs/null_vnops.c) will try to lock vnode's v_lock in the
> exclusive mode.  This will lead to panic, because v_vnlock is already
> locked at the time of VOP_RECLAIM processing and we have v_vnlock that
> points to v_lock.  Bingo!
> 
> >How-To-Repeat:
> 
> See http://codelabs.ru/fbsd/prs/2012-jan-nullfs-LK_SHARED/README.txt
> section "How to reproduce".
> 
> >Fix:
> 
> Patches
>   http://codelabs.ru/fbsd/prs/2012-jan-nullfs-LK_SHARED/0001-NULLFS-properly-destroy-node-hash.patch
This one is probably fine, assuming that the hashes are properly cleared
on unmount. Feel free to commit.

> and
>   http://codelabs.ru/fbsd/prs/2012-jan-nullfs-LK_SHARED/0002-NULLFS-fix-panics-when-lowervp-is-locked-with-LK_SHA.patch
> will fix the problem (in reality, the first patch is just some
> nitpicking).
And I do not even read this.

The issue that the backtrace is pointing to seems to be the misuse of vrele(),
after the vnode lock is switched to null vnode v_lock. Since the vnode that
is being thrown out is exclusively locked, cleanup path shall do vput()
instead of vrele().

Despite above, vrele(lowervp) call is be fine, despite lowervp also
being locked exclusively, because usecount for the vnode must be > 1,
due to null_hashins() successfully found the ovp in the hash.

Try the change below.

diff --git a/sys/fs/nullfs/null_subr.c b/sys/fs/nullfs/null_subr.c
index 319e404..6ba7508 100644
--- a/sys/fs/nullfs/null_subr.c
+++ b/sys/fs/nullfs/null_subr.c
@@ -252,7 +252,7 @@ null_nodeget(mp, lowervp, vpp)
 		vrele(lowervp);
 		vp->v_vnlock = &vp->v_lock;
 		xp->null_lowervp = NULL;
-		vrele(vp);
+		vput(vp);
 		return (0);
 	}
 	*vpp = vp;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20120118/ae55a2ac/attachment.pgp