Re: [BUG] ext4: NULL/invalid pointer dereference in ext4_writepages() during generic/039

Hello!

On Tue 04-08-26 01:18:49, Ojaswin Mujoo wrote:
> On Mon, Aug 03, 2026 at 12:22:35PM +0530, Ojaswin Mujoo wrote:
...
> > > Meanwhile, I will attempt bisection.
> > > 
> > > If you happen to fix this issue, please add below tag.
> > > 
> > > Reported-by: Venkat Rao Bagalkote <[email protected]>
> > > 
> > 
> > Hey thanks for reporting this Venkat, I'll try to replicate this at my
> > end.
> > 
> > In the meantime, can you please share the complete Oops message and also
> > your local.config. 
> > 
> > Thanks,
> > ojaswin
> 
> (+cc Jan, Christian)
> 
> Okay so I'm able to replicate this in an x86 machine as well with 4kb
> block size. The issue is replicable in vfs/vfs.all and is occurring
> after Jan's change to nojournal mode writeout [1], with fast_commit.
> 
> The issue is as follows:
> 
>   __ext4_fill_super
>     ext4_load_and_init_journal
>       ...
>       jbd2_journal_recover
>         do_one_pass
>           fc_do_one_pass
>             ...
>             ext4_fc_replay_unlink
>               __ext4_unlink
>                 __ext4_mark_iloc_dirty
>                   if (sbi->s_journal == NULL)  // s_journal is not set yet
>                     set_inode_metadata_writeback(inode); // sets I_METADATA_WRITEBACK
>               iput(inode)
>                 iput_final(inode)
>                   ...
>                   writeback_single_inode
>                     __writeback_single_inode  // since I_METADATA_WRITEBACK is set
>                       ...
>                       ext4_writepages
>                         ext4_writepages_down_read
>                           percpu_down_read(sbi->s_writepages_rwsem)
>                              <--- OOPS because this is initialized
>                                   later in ext4_percpu_param_init()
> 
> A simple fix seems to be to move ext4_percpu_param_init() call before
> ext4_load_journal() so that we can safely call ext4_writepages(). The
> below diff seems to be fixing the issue for me, does this look okay to
> you Jan?

Thanks for report and the analysis! I didn't realize fastcommit replay
actually runs in nojournal mode. I don't think moving
ext4_percpu_param_init() is really correct - it initializes also directory,
free blocks and other counters and those depend on filesystem already being
in consistent state (i.e., after journal replay). We would have to move
just s_writepages_rwsem initialization.

That being said I think a nicer fix is to just avoid setting
I_METADATA_WRITEBACK flag during fastcommit replay. We don't really need
it for anything during replay (it flushes the whole bdev anyway), it just
slows things down by more frequent buffer writeout. I've tested that change
and it fixes the problem for me. I'll post it officially shortly.

								Honza 

> Also, venkat can you please help test this in your system:
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 2877a6cf6d09..6e05f191542c 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5511,6 +5511,10 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>                         goto failed_mount3a;
>         }
> 
> +       err = ext4_percpu_param_init(sbi);
> +       if (err)
> +               goto failed_mount3a;
> +
>         err = -EINVAL;
>         /*
>          * The first inode we look at is the journal inode.  Don't try
> @@ -5659,10 +5663,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>                         clear_opt2(sb, MB_OPTIMIZE_SCAN);
>         }
> 
> -       err = ext4_percpu_param_init(sbi);
> -       if (err)
> -               goto failed_mount5;
> -
>         err = ext4_mb_init(sb);
>         if (err) {
>                 ext4_msg(sb, KERN_ERR, "failed to initialize mballoc (%d)",
> @@ -5764,7 +5764,6 @@ failed_mount8: __maybe_unused
>         ext4_mb_release(sb);
>         ext4_flex_groups_free(sbi);
>  failed_mount5:
> -       ext4_percpu_param_destroy(sbi);
>         ext4_ext_release(sb);
>         ext4_release_system_zone(sb);
>  failed_mount4a:
> @@ -5785,6 +5784,7 @@ failed_mount8: __maybe_unused
>                 ext4_journal_destroy(sbi, sbi->s_journal);
>         }
>  failed_mount3a:
> +       ext4_percpu_param_destroy(sbi);
>         ext4_es_unregister_shrinker(sbi);
>  failed_mount3:
>         /* flush s_sb_upd_work before sbi destroy */
> 
> [1] https://lore.kernel.org/linux-ext4/[email protected]/
> > > 
> > > 
> > > Thanks,
> > > Venkat.
> > > 
-- 
Jan Kara <[email protected]>
SUSE Labs, CR