* [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update @ 2025-09-18 17:55 Ahmet Eray Karadag 2025-09-18 18:18 ` Darrick J. Wong 2025-09-20 2:13 ` [PATCH v2] " Ahmet Eray Karadag 0 siblings, 2 replies; 7+ messages in thread From: Ahmet Eray Karadag @ 2025-09-18 17:55 UTC (permalink / raw) To: tytso, adilger.kernel Cc: linux-ext4, linux-kernel, Ahmet Eray Karadag, Albin Babu Varghese 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update 2025-09-18 17:55 [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update Ahmet Eray Karadag @ 2025-09-18 18:18 ` Darrick J. Wong 2025-09-19 14:45 ` Theodore Ts'o 2025-09-20 2:13 ` [PATCH v2] " Ahmet Eray Karadag 1 sibling, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2025-09-18 18:18 UTC (permalink / raw) To: Ahmet Eray Karadag Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, Albin Babu Varghese 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update 2025-09-18 18:18 ` Darrick J. Wong @ 2025-09-19 14:45 ` Theodore Ts'o 0 siblings, 0 replies; 7+ messages in thread From: Theodore Ts'o @ 2025-09-19 14:45 UTC (permalink / raw) To: Darrick J. Wong Cc: Ahmet Eray Karadag, adilger.kernel, linux-ext4, linux-kernel, Albin Babu Varghese 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update 2025-09-18 17:55 [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update Ahmet Eray Karadag 2025-09-18 18:18 ` Darrick J. Wong @ 2025-09-20 2:13 ` Ahmet Eray Karadag 2025-09-23 23:39 ` Darrick J. Wong 2025-09-26 21:47 ` Theodore Ts'o 1 sibling, 2 replies; 7+ messages in thread From: Ahmet Eray Karadag @ 2025-09-20 2:13 UTC (permalink / raw) To: tytso, adilger.kernel Cc: linux-ext4, linux-kernel, Ahmet Eray Karadag, syzbot+0be4f339a8218d2a5bb1, Albin Babu Varghese 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update 2025-09-20 2:13 ` [PATCH v2] " Ahmet Eray Karadag @ 2025-09-23 23:39 ` Darrick J. Wong 2025-09-24 22:53 ` Albin Babu Varghese 2025-09-26 21:47 ` Theodore Ts'o 1 sibling, 1 reply; 7+ messages in thread From: Darrick J. Wong @ 2025-09-23 23:39 UTC (permalink / raw) To: Ahmet Eray Karadag Cc: tytso, adilger.kernel, linux-ext4, linux-kernel, syzbot+0be4f339a8218d2a5bb1, Albin Babu Varghese 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update 2025-09-23 23:39 ` Darrick J. Wong @ 2025-09-24 22:53 ` Albin Babu Varghese 0 siblings, 0 replies; 7+ messages in thread From: Albin Babu Varghese @ 2025-09-24 22:53 UTC (permalink / raw) To: Darrick J. Wong Cc: Ahmet Eray Karadag, tytso, adilger.kernel, linux-ext4, linux-kernel, syzbot+0be4f339a8218d2a5bb1 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] Fix: ext4: guard against EA inode refcount underflow in xattr update 2025-09-20 2:13 ` [PATCH v2] " Ahmet Eray Karadag 2025-09-23 23:39 ` Darrick J. Wong @ 2025-09-26 21:47 ` Theodore Ts'o 1 sibling, 0 replies; 7+ messages in thread From: Theodore Ts'o @ 2025-09-26 21:47 UTC (permalink / raw) To: adilger.kernel, Ahmet Eray Karadag Cc: Theodore Ts'o, linux-ext4, linux-kernel, syzbot+0be4f339a8218d2a5bb1, Albin Babu Varghese 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-09-26 21:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-18 17:55 [PATCH] Fix: ext4: guard against EA inode refcount underflow in xattr update Ahmet Eray Karadag 2025-09-18 18:18 ` Darrick J. Wong 2025-09-19 14:45 ` Theodore Ts'o 2025-09-20 2:13 ` [PATCH v2] " Ahmet Eray Karadag 2025-09-23 23:39 ` Darrick J. Wong 2025-09-24 22:53 ` Albin Babu Varghese 2025-09-26 21:47 ` Theodore Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).