Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 18 Jan 2012 06:14:39 +0200
From:      Kostik Belousov <kostikbel@gmail.com>
To:        Eygene Ryabinkin <rea@freebsd.org>
Cc:        fs@freebsd.org
Subject:   Re: kern/164261: [patch] fix panic with NFS served from NULLFS
Message-ID:  <20120118041439.GU31224@deviant.kiev.zoral.com.ua>
In-Reply-To: <20120117202853.19F65DA81C@void.codelabs.ru>
References:  <20120117202853.19F65DA81C@void.codelabs.ru>

next in thread | previous in thread | raw e-mail | index | archive | help

--5Q4v4o6GEVspILCA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 18, 2012 at 12:28:53AM +0400, Eygene Ryabinkin wrote:
>=20
> >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:       =20
> >Keywords:      =20
> >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:
>=20
> System: FreeBSD 10.0-CURRENT, FreeBSD 9.0-STABLE
>=20
> >Description:
>=20
> When one exports NULLFS filesystems via NFS, he can face kernel
> panics if external clients use readdir+ feature and are accessing
> same directories simultaneously.
>=20
> The example of the backtrace can be obtained at
>   http://codelabs.ru/fbsd/prs/2012-jan-nullfs-LK_SHARED/panic-backtrace.t=
xt
> This backtrace is from 9.x as of December 2011.
>=20
> 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 =3D &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!
>=20
> >How-To-Repeat:
>=20
> See http://codelabs.ru/fbsd/prs/2012-jan-nullfs-LK_SHARED/README.txt
> section "How to reproduce".
>=20
> >Fix:
>=20
> Patches
>   http://codelabs.ru/fbsd/prs/2012-jan-nullfs-LK_SHARED/0001-NULLFS-prope=
rly-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-p=
anics-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 =3D &vp->v_lock;
 		xp->null_lowervp =3D NULL;
-		vrele(vp);
+		vput(vp);
 		return (0);
 	}
 	*vpp =3D vp;

--5Q4v4o6GEVspILCA
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)

iEYEARECAAYFAk8WRy4ACgkQC3+MBN1Mb4h1lgCfWijetcGpDGFZrJR0KAixVLNA
9mgAoIkfRpZUGkyXRAC1yHKlU95Ovkra
=6pWD
-----END PGP SIGNATURE-----

--5Q4v4o6GEVspILCA--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120118041439.GU31224>