[PATCH] jbd2: gracefully abort instead of panicking on unlocked buffer

[PATCH] jbd2: gracefully abort instead of panicking on unlocked buffer

[Milos Nikic]

In jbd2_journal_get_create_access(), if the caller passes an unlocked
buffer, the code currently triggers a fatal J_ASSERT.

While an unlocked buffer here is a clear API violation and a bug in the
caller, crashing the entire system is an overly severe response. It brings
down the whole machine for a localized filesystem inconsistency.

Replace the J_ASSERT with a WARN_ON_ONCE to capture the offending caller's
stack trace, and return an error (-EINVAL). This allows the journal to
gracefully abort the transaction, protecting data integrity without
causing a kernel panic.

Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
---
 fs/jbd2/transaction.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index dca4b5d8aaaa..04d17a5f2a82 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1302,7 +1302,12 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
 		goto out;
 	}
 
-	J_ASSERT_JH(jh, buffer_locked(jh2bh(jh)));
+	if (WARN_ON_ONCE(!buffer_locked(jh2bh(jh)))) {
+		err = -EINVAL;
+		spin_unlock(&jh->b_state_lock);
+		jbd2_journal_abort(journal, err);
+		goto out;
+	}
 
 	if (jh->b_transaction == NULL) {
 		/*
-- 
2.53.0

Re: [PATCH] jbd2: gracefully abort instead of panicking on unlocked buffer

[Zhang Yi]

On 3/2/2026 8:31 AM, Milos Nikic wrote:
> In jbd2_journal_get_create_access(), if the caller passes an unlocked
> buffer, the code currently triggers a fatal J_ASSERT.
> 
> While an unlocked buffer here is a clear API violation and a bug in the
> caller, crashing the entire system is an overly severe response. It brings
> down the whole machine for a localized filesystem inconsistency.
> 
> Replace the J_ASSERT with a WARN_ON_ONCE to capture the offending caller's
> stack trace, and return an error (-EINVAL). This allows the journal to
> gracefully abort the transaction, protecting data integrity without
> causing a kernel panic.
> 
> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>

Thank you for the patch, this makes sense to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/jbd2/transaction.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index dca4b5d8aaaa..04d17a5f2a82 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1302,7 +1302,12 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
>  		goto out;
>  	}
>  
> -	J_ASSERT_JH(jh, buffer_locked(jh2bh(jh)));
> +	if (WARN_ON_ONCE(!buffer_locked(jh2bh(jh)))) {
> +		err = -EINVAL;
> +		spin_unlock(&jh->b_state_lock);
> +		jbd2_journal_abort(journal, err);
> +		goto out;
> +	}
>  
>  	if (jh->b_transaction == NULL) {
>  		/*

Re: [PATCH] jbd2: gracefully abort instead of panicking on unlocked buffer

[Jan Kara]

On Sun 01-03-26 16:31:35, Milos Nikic wrote:
> In jbd2_journal_get_create_access(), if the caller passes an unlocked
> buffer, the code currently triggers a fatal J_ASSERT.
> 
> While an unlocked buffer here is a clear API violation and a bug in the
> caller, crashing the entire system is an overly severe response. It brings
> down the whole machine for a localized filesystem inconsistency.
> 
> Replace the J_ASSERT with a WARN_ON_ONCE to capture the offending caller's
> stack trace, and return an error (-EINVAL). This allows the journal to
> gracefully abort the transaction, protecting data integrity without
> causing a kernel panic.
> 
> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>

In principle I'm fine with this however we have lots of similar asserts in
the code. So how is this one special? Did you somehow trigger it?

								Honza

> ---
>  fs/jbd2/transaction.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index dca4b5d8aaaa..04d17a5f2a82 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1302,7 +1302,12 @@ int jbd2_journal_get_create_access(handle_t *handle, struct buffer_head *bh)
>  		goto out;
>  	}
>  
> -	J_ASSERT_JH(jh, buffer_locked(jh2bh(jh)));
> +	if (WARN_ON_ONCE(!buffer_locked(jh2bh(jh)))) {
> +		err = -EINVAL;
> +		spin_unlock(&jh->b_state_lock);
> +		jbd2_journal_abort(journal, err);
> +		goto out;
> +	}
>  
>  	if (jh->b_transaction == NULL) {
>  		/*
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] jbd2: gracefully abort instead of panicking on unlocked buffer

[Jan Kara]

On Mon 02-03-26 07:58:41, Milos Nikic wrote:
> Hi Jan,
> 
> No, I didn't trigger this in the wild. I have been auditing jbd2 for
> J_ASSERT usage to see where we could proactively swap hard panics for
> graceful journal aborts.
> You are right that there are many similar asserts, but I focused on this
> one because it belongs to a specific, easily-actionable group: it resides
> in a function that already returns an error code (int).
> 
> Many of the other J_ASSERTs are buried inside void functions. Converting
> those to return errors would require cascading API changes and rewriting
> caller error-handling paths across the subsystem, which is a much bigger
> and riskier lift.
> My goal was just to target the "low-hanging fruit"—the asserts where the
> function signature already supports returning an error (-EINVAL/-EIO) and
> aborting the journal safely without changing the API.
> 
> If you are open to it, I can audit the codebase for the rest of the asserts
> that fit this exact profile and submit them.
> Would you prefer them grouped into a single patch, or a short series?

OK, thanks for explanation. So if you can submit a patch addressing these
easy cases (possibly split to one patch per file in case the patch would be
too big) that would be good. For this patch feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> 
> Thanks,
> Milos
> 
> On Mon, Mar 2, 2026 at 5:08 AM Jan Kara <jack@suse.cz> wrote:
> 
> > On Sun 01-03-26 16:31:35, Milos Nikic wrote:
> > > In jbd2_journal_get_create_access(), if the caller passes an unlocked
> > > buffer, the code currently triggers a fatal J_ASSERT.
> > >
> > > While an unlocked buffer here is a clear API violation and a bug in the
> > > caller, crashing the entire system is an overly severe response. It
> > brings
> > > down the whole machine for a localized filesystem inconsistency.
> > >
> > > Replace the J_ASSERT with a WARN_ON_ONCE to capture the offending
> > caller's
> > > stack trace, and return an error (-EINVAL). This allows the journal to
> > > gracefully abort the transaction, protecting data integrity without
> > > causing a kernel panic.
> > >
> > > Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
> >
> > In principle I'm fine with this however we have lots of similar asserts in
> > the code. So how is this one special? Did you somehow trigger it?
> >
> >                                                                 Honza
> >
> > > ---
> > >  fs/jbd2/transaction.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> > > index dca4b5d8aaaa..04d17a5f2a82 100644
> > > --- a/fs/jbd2/transaction.c
> > > +++ b/fs/jbd2/transaction.c
> > > @@ -1302,7 +1302,12 @@ int jbd2_journal_get_create_access(handle_t
> > *handle, struct buffer_head *bh)
> > >               goto out;
> > >       }
> > >
> > > -     J_ASSERT_JH(jh, buffer_locked(jh2bh(jh)));
> > > +     if (WARN_ON_ONCE(!buffer_locked(jh2bh(jh)))) {
> > > +             err = -EINVAL;
> > > +             spin_unlock(&jh->b_state_lock);
> > > +             jbd2_journal_abort(journal, err);
> > > +             goto out;
> > > +     }
> > >
> > >       if (jh->b_transaction == NULL) {
> > >               /*
> > > --
> > > 2.53.0
> > >
> > --
> > Jan Kara <jack@suse.com>
> > SUSE Labs, CR
> >
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] jbd2: gracefully abort instead of panicking on unlocked buffer

[Milos Nikic]

Hi Jan,

No, I didn't trigger this in the wild. I have been auditing jbd2 for J_ASSERT usage to see where we could proactively swap hard panics for graceful journal aborts. 

You are right that there are many similar asserts, but I focused on this one because it belongs to a specific, easily-actionable group: it resides in a function that already returns an error code (int). 

Some of the other J_ASSERTs are buried inside void functions. Converting those to return errors would require cascading API changes and rewriting caller error-handling paths across the subsystem, which is a much bigger and riskier lift.

My goal was just to target the "low-hanging fruit" - the asserts where the function signature already supports returning an error (-EINVAL/-EIO) and aborting the journal safely without changing the API.

If you are open to it, I can audit the codebase for the rest of the asserts that fit this exact profile and submit them. Would you prefer them grouped into a single patch, or a short series?

Thanks,
Milos