[v5] jbd2: audit and convert legacy J_ASSERT usage

[PATCH v5 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer

Posted by Milos Nikic 1 month ago

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>
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
---
 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 v5 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer

Posted by Milos Nikic 2 weeks, 2 days ago

Hello,

Just sending a gentle ping on this v5 series from March 4th.
It looks like both patches have collected Reviewed-by tags from Jan,
Andreas and Zhang.

Please let me know if there is anything else you need from my side, or
if this is good to be queued up in the ext4 tree for the next merge
window.

Thanks, Milos

On Wed, Mar 4, 2026 at 9:20 AM Milos Nikic <nikic.milos@gmail.com> 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>
> Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> ---
>  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 v5 1/2] jbd2: gracefully abort instead of panicking on unlocked buffer

Posted by Jan Kara 2 weeks, 2 days ago

Hello!

On Mon 23-03-26 14:50:00, Milos Nikic wrote:
> Just sending a gentle ping on this v5 series from March 4th.
> It looks like both patches have collected Reviewed-by tags from Jan,
> Andreas and Zhang.
> 
> Please let me know if there is anything else you need from my side, or
> if this is good to be queued up in the ext4 tree for the next merge
> window.

Ted Tso is generally picking jbd2 patches. Since this is a cleanup, I
expect him to pick up this patch for the next merge window - i.e., in a
week or two :)

								Honza

> 
> Thanks, Milos
> 
> On Wed, Mar 4, 2026 at 9:20 AM Milos Nikic <nikic.milos@gmail.com> 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>
> > Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> > ---
> >  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