[PATCH 6.6.y v2 0/2] Fix `fremovexattr` missing `fdput`

[PATCH 6.6.y v2 0/2] Fix `fremovexattr` missing `fdput`

[Tomasz Kramkowski]

As discussed, a v2 which includes the revert from the previous version
[0] and a new attempt at backporiting the upstream change which doesn't
cause the regression introduced in the first attempt[1].

In total, this fixes the missing `fdput` in the `fremovexattr`
`copy_from_user` error path that the backport was intended for.

I tested both the error case and the happy case in qemu.

[0]: https://lore.kernel.org/stable/20260404112219.389495-1-tomasz@kramkow.ski/
[1]: https://lore.kernel.org/stable/tencent_72B5370E2D4C4AC319ED4F0DCB479CA4B406@qq.com/

Al Viro (1):
  xattr: switch to CLASS(fd)

Tomasz Kramkowski (1):
  Revert "xattr: switch to CLASS(fd)"

 fs/xattr.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

-- 
2.51.0

[PATCH 6.6.y v2 1/2] 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>
---
 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

[PATCH 6.6.y v2 2/2] xattr: switch to CLASS(fd)

[Tomasz Kramkowski]

From: Al Viro <viro@zeniv.linux.org.uk>

[ Upstream commit a71874379ec8c6e788a61d71b3ad014a8d9a5c08 ]

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Link: https://lore.kernel.org/all/20241002012230.4174585-1-viro@zeniv.linux.org.uk/
[ Neither `fd_file` nor `fd_empty` are available in 6.6.y, so the
  changes to the check are dropped. Kept the minor formatting change. ]
Signed-off-by: Tomasz Kramkowski <tomasz@kramkow.ski>
---
 fs/xattr.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 7574d24b982e..20a038b06d12 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -698,9 +698,9 @@ 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);
 	if (error)
@@ -810,16 +810,13 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
-	struct fd f = fdget(fd);
-	ssize_t error = -EBADF;
+	CLASS(fd, f)(fd);
 
 	if (!f.file)
-		return error;
+		return -EBADF;
 	audit_file(f.file);
