Page MenuHomeFreeBSD

Various fixes to unionfs error handling logic
ClosedPublic

Authored by jah on Apr 23 2023, 3:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
May 1 2024, 12:32 PM
Unknown Object (File)
Apr 26 2024, 6:03 AM
Unknown Object (File)
Apr 26 2024, 5:59 AM
Unknown Object (File)
Apr 26 2024, 5:59 AM
Unknown Object (File)
Apr 25 2024, 9:52 PM
Unknown Object (File)
Dec 20 2023, 7:21 AM
Unknown Object (File)
Aug 20 2023, 1:48 AM
Unknown Object (File)
Aug 14 2023, 8:05 PM
Subscribers

Details

Summary

unionfs: fixes to unionfs_nodeget() error handling

If either the lower or upper vnode is found to be doomed after
locking it, the newly-created unionfs node won't be associated
with it and its lock will be dropped. In that case, clear the
uppervp and lowervp locals as necessary to avoid further use
of the vnode in unionfs_nodeget(). If the upper vnode is doomed
but the lower vnode remains valid, additionally reset the unionfs
node's v_vnlock field to point to the lower vnode lock.

unionfs: prevent upperrootvp from being recycled during mount

If upperrootvp is doomed by a concurrent unmount, unionfs_nodeget()
may return without a reference or lock on it. unionfs_domount() must
prevent the vnode from being recycled for use by a different file until
it is finished with the vnode, namely once vfs_register_upper_from_vp()
fails. Accomplish this by holding the reference returned by namei()
a bit longer.

unionfs(): destroy root vnode if upper registration fails

If unionfs_domount() fails, the mount path will not call VFS_UNMOUNT()
to clean up after it. If this failure happens during upper vnode
registration, the unionfs root vnode will already be allocated.
vflush() it in order to prevent the vnode from being leaked and the
subsequent vfs_mount_destroy() call from getting stuck waiting for
the mountpoint reference count to drain.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jah requested review of this revision.Apr 23 2023, 3:10 AM
sys/fs/unionfs/union_vfsops.c
311 ↗(On Diff #120881)

Then wouldn't it be better to not try to register upper for uppervp at all, if lowermp is NULL?

sys/fs/unionfs/union_vfsops.c
311 ↗(On Diff #120881)

Personal style I suppose. I slightly prefer the cleaner logic to unconditionally register both uppers, since the cleanup logic below this will need to be the same no matter what.

This revision is now accepted and ready to land.Apr 24 2023, 1:15 AM
markj added inline comments.
sys/fs/unionfs/union_subr.c
405

Is there any reason to use NULLVP instead of plain NULL?

I ran the unionfs tests for a day and a half with the D39767.120881.patch. I did not see any problems.