From 90f095da8d158ff6fdf8161818b383cf8676f894 Mon Sep 17 00:00:00 2001
From: Augustin Cavalier <waddlesplash@gmail.com>
Date: Wed, 28 May 2025 16:13:57 -0400
Subject: [PATCH] ramfs: Handle renames replacing existing files properly.

Previously, Entry::Link() behaved incorrectly in this case:
it would unlink the old Node before linking the new one.
But there's only one DoublyLinkedListLink inside Entry
for the Node to use, so this would clobber the lists
and thus produce KDLs.

Instead, make Link fail if there's already a node, and
thus force the caller to Unlink first. For now, just use
a "naive" implementation of this in the one case in
rename(); in the future we could make it more robust if
necessary.

Fixes the other KDL in #19583.
---
 .../kernel/file_systems/ramfs/Entry.cpp       | 29 ++++++++++---------
 src/add-ons/kernel/file_systems/ramfs/Entry.h |  4 +--
 .../file_systems/ramfs/kernel_interface.cpp   |  5 ++--
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/add-ons/kernel/file_systems/ramfs/Entry.cpp b/src/add-ons/kernel/file_systems/ramfs/Entry.cpp
index f0877a7163..1a8fc79ba2 100644
--- a/src/add-ons/kernel/file_systems/ramfs/Entry.cpp
+++ b/src/add-ons/kernel/file_systems/ramfs/Entry.cpp
@@ -36,7 +36,7 @@ Entry::InitCheck() const
 	return (fName.GetString() ? B_OK : B_NO_INIT);
 }
 
-// Link
+
 status_t
 Entry::Link(Node *node)
 {
@@ -45,25 +45,28 @@ Entry::Link(Node *node)
 	if (node == fNode)
 		return B_OK;
 
-	// We first link to the new node and then unlink the old one.
-	Node *oldNode = fNode;
-	status_t error = node->Link(this);
-	if (error == B_OK) {
+	// We can only be linked to one Node at a time, so force callers
+	// to decide what to do when we're already linked to a Node.
+	if (fNode != NULL)
+		return B_BAD_VALUE;
+
+	status_t status = node->Link(this);
+	if (status == B_OK)
 		fNode = node;
-		if (oldNode)
-			oldNode->Unlink(this);
-	}
-	return error;
+	return status;
 }
 
-// Unlink
+
 status_t
 Entry::Unlink()
 {
-	status_t error = (fNode ? B_OK : B_BAD_VALUE);
-	if (error == B_OK && (error = fNode->Unlink(this)) == B_OK)
+	if (fNode == NULL)
+		return B_BAD_VALUE;
+
+	status_t status = fNode->Unlink(this);
+	if (status == B_OK)
 		fNode = NULL;
-	return error;
+	return status;
 }
 
 // SetName
diff --git a/src/add-ons/kernel/file_systems/ramfs/Entry.h b/src/add-ons/kernel/file_systems/ramfs/Entry.h
index 6fb55517e0..c7c8d75873 100644
--- a/src/add-ons/kernel/file_systems/ramfs/Entry.h
+++ b/src/add-ons/kernel/file_systems/ramfs/Entry.h
@@ -29,7 +29,6 @@ public:
 	inline void SetParent(Directory *parent)	{ fParent = parent; }
 	Directory *GetParent() const				{ return fParent; }
 
-//	inline void SetNode(Node *node)				{ fNode = node; }
 	status_t Link(Node *node);
 	status_t Unlink();
 	Node *GetNode() const						{ return fNode; }
@@ -37,8 +36,6 @@ public:
 	status_t SetName(const char *newName);
 	inline const char *GetName() const			{ return fName.GetString(); }
 
-//	inline Volume *GetVolume() const			{ return fVolume; }
-
 	inline DoublyLinkedListLink<Entry> *GetReferrerLink()
 		{ return &fReferrerLink; }
 
@@ -57,6 +54,7 @@ private:
 	Node					*fNode;
 	String					fName;
 	DoublyLinkedListLink<Entry>		fReferrerLink;
+
 	// iterator management
 	DoublyLinkedList<EntryIterator>	fIterators;
 };
diff --git a/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp
index 57ee444862..29c85553c5 100644
--- a/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp
+++ b/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp
@@ -622,9 +622,10 @@ ramfs_rename(fs_volume* _volume, fs_vnode* _oldDir, const char *oldName,
 				error = oldDir->DeleteEntry(entry);
 				if (error == B_OK) {
 					// create the new one/relink the target entry
-					if (clobberEntry)
+					if (clobberEntry != NULL) {
+						clobberEntry->Unlink();
 						error = clobberEntry->Link(node);
-					else
+					} else
 						error = newDir->CreateEntry(node, newName);
 
 					if (error == B_OK) {