[PATCH] Revert "xattr: switch to CLASS(fd)"

[PATCH] Revert "xattr: switch to CLASS(fd)"

[Tomasz Kramkowski]

This reverts commit 5a1e865e51063d6c56f673ec8ad4b6604321b455 which is
commit a71874379ec8c6e788a61d71b3ad014a8d9a5c08 upstream.

A backporting mistake erroneously removed file descriptor checks for
`fgetxattr`, `flistxattr`, `fremovexattr`, and `fsetxattr` which lead to
kernel panics when those functions were called from userspace with a
file descriptor which did not reference an open file.

Reported-by: Brad Spengler <spender@grsecurity.net>
Closes: https://x.com/spendergrsec/status/2040049852793450561
Cc: Alva Lan <alvalan9@foxmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Tomasz Kramkowski <tomasz@kramkow.ski>
---
Was asked to send a revert instead of a fix. Previous patch was here:
https://lore.kernel.org/stable/20260403230636.344097-1-tomasz@kramkow.ski/

Tested via qemu to verify the fix and ensure there were no unexpected
consequences.

I was made aware of this after being shown a screenshot from Brad
Spengler's twitter feed. I looked around for other reverts and tried to
match the trailers.

 fs/xattr.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 5f2d74332ea6..7574d24b982e 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -698,6 +698,8 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 	int error;
 
 	CLASS(fd, f)(fd);
+	if (!f.file)
+		return -EBADF;
 
 	audit_file(f.file);
 	error = setxattr_copy(name, &ctx);
@@ -808,11 +810,16 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
-	CLASS(fd, f)(fd);
+	struct fd f = fdget(fd);
+	ssize_t error = -EBADF;
 
+	if (!f.file)
+		return error;
 	audit_file(f.file);
-	return getxattr(file_mnt_idmap(f.file), f.file->f_path.dentry,
+	error = getxattr(file_mnt_idmap(f.file), f.file->f_path.dentry,
 			 name, value, size);
+	fdput(f);
+	return error;
 }
 
 /*
@@ -879,10 +886,15 @@ SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
 
 SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 {
-	CLASS(fd, f)(fd);
+	struct fd f = fdget(fd);
+	ssize_t error = -EBADF;
 
+	if (!f.file)
+		return error;
 	audit_file(f.file);
-	return listxattr(f.file->f_path.dentry, list, size);
+	error = listxattr(f.file->f_path.dentry, list, size);
+	fdput(f);
+	return error;
 }
 
 /*
@@ -939,10 +951,12 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
-	CLASS(fd, f)(fd);
+	struct fd f = fdget(fd);
 	char kname[XATTR_NAME_MAX + 1];
-	int error;
+	int error = -EBADF;
 
+	if (!f.file)
+		return error;
 	audit_file(f.file);
 
 	error = strncpy_from_user(kname, name, sizeof(kname));
@@ -957,6 +971,7 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 				    f.file->f_path.dentry, kname);
 		mnt_drop_write_file(f.file);
 	}
+	fdput(f);
 	return error;
 }
 
-- 
2.51.0

Re: [PATCH] Revert "xattr: switch to CLASS(fd)"

[Tomasz Kramkowski]

On Sat Apr 4, 2026 at 12:22 PM BST, Tomasz Kramkowski wrote:
> Was asked to send a revert instead of a fix. Previous patch was here:
> https://lore.kernel.org/stable/20260403230636.344097-1-tomasz@kramkow.ski/
>
> Tested via qemu to verify the fix and ensure there were no unexpected
> consequences.

I should note, however, the backport was intended to fix a specific bug
in `fremovexattr`, and now that bug is there after the revert.

Shall I just submit a v2 of this with the revert _and_ a new backport?
Or would you still prefer to just revert and then have attempt #2 at the
backport separately?

-- 
Tomasz Kramkowski

Re: [PATCH] Revert "xattr: switch to CLASS(fd)"

[Greg Kroah-Hartman]

On Sat, Apr 04, 2026 at 01:11:57PM +0100, Tomasz Kramkowski wrote:
> On Sat Apr 4, 2026 at 12:22 PM BST, Tomasz Kramkowski wrote:
> > Was asked to send a revert instead of a fix. Previous patch was here:
> > https://lore.kernel.org/stable/20260403230636.344097-1-tomasz@kramkow.ski/
> >
> > Tested via qemu to verify the fix and ensure there were no unexpected
> > consequences.
> 
> I should note, however, the backport was intended to fix a specific bug
> in `fremovexattr`, and now that bug is there after the revert.
> 
> Shall I just submit a v2 of this with the revert _and_ a new backport?
> Or would you still prefer to just revert and then have attempt #2 at the
> backport separately?

A new backport would be great, as the original was broken, and this
makes it more obvious where things were actually fixed.

thanks,

greg k-h