p9fs: fix use-after-free when p9fs_vget_common fails

[njain15_protonmail.com]

Summary

Some callers of p9fs_vget_common (ex. p9fs_lookup) clunk the new fid as part of their error cleanup procedure. However, during certain points of the vnode creation, failure already reclaims the node. This causes double clunking of the VFID, which can lead to a panic.

[kib]

sys/fs/p9fs/p9fs_vfsops.c

356

With your change, what would clunk the vfid in this case? Most likely, an unmount is already in the progress.

370

And there?

[njain15_protonmail.com]

sys/fs/p9fs/p9fs_vfsops.c

356

what would clunk the vfid in this case

The caller. All the callers (except p9fs_root, but we don't set its SKIP_VFID flag) clunk fids if p9fs_vget_common returns non-zero error.

Actually, on reading the insmntque1_int() code, I notice something. If a force unmount is set, then we set vp->v_data = NULL. Doesn't this just leak the node? Should we call insmntque1() instead and handle the same way as the p9fs_reload_stats_dotl() case?

370

Yes, I missed the vfs_hash_insert race case. I think it should be:

error = vfs_hash_insert(vp, hash, flags, td, vpp,
	    p9fs_node_cmp, &fid->qid);
	if (error != 0)
		return (error);
	if (*vpp != NULL) {
		p9_client_clunk(fid);
		return (error);
	}

[kib]

sys/fs/p9fs/p9fs_vfsops.c

356

But then, instead of trying to catch all the places where the double-clunking is done, is it possible to make the double-clunking nop instead?

Can you please also explain what is the panic that you see?

[njain15_protonmail.com]

sys/fs/p9fs/p9fs_vfsops.c

356

is it possible to make the double-clunking nop instead

I cannot think of a way to do that.

Image of panic:

[kib]

sys/fs/p9fs/p9fs_vfsops.c

356

And what is the source line for p9_client_request()? Also it would be useful to load the vmcore into kgdb and get the full backtrace together with the locals values from the panic.