Page MenuHomeFreeBSD

p9fs: fix panic in freevnode when vfs_hash_insert finds a duplicate
AcceptedPublic

Authored by delphij on Mon, Mar 9, 5:04 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 13, 9:24 PM
Unknown Object (File)
Fri, Mar 13, 8:07 PM
Unknown Object (File)
Fri, Mar 13, 1:55 PM
Unknown Object (File)
Wed, Mar 11, 3:39 AM
Subscribers

Details

Reviewers
markj
kib
khng
Summary

When two threads concurrently look up the same path, both may call
p9fs_vget_common() and allocate a new vnode/node pair. Whichever
thread loses the vfs_hash_insert() race has vgone() called on its vp
by vfs_hash_insert(), triggering VOP_RECLAIM -> p9fs_reclaim() ->
p9fs_cleanup().

The bug: p9fs_cleanup() returned early when P9FS_NODE_IN_SESSION was
not set (the node is never added to virt_node_list until *after* a
successful hash insert), leaving vp->v_data non-NULL. The subsequent
vput() inside vfs_hash_insert() then called freevnode(), which
VNASSERT()s that vp->v_data == NULL, causing a panic.

Fix p9fs_cleanup() to always call p9fs_fid_remove_all() and
p9fs_destroy_node() regardless of P9FS_NODE_IN_SESSION status.
cache_purge() and vnode_destroy_vobject() remain conditional on the
node having been fully active (in session), as they are unnecessary
for a node that was never visible.

Fix p9fs_vget_common() to not attempt to free np after vfs_hash_insert()
has already triggered p9fs_reclaim() and freed it. Set np = NULL in
both the duplicate-found-no-error and duplicate-found-with-error paths,
and guard the out: label cleanup with np != NULL to prevent double-free.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 71277
Build 68160: arc lint + arc unit

Event Timeline

Looks ok to me.

sys/fs/p9fs/p9fs_vfsops.c
390

What's the purpose of this assignment?

sys/fs/p9fs/p9fs_vnops.c
126
This revision is now accepted and ready to land.Tue, Mar 10, 1:43 AM
sys/fs/p9fs/p9fs_vfsops.c
356

Isn't the same problem exist there as well?

375

Why not simply return (error) there?