Re: ext4 regression panic
From: Jan Kara <jack@suse.cz>
Date: 2021-01-21 21:11:24
On Thu 21-01-21 12:40:56, Theodore Ts'o wrote:
On Thu, Jan 21, 2021 at 06:15:47PM +0800, Murphy Zhou wrote:quoted
Hi Jack, A panic was introduced by this commit. It's easy and reliable to reproduce. commit 2d01ddc86606564fb08c56e3bc93a0693895f710 Author: Jan Kara [off-list ref] Date: Wed Dec 16 11:18:40 2020 +0100 ext4: save error info to sb through journal if availableHi Murphy, Thanks for the bug report. What's happening is that we haven't yet initialized mballoc yet --- that happens in line 4943 of fs/ext4/super.c, in ext4_fill_super(). But in line 4903 (in the case of the BZ #199275 reproducer), we attempt to fetch the root inode, which is fails because it is unallocated. That then triggers a call to ext4_error(), which now results in a journalled change, since the journal is initialized starting in line 4793, and in line 4838, we set up the j_commit_callback, which is what ends up calling ext4_process_freed_data(), but since the multiblock allocator hasn't been set up yet, that causes the NULL pointer dereference. So what we need to do is to *not* set up the callback until after the call to ext4_mb_init(). We should probably create an ext4-specific test in xfstests which tries mounting a small, deliberately corrupted file system, to make sure we handle this case correctly in the future. - Ted
Thanks for looking into this. You beat me to my fix (which was slightly different - I moved ext4_mb_init() somewhat earlier during mount). But this should work fine as well. So feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza
quoted hunk ↗ jump to hunk
commit 6c2f9a8247273cf1108ff71c99680b7457f48318 Author: Theodore Ts'o [off-list ref] Date: Thu Jan 21 12:33:20 2021 -0500 ext4: don't try to processed freed blocks until mballoc is initialized If we try to make any changes via the journal between when the journal is initialized, but before the multi-block allocated is initialized, we will end up deferencing a NULL pointer when the journal commit callback function calls ext4_process_freed_data(). The proximate cause of this failure was commit 2d01ddc86606 ("ext4: save error info to sb through journal if available") since file system corruption problems detected before the call to ext4_mb_init() would result in a journal commit before we aborted the mount of the file system.... and we would then trigger the NULL pointer deref. Cc: Jan Kara [off-list ref] Reported by: Murphy Zhou [off-list ref] Signed-off-by: Theodore Ts'o [off-list ref]diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 0f0db49031dc..802ef55f0a55 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c@@ -4876,7 +4876,6 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) set_task_ioprio(sbi->s_journal->j_task, journal_ioprio); - sbi->s_journal->j_commit_callback = ext4_journal_commit_callback; sbi->s_journal->j_submit_inode_data_buffers = ext4_journal_submit_inode_data_buffers; sbi->s_journal->j_finish_inode_data_buffers =@@ -4993,6 +4992,14 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) goto failed_mount5; } + /* + * We can only set up the journal commit callback once + * mballoc is initialized + */ + if (sbi->s_journal) + sbi->s_journal->j_commit_callback = + ext4_journal_commit_callback; + block = ext4_count_free_clusters(sb); ext4_free_blocks_count_set(sbi->s_es, EXT4_C2B(sbi, block));
-- Jan Kara [off-list ref] SUSE Labs, CR