linux-kernel - [PATCH] NFS: ensure nfs_safe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <qqu6ndrq6ytkt7rfe7hw62iu34fkt6eckixjgx7bkhqgvzvcm6@h4tj3bkvvidi>
Date: Mon, 17 Nov 2025 13:03:29 -0500
From: Aiden Lambert <alambert48@...ech.edu>
To: trondmy@...nel.org, anna@...nel.org, chuck.lever@...cle.org, 
	jlayton@...nel.org
Cc: linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] NFS: ensure nfs_safe_remove() atomic nlink drop

A race condition occurs when both unlink() and link() are running
concurrently on the same inode, and the nlink count from the nfs server
received in link()->nfs_do_access() clobbers the nlink count of the
inode in nfs_safe_remove() after the "remove" RPC is made to the server
but before we decrement the link count. If the nlink value from
nfs_do_access() reflects the decremented nlink of the "remove" RPC, a
double decrement occurs, which can lead to the dropping of the client
side inode, causing the link call to return ENOENT. To fix this, we
record an expected nlink value before the "remove" RPC and compare it
with the value afterwards---if these two are the same, the drop is
performed. Note that this does not take into account nlink values that
are a result of multi-client (un)link operations as these are not
guaranteed to be atomic by the NFS spec.

Signed-off-by: Aiden Lambert <alambert48@...ech.edu>
---
 fs/nfs/dir.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 46d9c65d50f..965787a8eee 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1892,19 +1892,24 @@ static int nfs_dentry_delete(const struct dentry *dentry)
 	return 0;
 
 }
 
-/* Ensure that we revalidate inode->i_nlink */
-static void nfs_drop_nlink(struct inode *inode)
+static void nfs_drop_nlink_locked(struct inode *inode)
 {
-	spin_lock(&inode->i_lock);
 	/* drop the inode if we're reasonably sure this is the last link */
 	if (inode->i_nlink > 0)
 		drop_nlink(inode);
 	NFS_I(inode)->attr_gencount = nfs_inc_attr_generation_counter();
 	nfs_set_cache_invalid(
 		inode, NFS_INO_INVALID_CHANGE | NFS_INO_INVALID_CTIME |
 			       NFS_INO_INVALID_NLINK);
+}
+
+/* Ensure that we revalidate inode->i_nlink */
+static void nfs_drop_nlink(struct inode *inode)
+{
+	spin_lock(&inode->i_lock);
+	nfs_drop_nlink_locked(inode);
 	spin_unlock(&inode->i_lock);
 }
 
 /*
@@ -2505,11 +2510,18 @@ static int nfs_safe_remove(struct dentry *dentry)
 	}
 
 	trace_nfs_remove_enter(dir, dentry);
 	if (inode != NULL) {
+		spin_lock(&inode->i_lock);
+		unsigned int expected_nlink = inode->i_nlink;
+
+		spin_unlock(&inode->i_lock);
+
 		error = NFS_PROTO(dir)->remove(dir, dentry);
-		if (error == 0)
-			nfs_drop_nlink(inode);
+
+		spin_lock(&inode->i_lock);
+		if (error == 0 && expected_nlink == inode->i_nlink)
+			nfs_drop_nlink_locked(inode);
+		spin_unlock(&inode->i_lock);
 	} else
 		error = NFS_PROTO(dir)->remove(dir, dentry);
 	if (error == -ENOENT)
 		nfs_dentry_handle_enoent(dentry);
-- 
2.51.1