-	error = getxattr(file_mnt_idmap(f.file), f.file->f_path.dentry,
+	return getxattr(file_mnt_idmap(f.file), f.file->f_path.dentry,
 			 name, value, size);
-	fdput(f);
-	return error;
 }
 
 /*
@@ -886,15 +883,12 @@ SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
 
 SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 {
-	struct fd f = fdget(fd);
-	ssize_t error = -EBADF;
+	CLASS(fd, f)(fd);
 
 	if (!f.file)
-		return error;
+		return -EBADF;
 	audit_file(f.file);
-	error = listxattr(f.file->f_path.dentry, list, size);
-	fdput(f);
-	return error;
+	return  listxattr(f.file->f_path.dentry, list, size);
 }
 
 /*
@@ -951,12 +945,12 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
-	struct fd f = fdget(fd);
+	CLASS(fd, f)(fd);
 	char kname[XATTR_NAME_MAX + 1];
-	int error = -EBADF;
+	int error;
 
 	if (!f.file)
-		return error;
+		return -EBADF;
 	audit_file(f.file);
 
 	error = strncpy_from_user(kname, name, sizeof(kname));
@@ -971,7 +965,6 @@ 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 6.6.y v2 0/2] Fix `fremovexattr` missing `fdput`

[Barry K. Nathan]

On 4/5/26 04:45, Tomasz Kramkowski wrote:
> As discussed, a v2 which includes the revert from the previous version
> [0] and a new attempt at backporiting the upstream change which doesn't
> cause the regression introduced in the first attempt[1].
> 
> In total, this fixes the missing `fdput` in the `fremovexattr`
> `copy_from_user` error path that the backport was intended for.
> 
> I tested both the error case and the happy case in qemu.
> 
> [0]: https://lore.kernel.org/stable/20260404112219.389495-1-tomasz@kramkow.ski/
> [1]: https://lore.kernel.org/stable/tencent_72B5370E2D4C4AC319ED4F0DCB479CA4B406@qq.com/
> 
> Al Viro (1):
>    xattr: switch to CLASS(fd)
> 
> Tomasz Kramkowski (1):
>    Revert "xattr: switch to CLASS(fd)"
> 
>   fs/xattr.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 

I tested the following two (groups of) proof-of-concept exploits
against 6.6.130, 6.6.132, and 6.6.132 + this patch series:


1. "CVE-2024-14027 - SlopSploit" proof-of-concept exploit for the bug
fixed by the original mainline commit. This only works on i386 kernels,
so I tested with i386 kernels on amd64 hardware.

https://github.com/lcfr-eth/CVE-2024-14027_slop

(I used exploit.c. For me, the exploit never reached its intended goal
of allowing a normal user to read /etc/shadow, but as far as I can tell
it still causes a parade of oopses on vulnerable i386 kernels but no
oopses on invulnerable i386 kernels. So it's still a good test of whether
this patch series works.)


2. Brad Spengler's proof-of-concept exploits for the 6.6.132 regression,
posted on Twitter (I tested on i386 and amd64 kernels, on amd64 hardware):

https://x.com/spendergrsec/status/2040049852793450561

(Note that one of these has a missing parameter, but it's easy enough
to fix.)


Test results:
6.6.130: #1 causes oopses (but not #2)
6.6.132: #2 causes oopses (but not #1)
6.6.132 + this patch series: Neither #1 nor #2 cause oopses

So, at least in my testing, this patch series successfully fixes both
the old and new bugs (both CVE-2024-14027 and the 6.6.132 regression).

Tested-by: Barry K. Nathan <barryn@pobox.com>

-- 
-Barry K. Nathan  <barryn@pobox.com>

Re: [PATCH 6.6.y v2 0/2] Fix `fremovexattr` missing `fdput`

[Greg Kroah-Hartman]

On Sun, Apr 05, 2026 at 10:05:35AM -0700, Barry K. Nathan wrote:
> On 4/5/26 04:45, Tomasz Kramkowski wrote:
> > As discussed, a v2 which includes the revert from the previous version
> > [0] and a new attempt at backporiting the upstream change which doesn't
> > cause the regression introduced in the first attempt[1].
> > 
> > In total, this fixes the missing `fdput` in the `fremovexattr`
> > `copy_from_user` error path that the backport was intended for.
> > 
> > I tested both the error case and the happy case in qemu.
> > 
> > [0]: https://lore.kernel.org/stable/20260404112219.389495-1-tomasz@kramkow.ski/
> > [1]: https://lore.kernel.org/stable/tencent_72B5370E2D4C4AC319ED4F0DCB479CA4B406@qq.com/
> > 
> > Al Viro (1):
> >    xattr: switch to CLASS(fd)
> > 
> > Tomasz Kramkowski (1):
> >    Revert "xattr: switch to CLASS(fd)"
> > 
> >   fs/xattr.c | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> 
> I tested the following two (groups of) proof-of-concept exploits
> against 6.6.130, 6.6.132, and 6.6.132 + this patch series:
> 
> 
> 1. "CVE-2024-14027 - SlopSploit" proof-of-concept exploit for the bug
> fixed by the original mainline commit. This only works on i386 kernels,
> so I tested with i386 kernels on amd64 hardware.
> 
> https://github.com/lcfr-eth/CVE-2024-14027_slop
> 
> (I used exploit.c. For me, the exploit never reached its intended goal
> of allowing a normal user to read /etc/shadow, but as far as I can tell
> it still causes a parade of oopses on vulnerable i386 kernels but no
> oopses on invulnerable i386 kernels. So it's still a good test of whether
> this patch series works.)
> 
> 
> 2. Brad Spengler's proof-of-concept exploits for the 6.6.132 regression,
> posted on Twitter (I tested on i386 and amd64 kernels, on amd64 hardware):
> 
> https://x.com/spendergrsec/status/2040049852793450561
> 
> (Note that one of these has a missing parameter, but it's easy enough
> to fix.)
> 
> 
> Test results:
> 6.6.130: #1 causes oopses (but not #2)
> 6.6.132: #2 causes oopses (but not #1)
> 6.6.132 + this patch series: Neither #1 nor #2 cause oopses
> 
> So, at least in my testing, this patch series successfully fixes both
> the old and new bugs (both CVE-2024-14027 and the 6.6.132 regression).
> 
> Tested-by: Barry K. Nathan <barryn@pobox.com>
> 

Thanks for the testing, and thanks Tomasz for the revert and the
backport, I'll go do a release right now with these in it as this is
pretty big.

greg k-h