Re: [f2fs-dev] [PATCH v2] f2fs: initialize ino_entry_info before checkpoint load
Chao Yu via Linux-f2fs-devel <[email protected]>
| Newsgroups | net.sourceforge.lists.linux-f2fs-devel,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 6/21/26 16:18, Deepanshu Kartikey wrote: > On Sun, May 10, 2026 at 9:53 AM Deepanshu Kartikey > <[email protected]> wrote: >> >> When f2fs_get_valid_checkpoint() fails during mount (e.g. due to an >> invalid checkpoint CRC on a malformed image), f2fs_fill_super() takes >> an error path that eventually calls iput() on the root inode. This >> invokes f2fs_drop_inode() -> f2fs_exist_written_data(), which acquires >> sbi->im[]->ino_lock. However, f2fs_init_ino_entry_info() has not run >> yet at this point, so the spinlock is uninitialized and lockdep >> complains: >> >> F2FS-fs (loop0): invalid crc value >> F2FS-fs (loop0): Failed to get valid F2FS checkpoint >> INFO: trying to register non-static key. >> The code is fine but needs lockdep annotation, or maybe >> you didn't initialize this object before use? >> ... >> f2fs_exist_written_data+0x53/0x90 fs/f2fs/checkpoint.c:787 >> f2fs_drop_inode+0xda/0xbf0 fs/f2fs/super.c:1852 >> iput+0x651/0xe80 fs/inode.c:2009 >> f2fs_fill_super+0x6047/0x7850 fs/f2fs/super.c:5461 >> >> Move f2fs_init_ino_entry_info() to before f2fs_get_valid_checkpoint() >> so that sbi->im[] is always fully initialized before any error path >> can trigger iput() -> f2fs_drop_inode(). The init function only >> depends on raw superblock fields (BLKS_PER_SEG, F2FS_CP_PACKS, >> NR_CURSEG_PERSIST_TYPE, __cp_payload), which are populated well >> before checkpoint load, so the move is safe. >> >> Fixes: 3063c80776e3 ("f2fs: another way to set large folio by remembering inode number") >> Cc: [email protected] >> Reported-by: [email protected] >> Closes: https://syzkaller.appspot.com/bug?extid=eec8f2693d71386bd600 >> Tested-by: [email protected] >> Signed-off-by: Deepanshu Kartikey <[email protected]> >> --- >> Changes in v2: >> - Add Fixes: tag (suggested by Chao Yu) >> - Add Cc: [email protected] >> --- >> fs/f2fs/super.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index c6afdbd6e1cd..6a231a5b0d62 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -5140,6 +5140,13 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) >> goto free_page_array_cache; >> } >> >> + /* >> + * Initialize ino entry info early so f2fs_drop_inode -> >> + * f2fs_exist_written_data can safely take im->ino_lock if mount >> + * fails after this point and triggers iput on cleanup. >> + */ >> + f2fs_init_ino_entry_info(sbi); >> + >> err = f2fs_get_valid_checkpoint(sbi); >> if (err) { >> f2fs_err(sbi, "Failed to get valid F2FS checkpoint"); >> @@ -5184,8 +5191,6 @@ static int f2fs_fill_super(struct super_block *sb, struct fs_context *fc) >> >> f2fs_init_extent_cache_info(sbi); >> >> - f2fs_init_ino_entry_info(sbi); >> - >> f2fs_init_fsync_node_info(sbi); >> >> /* setup checkpoint request control and start checkpoint issue thread */ >> -- >> 2.43.0 >> > > Hi Chao, > > Please let me know the status of this patch. This is already "Reviewed-by:" you. https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/commit/?h=dev-test&id=065a6f8cd23a9297b543dcec913feb3cb787a25e It was merged, however, I think it should be reverted because commit 3063c80776e3 ("f2fs: another way to set large folio by remembering inode number") was removed from dev branch, we won't call f2fs_exist_written_data() from f2fs_drop_inode(). Thanks, > > Please let me know if anything needed from my side. > > Thanks > > Deepanshu Kartikey _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel