From 6b86bc6ea3c5e68177159fd963762638267fdea8 Mon Sep 17 00:00:00 2001
From: Augustin Cavalier <waddlesplash@gmail.com>
Date: Fri, 29 Aug 2025 15:01:57 -0400
Subject: [PATCH] kernel/fs: Handle root vnode ownership correctly and add more
 assertions.

In 5b14757a30be0b805e2688cdc5341177f2d703fb (2005), a change was made
to have root nodes be owned by filesystems, rather than
by the VFS (as BeOS did.) However, later on,
in fe5928847a347b3a46eea68628388e60796e71db (2011),
the adding of covered vnodes broke that.

As put_vnode's error was silently ignored, this went unnoticed
until this week, when the new asserts started firing.

So, to fix this, increment the reference count of the root when
setting its covering vnode, and handle the root specially.
Assert in ~fs_mount that there are no vnodes left; if the
filesystem failed to free it, this assert will trip.

Fixes the new assert seen in #19642.
---
 src/system/kernel/fs/vfs.cpp | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp
index 430e292db4..54731b6b0e 100644
--- a/src/system/kernel/fs/vfs.cpp
+++ b/src/system/kernel/fs/vfs.cpp
@@ -144,6 +144,8 @@ struct fs_mount {
 
 	~fs_mount()
 	{
+		ASSERT(vnodes.IsEmpty());
+
 		mutex_destroy(&lock);
 		free(device_name);
 
@@ -1069,6 +1071,9 @@ dec_vnode_ref_count(struct vnode* vnode, bool alwaysFree, bool reenter)
 	if (vnode->IsBusy())
 		panic("dec_vnode_ref_count: called on busy vnode %p\n", vnode);
 
+	if (vnode->mount->unmounting)
+		alwaysFree = true;
+
 	bool freeNode = false;
 	bool freeUnusedNodes = false;
 
@@ -1359,7 +1364,7 @@ free_unused_vnodes(int32 level)
 		if (vnode != sUnusedVnodeList.First())
 			continue;
 
-		ASSERT(!vnode->IsBusy());
+		ASSERT(!vnode->IsBusy() && vnode->ref_count == 0);
 
 		// grab a reference
 		inc_vnode_ref_count(vnode);
@@ -7732,6 +7737,7 @@ fs_mount(char* path, const char* device, const char* fsName, uint32 flags,
 
 		coveredNode->covered_by = mount->root_vnode;
 		coveredNode->SetCovered(true);
+		inc_vnode_ref_count(mount->root_vnode);
 	}
 	rw_lock_write_unlock(&sVnodeLock);
 
@@ -7864,6 +7870,8 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
 				refCount--;
 			if (vnode->covered_by != NULL)
 				refCount--;
+			if (vnode == mount->root_vnode)
+				refCount--;
 
 			if (refCount != 0) {
 				dprintf("fs_unmount(): inode %" B_PRIdINO " is still referenced\n", vnode->id);
@@ -7955,6 +7963,9 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
 			}
 		}
 
+		if (vnode == mount->root_vnode)
+			continue;
+
 		vnode->SetBusy(true);
 		vnode_to_be_freed(vnode);
 	}
@@ -7962,8 +7973,6 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
 	vnodesWriteLocker.Unlock();
 
 	// Free all vnodes associated with this mount.
-	// They will be removed from the mount list by free_vnode(), so
-	// we don't have to do this.
 	while (struct vnode* vnode = mount->vnodes.Head()) {
 		// Put the references to external covered/covering vnodes we kept above.
 		if (Vnode* coveredNode = vnode->covers)
@@ -7971,9 +7980,18 @@ fs_unmount(char* path, dev_t mountID, uint32 flags, bool kernel)
 		if (Vnode* coveringNode = vnode->covered_by)
 			put_vnode(coveringNode);
 
-		free_vnode(vnode, false);
+		// free_vnode() removes nodes from the mount list. However, the root
+		// will still be referenced by the FS, so we can't free it yet.
+		if (vnode == mount->root_vnode)
+			remove_vnode_from_mount_list(vnode, mount);
+		else
+			free_vnode(vnode, false);
 	}
 
+	// Re-add the root to the mount list, so it can be freed.
+	add_vnode_to_mount_list(mount->root_vnode, mount);
+	mount->root_vnode = NULL;
+
 	// remove the mount structure from the hash table
 	rw_lock_write_lock(&sMountLock);
 	sMountsTable->Remove(mount);