Page MenuHomeFreeBSD

NDFREE(): Fix unlocking for LOCKPARENT|LOCKLEAF and ndp->ni_dvp == ndp->ni_vp
ClosedPublic

Authored by kib on May 18 2019, 5:57 PM.

Details

Summary

NDFREE() calculates unlock_dvp after ndp->ni_vp is unlocked and zeroed out. This makes the comparision of ni_dvp with ni_vp always fail.

Move the calculation of unlock_dvp right after unlock_vp, so that the code sees correct ni_vp value.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

kib created this revision.May 18 2019, 5:57 PM
markj added a comment.May 18 2019, 6:04 PM

So if ni_dvp == ni_vp, does the caller hold two references on vp or one? It was not clear to me from some quick reading of the code.

kib added a comment.May 18 2019, 6:15 PM

So if ni_dvp == ni_vp, does the caller hold two references on vp or one? It was not clear to me from some quick reading of the code.

dvp == vp means that this was either dot lookup, or dotdot with vp == rootvp. For dotdot case, lookup() itself does the following:

				ndp->ni_dvp = dp;
				ndp->ni_vp = dp;
				VREF(dp);

for the root directory case (also chroot, jail pseudo-roots).

For UFS, ufs_lookup() takes a reference on the dvp directly in case of dot, see ufs_lookup().

So vp holds two references, but In either case, the code does not recurse on lock.

markj accepted this revision.May 18 2019, 9:51 PM
This revision is now accepted and ready to land.May 18 2019, 9:51 PM
mckusick accepted this revision.May 20 2019, 9:44 PM

This change looks correct to me.

pho added a comment.May 21 2019, 11:54 AM

I reproduced the problem and verified that the patch fixes it.
I ran all of the stress2 tests on both amd64 and i386.
No problems seen.

This revision was automatically updated to reflect the committed changes.