[PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update

[PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update

[Ahmet Eray Karadag]

syzkaller found a path where ext4_xattr_inode_update_ref() reads an EA
inode refcount that is already <= 0 and then applies ref_change (often
-1). That lets the refcount underflow and we proceed with a bogus value,
triggering errors like:

  EXT4-fs error: EA inode <n> ref underflow: ref_count=-1 ref_change=-1
  EXT4-fs warning: ea_inode dec ref err=-117

Make the invariant explicit: if the current refcount is non-positive,
treat this as on-disk corruption, emit EXT4_ERROR_INODE(), and fail the
operation with -EFSCORRUPTED instead of updating the refcount. Delete the
WARN_ONCE() as negative refcounts are now impossible; keep error reporting
in ext4_error_inode().

This prevents the underflow and the follow-on orphan/cleanup churn.

Fixes: https://syzbot.org/bug?extid=0be4f339a8218d2a5bb1
Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
---
 fs/ext4/xattr.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 5a6fe1513fd2..a056f98579c3 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1030,6 +1030,13 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 
 	ref_count = ext4_xattr_inode_get_ref(ea_inode);
 	ref_count += ref_change;
+	if (ref_count < 0) {
+		ext4_error_inode(ea_inode, __func__, __LINE__, 0,
+				"EA inode %lu ref underflow: ref_count=%lld ref_change=%d",
+				ea_inode->i_ino, ref_count, ref_change);
+		ret = -EFSCORRUPTED;
+		goto out;
+	}
 	ext4_xattr_inode_set_ref(ea_inode, ref_count);
 
 	if (ref_change > 0) {
@@ -1044,9 +1051,6 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 			ext4_orphan_del(handle, ea_inode);
 		}
 	} else {
-		WARN_ONCE(ref_count < 0, "EA inode %lu ref_count=%lld",
-			  ea_inode->i_ino, ref_count);
-
 		if (ref_count == 0) {
 			WARN_ONCE(ea_inode->i_nlink != 1,
 				  "EA inode %lu i_nlink=%u",
-- 
2.34.1

Re: [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update

[Darrick J. Wong]

On Thu, Sep 18, 2025 at 08:55:46PM +0300, Ahmet Eray Karadag wrote:
> syzkaller found a path where ext4_xattr_inode_update_ref() reads an EA
> inode refcount that is already <= 0 and then applies ref_change (often
> -1). That lets the refcount underflow and we proceed with a bogus value,
> triggering errors like:
> 
>   EXT4-fs error: EA inode <n> ref underflow: ref_count=-1 ref_change=-1
>   EXT4-fs warning: ea_inode dec ref err=-117
> 
> Make the invariant explicit: if the current refcount is non-positive,
> treat this as on-disk corruption, emit EXT4_ERROR_INODE(), and fail the
> operation with -EFSCORRUPTED instead of updating the refcount. Delete the
> WARN_ONCE() as negative refcounts are now impossible; keep error reporting
> in ext4_error_inode().
> 
> This prevents the underflow and the follow-on orphan/cleanup churn.
> 
> Fixes: https://syzbot.org/bug?extid=0be4f339a8218d2a5bb1
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
> ---
>  fs/ext4/xattr.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 5a6fe1513fd2..a056f98579c3 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1030,6 +1030,13 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
>  
>  	ref_count = ext4_xattr_inode_get_ref(ea_inode);
>  	ref_count += ref_change;
> +	if (ref_count < 0) {

Shouldn't this check ref_count >= ref_change *before* updating it?

--D

> +		ext4_error_inode(ea_inode, __func__, __LINE__, 0,
> +				"EA inode %lu ref underflow: ref_count=%lld ref_change=%d",
> +				ea_inode->i_ino, ref_count, ref_change);
> +		ret = -EFSCORRUPTED;
> +		goto out;
> +	}
>  	ext4_xattr_inode_set_ref(ea_inode, ref_count);
>  
>  	if (ref_change > 0) {
> @@ -1044,9 +1051,6 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
>  			ext4_orphan_del(handle, ea_inode);
>  		}
>  	} else {
> -		WARN_ONCE(ref_count < 0, "EA inode %lu ref_count=%lld",
> -			  ea_inode->i_ino, ref_count);
> -
>  		if (ref_count == 0) {
>  			WARN_ONCE(ea_inode->i_nlink != 1,
>  				  "EA inode %lu i_nlink=%u",
> -- 
> 2.34.1
> 
> 

Re: [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update

[Theodore Ts'o]

On Thu, Sep 18, 2025 at 11:18:01AM -0700, Darrick J. Wong wrote:
> > diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> > index 5a6fe1513fd2..a056f98579c3 100644
> > --- a/fs/ext4/xattr.c
> > +++ b/fs/ext4/xattr.c
> > @@ -1030,6 +1030,13 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
> >  
> >  	ref_count = ext4_xattr_inode_get_ref(ea_inode);
> >  	ref_count += ref_change;
> > +	if (ref_count < 0) {
> 
> Shouldn't this check ref_count >= ref_change *before* updating it?

As Ahmet pointed out, so long as we don't actually update the on-disk
data structure, it's fine.  The issue I'm more concerned about is that
if ref_change is +1, we could also have an overflow where we go from
an ridiculously large positive number (~0) to 0.

Your change might fix one potential syzbot-discovered issue caused by
a maliciously fuzzed file system, but we should harden it against
similar problems going in the opposite problem.

Cheers,

						- Ted

[PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update

[Ahmet Eray Karadag]

syzkaller found a path where ext4_xattr_inode_update_ref() reads an EA
inode refcount that is already <= 0 and then applies ref_change (often
-1). That lets the refcount underflow and we proceed with a bogus value,
triggering errors like:

  EXT4-fs error: EA inode <n> ref underflow: ref_count=-1 ref_change=-1
  EXT4-fs warning: ea_inode dec ref err=-117

Make the invariant explicit: if the current refcount is non-positive,
treat this as on-disk corruption, emit ext4_error_inode(), and fail the
operation with -EFSCORRUPTED instead of updating the refcount. Delete the
WARN_ONCE() as negative refcounts are now impossible; keep error reporting
in ext4_error_inode().

This prevents the underflow and the follow-on orphan/cleanup churn.

Reported-by: syzbot+0be4f339a8218d2a5bb1@syzkaller.appspotmail.com
Fixes: https://syzbot.org/bug?extid=0be4f339a8218d2a5bb1
Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
---
v2:
 - Move underflow guard before the update
 - Add overflow guard for the opposite case
 - Use u64 type instead s64, since ext4_xattr_inode_update_ref() returns u64 and ext4_xattr_inode_set_ref() expects u64.

---
 fs/ext4/xattr.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 5a6fe1513fd2..a510693e04ac 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1019,7 +1019,7 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 				       int ref_change)
 {
 	struct ext4_iloc iloc;
-	s64 ref_count;
+	u64 ref_count;
 	int ret;
 
 	inode_lock_nested(ea_inode, I_MUTEX_XATTR);
@@ -1029,13 +1029,17 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 		goto out;
 
 	ref_count = ext4_xattr_inode_get_ref(ea_inode);
+	if ((ref_count == 0 && ref_change < 0) || (ref_count == U64_MAX && ref_change > 0)) {
+		ext4_error_inode(ea_inode, __func__, __LINE__, 0,
+			"EA inode %lu ref wraparound: ref_count=%lld ref_change=%d",
+			ea_inode->i_ino, ref_count, ref_change);
+		ret = -EFSCORRUPTED;
+		goto out;
+	}
 	ref_count += ref_change;
 	ext4_xattr_inode_set_ref(ea_inode, ref_count);
 
 	if (ref_change > 0) {
-		WARN_ONCE(ref_count <= 0, "EA inode %lu ref_count=%lld",
-			  ea_inode->i_ino, ref_count);
-
 		if (ref_count == 1) {
 			WARN_ONCE(ea_inode->i_nlink, "EA inode %lu i_nlink=%u",
 				  ea_inode->i_ino, ea_inode->i_nlink);
@@ -1044,9 +1048,6 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
 			ext4_orphan_del(handle, ea_inode);
 		}
 	} else {
-		WARN_ONCE(ref_count < 0, "EA inode %lu ref_count=%lld",
-			  ea_inode->i_ino, ref_count);
-
 		if (ref_count == 0) {
 			WARN_ONCE(ea_inode->i_nlink != 1,
 				  "EA inode %lu i_nlink=%u",
-- 
2.34.1

Re: [PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update

[Darrick J. Wong]

On Sat, Sep 20, 2025 at 05:13:43AM +0300, Ahmet Eray Karadag wrote:
> syzkaller found a path where ext4_xattr_inode_update_ref() reads an EA
> inode refcount that is already <= 0 and then applies ref_change (often
> -1). That lets the refcount underflow and we proceed with a bogus value,
> triggering errors like:
> 
>   EXT4-fs error: EA inode <n> ref underflow: ref_count=-1 ref_change=-1
>   EXT4-fs warning: ea_inode dec ref err=-117
> 
> Make the invariant explicit: if the current refcount is non-positive,
> treat this as on-disk corruption, emit ext4_error_inode(), and fail the
> operation with -EFSCORRUPTED instead of updating the refcount. Delete the
> WARN_ONCE() as negative refcounts are now impossible; keep error reporting
> in ext4_error_inode().
> 
> This prevents the underflow and the follow-on orphan/cleanup churn.
> 
> Reported-by: syzbot+0be4f339a8218d2a5bb1@syzkaller.appspotmail.com
> Fixes: https://syzbot.org/bug?extid=0be4f339a8218d2a5bb1
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
> ---
> v2:
>  - Move underflow guard before the update
>  - Add overflow guard for the opposite case
>  - Use u64 type instead s64, since ext4_xattr_inode_update_ref() returns u64 and ext4_xattr_inode_set_ref() expects u64.
> 
> ---
>  fs/ext4/xattr.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 5a6fe1513fd2..a510693e04ac 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -1019,7 +1019,7 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
>  				       int ref_change)
>  {
>  	struct ext4_iloc iloc;
> -	s64 ref_count;
> +	u64 ref_count;
>  	int ret;
>  
>  	inode_lock_nested(ea_inode, I_MUTEX_XATTR);
> @@ -1029,13 +1029,17 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
>  		goto out;
>  
>  	ref_count = ext4_xattr_inode_get_ref(ea_inode);
> +	if ((ref_count == 0 && ref_change < 0) || (ref_count == U64_MAX && ref_change > 0)) {

/me wonders if you could use check_add_overflow for this, but otherwise
everthing looks fine to me...

> +		ext4_error_inode(ea_inode, __func__, __LINE__, 0,
> +			"EA inode %lu ref wraparound: ref_count=%lld ref_change=%d",

Nit: %llu since ref_count is now unsigned.

> +			ea_inode->i_ino, ref_count, ref_change);
> +		ret = -EFSCORRUPTED;
> +		goto out;
> +	}
>  	ref_count += ref_change;
>  	ext4_xattr_inode_set_ref(ea_inode, ref_count);
>  
>  	if (ref_change > 0) {
> -		WARN_ONCE(ref_count <= 0, "EA inode %lu ref_count=%lld",
> -			  ea_inode->i_ino, ref_count);
> -
>  		if (ref_count == 1) {
>  			WARN_ONCE(ea_inode->i_nlink, "EA inode %lu i_nlink=%u",
>  				  ea_inode->i_ino, ea_inode->i_nlink);

...though while you're modifying the precondition checking here, I think
these i_nlink preconditions should also be hoisted to the top and cause
an EFSCORRUPTED return on bad inputs.

--D

> @@ -1044,9 +1048,6 @@ static int ext4_xattr_inode_update_ref(handle_t *handle, struct inode *ea_inode,
>  			ext4_orphan_del(handle, ea_inode);
>  		}
>  	} else {
> -		WARN_ONCE(ref_count < 0, "EA inode %lu ref_count=%lld",
> -			  ea_inode->i_ino, ref_count);
> -
>  		if (ref_count == 0) {
>  			WARN_ONCE(ea_inode->i_nlink != 1,
>  				  "EA inode %lu i_nlink=%u",
> -- 
> 2.34.1
> 
> 

Re: [PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update

[Albin Babu Varghese]

Hi Darrick, Ted,

Thanks a lot for taking the time to review this patch and for the helpful
suggestions. 

> /me wonders if you could use check_add_overflow for this, but otherwise
> everthing looks fine to me...
We looked at check_add_overflow() and check_sub_overflow(), but our
understanding is that they are mainly useful if ref_change can vary beyond the
current ±1. Since the call site appear to only pass increments or decrements
of one, would you prefer we still use the helpers for defensive hardening, or
is it acceptable to rely on explicit 0 / U64_MAX boundary checks in this case?

> ...though while you're modifying the precondition checking here, I think
> these i_nlink preconditions should also be hoisted to the top and cause
> an EFSCORRUPTED return on bad inputs.
Thanks for pointing this out. We will include this in V3.

Cheers,
	Albin

Re: [PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update

[Theodore Ts'o]

On Sat, 20 Sep 2025 05:13:43 +0300, Ahmet Eray Karadag wrote:
> syzkaller found a path where ext4_xattr_inode_update_ref() reads an EA
> inode refcount that is already <= 0 and then applies ref_change (often
> -1). That lets the refcount underflow and we proceed with a bogus value,
> triggering errors like:
> 
>   EXT4-fs error: EA inode <n> ref underflow: ref_count=-1 ref_change=-1
>   EXT4-fs warning: ea_inode dec ref err=-117
> 
> [...]

Applied, thanks!

[1/1] Fix: ext4: guard against EA inode refcount underflow in xattr update
      commit: 57295e835408d8d425bef58da5253465db3d6888

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>