* [PATCH v4] f2fs: add check for deleted inode
@ 2025-02-12 7:27 Chao Yu
2025-02-12 16:47 ` Jaegeuk Kim
0 siblings, 1 reply; 10+ messages in thread
From: Chao Yu @ 2025-02-12 7:27 UTC (permalink / raw)
To: jaegeuk
Cc: linux-f2fs-devel, linux-kernel, Leo Stone,
syzbot+b01a36acd7007e273a83, Chao Yu
From: Leo Stone <leocstone@gmail.com>
The syzbot reproducer mounts a f2fs image, then tries to unlink an
existing file. However, the unlinked file already has a link count of 0
when it is read for the first time in do_read_inode().
Add a check to sanity_check_inode() for i_nlink == 0.
[Chao Yu: rebase the code and fix orphan inode recovery issue]
Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83
Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure")
Signed-off-by: Leo Stone <leocstone@gmail.com>
Signed-off-by: Chao Yu <chao@kernel.org>
---
fs/f2fs/checkpoint.c | 4 ++++
fs/f2fs/f2fs.h | 1 +
fs/f2fs/inode.c | 6 ++++++
3 files changed, 11 insertions(+)
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index bd890738b94d..ada2c548645c 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE))
f2fs_info(sbi, "orphan cleanup on readonly fs");
+ set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY);
+
start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi);
@@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
}
f2fs_put_page(page, 1);
}
+
/* clear Orphan Flag */
clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
out:
+ clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY);
set_sbi_flag(sbi, SBI_IS_RECOVERED);
return err;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 05879c6dc4d6..1c75081c0c14 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1322,6 +1322,7 @@ enum {
SBI_IS_CLOSE, /* specify unmounting */
SBI_NEED_FSCK, /* need fsck.f2fs to fix */
SBI_POR_DOING, /* recovery is doing or not */
+ SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */
SBI_NEED_SB_WRITE, /* need to recover superblock */
SBI_NEED_CP, /* need to checkpoint */
SBI_IS_SHUTDOWN, /* shutdown by ioctl */
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index d6ad7810df69..02f1b69d03d8 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page)
}
}
+ if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) {
+ f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0",
+ __func__, inode->i_ino);
+ return false;
+ }
+
return true;
}
--
2.48.1.502.g6dc24dfdaf-goog
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-12 7:27 [PATCH v4] f2fs: add check for deleted inode Chao Yu @ 2025-02-12 16:47 ` Jaegeuk Kim 2025-02-13 2:00 ` Chao Yu 0 siblings, 1 reply; 10+ messages in thread From: Jaegeuk Kim @ 2025-02-12 16:47 UTC (permalink / raw) To: Chao Yu Cc: linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 02/12, Chao Yu wrote: > From: Leo Stone <leocstone@gmail.com> > > The syzbot reproducer mounts a f2fs image, then tries to unlink an > existing file. However, the unlinked file already has a link count of 0 > when it is read for the first time in do_read_inode(). > > Add a check to sanity_check_inode() for i_nlink == 0. > > [Chao Yu: rebase the code and fix orphan inode recovery issue] > Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 > Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") > Signed-off-by: Leo Stone <leocstone@gmail.com> > Signed-off-by: Chao Yu <chao@kernel.org> > --- > fs/f2fs/checkpoint.c | 4 ++++ > fs/f2fs/f2fs.h | 1 + > fs/f2fs/inode.c | 6 ++++++ > 3 files changed, 11 insertions(+) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index bd890738b94d..ada2c548645c 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) > f2fs_info(sbi, "orphan cleanup on readonly fs"); > > + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); What about using SBI_POR_DOING? > + > start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); > orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); > > @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > } > f2fs_put_page(page, 1); > } > + > /* clear Orphan Flag */ > clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > out: > + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > set_sbi_flag(sbi, SBI_IS_RECOVERED); > > return err; > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 05879c6dc4d6..1c75081c0c14 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -1322,6 +1322,7 @@ enum { > SBI_IS_CLOSE, /* specify unmounting */ > SBI_NEED_FSCK, /* need fsck.f2fs to fix */ > SBI_POR_DOING, /* recovery is doing or not */ > + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ > SBI_NEED_SB_WRITE, /* need to recover superblock */ > SBI_NEED_CP, /* need to checkpoint */ > SBI_IS_SHUTDOWN, /* shutdown by ioctl */ > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index d6ad7810df69..02f1b69d03d8 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) > } > } > > + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { > + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", > + __func__, inode->i_ino); > + return false; > + } > + > return true; > } > > -- > 2.48.1.502.g6dc24dfdaf-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-12 16:47 ` Jaegeuk Kim @ 2025-02-13 2:00 ` Chao Yu 2025-02-13 17:38 ` Jaegeuk Kim 0 siblings, 1 reply; 10+ messages in thread From: Chao Yu @ 2025-02-13 2:00 UTC (permalink / raw) To: Jaegeuk Kim Cc: chao, linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 2/13/25 00:47, Jaegeuk Kim wrote: > On 02/12, Chao Yu wrote: >> From: Leo Stone <leocstone@gmail.com> >> >> The syzbot reproducer mounts a f2fs image, then tries to unlink an >> existing file. However, the unlinked file already has a link count of 0 >> when it is read for the first time in do_read_inode(). >> >> Add a check to sanity_check_inode() for i_nlink == 0. >> >> [Chao Yu: rebase the code and fix orphan inode recovery issue] >> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com >> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 >> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") >> Signed-off-by: Leo Stone <leocstone@gmail.com> >> Signed-off-by: Chao Yu <chao@kernel.org> >> --- >> fs/f2fs/checkpoint.c | 4 ++++ >> fs/f2fs/f2fs.h | 1 + >> fs/f2fs/inode.c | 6 ++++++ >> 3 files changed, 11 insertions(+) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index bd890738b94d..ada2c548645c 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) >> f2fs_info(sbi, "orphan cleanup on readonly fs"); >> >> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > > What about using SBI_POR_DOING? SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow iget() of root_inode and all inodes during roll-forward recovery to do sanity check nlink w/ zero. What do you think? Thanks, > >> + >> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); >> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); >> >> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >> } >> f2fs_put_page(page, 1); >> } >> + >> /* clear Orphan Flag */ >> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >> out: >> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >> set_sbi_flag(sbi, SBI_IS_RECOVERED); >> >> return err; >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 05879c6dc4d6..1c75081c0c14 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -1322,6 +1322,7 @@ enum { >> SBI_IS_CLOSE, /* specify unmounting */ >> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ >> SBI_POR_DOING, /* recovery is doing or not */ >> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ >> SBI_NEED_SB_WRITE, /* need to recover superblock */ >> SBI_NEED_CP, /* need to checkpoint */ >> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ >> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >> index d6ad7810df69..02f1b69d03d8 100644 >> --- a/fs/f2fs/inode.c >> +++ b/fs/f2fs/inode.c >> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) >> } >> } >> >> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { >> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", >> + __func__, inode->i_ino); >> + return false; >> + } >> + >> return true; >> } >> >> -- >> 2.48.1.502.g6dc24dfdaf-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-13 2:00 ` Chao Yu @ 2025-02-13 17:38 ` Jaegeuk Kim 2025-02-14 1:44 ` Chao Yu 0 siblings, 1 reply; 10+ messages in thread From: Jaegeuk Kim @ 2025-02-13 17:38 UTC (permalink / raw) To: Chao Yu Cc: linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 02/13, Chao Yu wrote: > On 2/13/25 00:47, Jaegeuk Kim wrote: > > On 02/12, Chao Yu wrote: > >> From: Leo Stone <leocstone@gmail.com> > >> > >> The syzbot reproducer mounts a f2fs image, then tries to unlink an > >> existing file. However, the unlinked file already has a link count of 0 > >> when it is read for the first time in do_read_inode(). > >> > >> Add a check to sanity_check_inode() for i_nlink == 0. > >> > >> [Chao Yu: rebase the code and fix orphan inode recovery issue] > >> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com > >> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 > >> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") > >> Signed-off-by: Leo Stone <leocstone@gmail.com> > >> Signed-off-by: Chao Yu <chao@kernel.org> > >> --- > >> fs/f2fs/checkpoint.c | 4 ++++ > >> fs/f2fs/f2fs.h | 1 + > >> fs/f2fs/inode.c | 6 ++++++ > >> 3 files changed, 11 insertions(+) > >> > >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > >> index bd890738b94d..ada2c548645c 100644 > >> --- a/fs/f2fs/checkpoint.c > >> +++ b/fs/f2fs/checkpoint.c > >> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > >> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) > >> f2fs_info(sbi, "orphan cleanup on readonly fs"); > >> > >> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > > > > What about using SBI_POR_DOING? > > SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a > separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow > iget() of root_inode and all inodes during roll-forward recovery to do sanity > check nlink w/ zero. What do you think? Can we do this sanity check after f2fs_iget in the f2fs_unlink() only? > > Thanks, > > > > >> + > >> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); > >> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); > >> > >> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > >> } > >> f2fs_put_page(page, 1); > >> } > >> + > >> /* clear Orphan Flag */ > >> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > >> out: > >> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > >> set_sbi_flag(sbi, SBI_IS_RECOVERED); > >> > >> return err; > >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >> index 05879c6dc4d6..1c75081c0c14 100644 > >> --- a/fs/f2fs/f2fs.h > >> +++ b/fs/f2fs/f2fs.h > >> @@ -1322,6 +1322,7 @@ enum { > >> SBI_IS_CLOSE, /* specify unmounting */ > >> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ > >> SBI_POR_DOING, /* recovery is doing or not */ > >> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ > >> SBI_NEED_SB_WRITE, /* need to recover superblock */ > >> SBI_NEED_CP, /* need to checkpoint */ > >> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ > >> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > >> index d6ad7810df69..02f1b69d03d8 100644 > >> --- a/fs/f2fs/inode.c > >> +++ b/fs/f2fs/inode.c > >> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) > >> } > >> } > >> > >> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { > >> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", > >> + __func__, inode->i_ino); > >> + return false; > >> + } > >> + > >> return true; > >> } > >> > >> -- > >> 2.48.1.502.g6dc24dfdaf-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-13 17:38 ` Jaegeuk Kim @ 2025-02-14 1:44 ` Chao Yu 2025-02-24 10:47 ` Chao Yu 0 siblings, 1 reply; 10+ messages in thread From: Chao Yu @ 2025-02-14 1:44 UTC (permalink / raw) To: Jaegeuk Kim Cc: chao, linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 2/14/25 01:38, Jaegeuk Kim wrote: > On 02/13, Chao Yu wrote: >> On 2/13/25 00:47, Jaegeuk Kim wrote: >>> On 02/12, Chao Yu wrote: >>>> From: Leo Stone <leocstone@gmail.com> >>>> >>>> The syzbot reproducer mounts a f2fs image, then tries to unlink an >>>> existing file. However, the unlinked file already has a link count of 0 >>>> when it is read for the first time in do_read_inode(). >>>> >>>> Add a check to sanity_check_inode() for i_nlink == 0. >>>> >>>> [Chao Yu: rebase the code and fix orphan inode recovery issue] >>>> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com >>>> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 >>>> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") >>>> Signed-off-by: Leo Stone <leocstone@gmail.com> >>>> Signed-off-by: Chao Yu <chao@kernel.org> >>>> --- >>>> fs/f2fs/checkpoint.c | 4 ++++ >>>> fs/f2fs/f2fs.h | 1 + >>>> fs/f2fs/inode.c | 6 ++++++ >>>> 3 files changed, 11 insertions(+) >>>> >>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >>>> index bd890738b94d..ada2c548645c 100644 >>>> --- a/fs/f2fs/checkpoint.c >>>> +++ b/fs/f2fs/checkpoint.c >>>> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) >>>> f2fs_info(sbi, "orphan cleanup on readonly fs"); >>>> >>>> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>> >>> What about using SBI_POR_DOING? >> >> SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a >> separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow >> iget() of root_inode and all inodes during roll-forward recovery to do sanity >> check nlink w/ zero. What do you think? > > Can we do this sanity check after f2fs_iget in the f2fs_unlink() only? Sure, we need to cover f2fs_rename() as well, please check this: https://lore.kernel.org/all/67450f9a.050a0220.21d33d.0003.GAE@google.com Thanks, > >> >> Thanks, >> >>> >>>> + >>>> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); >>>> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); >>>> >>>> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>> } >>>> f2fs_put_page(page, 1); >>>> } >>>> + >>>> /* clear Orphan Flag */ >>>> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >>>> out: >>>> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>>> set_sbi_flag(sbi, SBI_IS_RECOVERED); >>>> >>>> return err; >>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>> index 05879c6dc4d6..1c75081c0c14 100644 >>>> --- a/fs/f2fs/f2fs.h >>>> +++ b/fs/f2fs/f2fs.h >>>> @@ -1322,6 +1322,7 @@ enum { >>>> SBI_IS_CLOSE, /* specify unmounting */ >>>> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ >>>> SBI_POR_DOING, /* recovery is doing or not */ >>>> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ >>>> SBI_NEED_SB_WRITE, /* need to recover superblock */ >>>> SBI_NEED_CP, /* need to checkpoint */ >>>> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ >>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >>>> index d6ad7810df69..02f1b69d03d8 100644 >>>> --- a/fs/f2fs/inode.c >>>> +++ b/fs/f2fs/inode.c >>>> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) >>>> } >>>> } >>>> >>>> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { >>>> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", >>>> + __func__, inode->i_ino); >>>> + return false; >>>> + } >>>> + >>>> return true; >>>> } >>>> >>>> -- >>>> 2.48.1.502.g6dc24dfdaf-goog ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-14 1:44 ` Chao Yu @ 2025-02-24 10:47 ` Chao Yu 2025-02-26 3:28 ` Jaegeuk Kim 0 siblings, 1 reply; 10+ messages in thread From: Chao Yu @ 2025-02-24 10:47 UTC (permalink / raw) To: Jaegeuk Kim Cc: chao, linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 2/14/25 09:44, Chao Yu wrote: > On 2/14/25 01:38, Jaegeuk Kim wrote: >> On 02/13, Chao Yu wrote: >>> On 2/13/25 00:47, Jaegeuk Kim wrote: >>>> On 02/12, Chao Yu wrote: >>>>> From: Leo Stone <leocstone@gmail.com> >>>>> >>>>> The syzbot reproducer mounts a f2fs image, then tries to unlink an >>>>> existing file. However, the unlinked file already has a link count of 0 >>>>> when it is read for the first time in do_read_inode(). >>>>> >>>>> Add a check to sanity_check_inode() for i_nlink == 0. >>>>> >>>>> [Chao Yu: rebase the code and fix orphan inode recovery issue] >>>>> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com >>>>> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 >>>>> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") >>>>> Signed-off-by: Leo Stone <leocstone@gmail.com> >>>>> Signed-off-by: Chao Yu <chao@kernel.org> >>>>> --- >>>>> fs/f2fs/checkpoint.c | 4 ++++ >>>>> fs/f2fs/f2fs.h | 1 + >>>>> fs/f2fs/inode.c | 6 ++++++ >>>>> 3 files changed, 11 insertions(+) >>>>> >>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >>>>> index bd890738b94d..ada2c548645c 100644 >>>>> --- a/fs/f2fs/checkpoint.c >>>>> +++ b/fs/f2fs/checkpoint.c >>>>> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>>> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) >>>>> f2fs_info(sbi, "orphan cleanup on readonly fs"); >>>>> >>>>> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>>> >>>> What about using SBI_POR_DOING? >>> >>> SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a >>> separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow >>> iget() of root_inode and all inodes during roll-forward recovery to do sanity >>> check nlink w/ zero. What do you think? >> >> Can we do this sanity check after f2fs_iget in the f2fs_unlink() only? > > Sure, we need to cover f2fs_rename() as well, please check this: > > https://lore.kernel.org/all/67450f9a.050a0220.21d33d.0003.GAE@google.com Hi Jaegeuk, I'm testing this, seems there is a problem, once we opened an inode that has zeroed nlink, in f2fs_evict_inode(), the inode and all its data will be deleted, then leaving its stale dir entry in parent directory. What do you think using v4? so that we may has chance to repair it w/ fsck rather than just deleting it? --- fs/f2fs/namei.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index a278c7da8177..949621bc0d07 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -547,6 +547,16 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) goto fail; } + if (unlikely(S_ISDIR(inode->i_mode) ? + inode->i_nlink <= 1 : inode->i_nlink == 0)) { + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", + __func__, inode->i_ino, inode->i_nlink, + S_ISDIR(inode->i_mode)); + err = -EFSCORRUPTED; + set_sbi_flag(sbi, SBI_NEED_FSCK); + goto fail; + } + err = f2fs_dquot_initialize(dir); if (err) goto fail; @@ -968,6 +978,15 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, } if (new_inode) { + if (unlikely(old_is_dir ? + new_inode->i_nlink <= 1 : new_inode->i_nlink == 0)) { + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", + __func__, new_inode->i_ino, new_inode->i_nlink, + S_ISDIR(new_inode->i_mode)); + err = -EFSCORRUPTED; + set_sbi_flag(sbi, SBI_NEED_FSCK); + goto out_dir; + } err = -ENOTEMPTY; if (old_is_dir && !f2fs_empty_dir(new_inode)) -- 2.48.1.601.g30ceb7b040-goog > > Thanks, > >> >>> >>> Thanks, >>> >>>> >>>>> + >>>>> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); >>>>> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); >>>>> >>>>> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>>> } >>>>> f2fs_put_page(page, 1); >>>>> } >>>>> + >>>>> /* clear Orphan Flag */ >>>>> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >>>>> out: >>>>> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>>>> set_sbi_flag(sbi, SBI_IS_RECOVERED); >>>>> >>>>> return err; >>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>>> index 05879c6dc4d6..1c75081c0c14 100644 >>>>> --- a/fs/f2fs/f2fs.h >>>>> +++ b/fs/f2fs/f2fs.h >>>>> @@ -1322,6 +1322,7 @@ enum { >>>>> SBI_IS_CLOSE, /* specify unmounting */ >>>>> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ >>>>> SBI_POR_DOING, /* recovery is doing or not */ >>>>> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ >>>>> SBI_NEED_SB_WRITE, /* need to recover superblock */ >>>>> SBI_NEED_CP, /* need to checkpoint */ >>>>> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ >>>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >>>>> index d6ad7810df69..02f1b69d03d8 100644 >>>>> --- a/fs/f2fs/inode.c >>>>> +++ b/fs/f2fs/inode.c >>>>> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) >>>>> } >>>>> } >>>>> >>>>> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { >>>>> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", >>>>> + __func__, inode->i_ino); >>>>> + return false; >>>>> + } >>>>> + >>>>> return true; >>>>> } >>>>> >>>>> -- >>>>> 2.48.1.502.g6dc24dfdaf-goog > ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-24 10:47 ` Chao Yu @ 2025-02-26 3:28 ` Jaegeuk Kim 2025-02-26 3:49 ` Chao Yu 0 siblings, 1 reply; 10+ messages in thread From: Jaegeuk Kim @ 2025-02-26 3:28 UTC (permalink / raw) To: Chao Yu Cc: linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 02/24, Chao Yu wrote: > On 2/14/25 09:44, Chao Yu wrote: > > On 2/14/25 01:38, Jaegeuk Kim wrote: > >> On 02/13, Chao Yu wrote: > >>> On 2/13/25 00:47, Jaegeuk Kim wrote: > >>>> On 02/12, Chao Yu wrote: > >>>>> From: Leo Stone <leocstone@gmail.com> > >>>>> > >>>>> The syzbot reproducer mounts a f2fs image, then tries to unlink an > >>>>> existing file. However, the unlinked file already has a link count of 0 > >>>>> when it is read for the first time in do_read_inode(). > >>>>> > >>>>> Add a check to sanity_check_inode() for i_nlink == 0. > >>>>> > >>>>> [Chao Yu: rebase the code and fix orphan inode recovery issue] > >>>>> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com > >>>>> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 > >>>>> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") > >>>>> Signed-off-by: Leo Stone <leocstone@gmail.com> > >>>>> Signed-off-by: Chao Yu <chao@kernel.org> > >>>>> --- > >>>>> fs/f2fs/checkpoint.c | 4 ++++ > >>>>> fs/f2fs/f2fs.h | 1 + > >>>>> fs/f2fs/inode.c | 6 ++++++ > >>>>> 3 files changed, 11 insertions(+) > >>>>> > >>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > >>>>> index bd890738b94d..ada2c548645c 100644 > >>>>> --- a/fs/f2fs/checkpoint.c > >>>>> +++ b/fs/f2fs/checkpoint.c > >>>>> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > >>>>> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) > >>>>> f2fs_info(sbi, "orphan cleanup on readonly fs"); > >>>>> > >>>>> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > >>>> > >>>> What about using SBI_POR_DOING? > >>> > >>> SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a > >>> separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow > >>> iget() of root_inode and all inodes during roll-forward recovery to do sanity > >>> check nlink w/ zero. What do you think? > >> > >> Can we do this sanity check after f2fs_iget in the f2fs_unlink() only? > > > > Sure, we need to cover f2fs_rename() as well, please check this: > > > > https://lore.kernel.org/all/67450f9a.050a0220.21d33d.0003.GAE@google.com > > Hi Jaegeuk, > > I'm testing this, seems there is a problem, once we opened an inode that > has zeroed nlink, in f2fs_evict_inode(), the inode and all its data will be > deleted, then leaving its stale dir entry in parent directory. > > What do you think using v4? so that we may has chance to repair it w/ fsck > rather than just deleting it? Do you mean v4 as the below change? > > --- > fs/f2fs/namei.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index a278c7da8177..949621bc0d07 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -547,6 +547,16 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) > goto fail; > } > > + if (unlikely(S_ISDIR(inode->i_mode) ? > + inode->i_nlink <= 1 : inode->i_nlink == 0)) { > + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", > + __func__, inode->i_ino, inode->i_nlink, > + S_ISDIR(inode->i_mode)); > + err = -EFSCORRUPTED; > + set_sbi_flag(sbi, SBI_NEED_FSCK); > + goto fail; > + } > + > err = f2fs_dquot_initialize(dir); > if (err) > goto fail; > @@ -968,6 +978,15 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, > } > > if (new_inode) { > + if (unlikely(old_is_dir ? > + new_inode->i_nlink <= 1 : new_inode->i_nlink == 0)) { > + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", > + __func__, new_inode->i_ino, new_inode->i_nlink, > + S_ISDIR(new_inode->i_mode)); > + err = -EFSCORRUPTED; > + set_sbi_flag(sbi, SBI_NEED_FSCK); > + goto out_dir; > + } > > err = -ENOTEMPTY; > if (old_is_dir && !f2fs_empty_dir(new_inode)) > -- > 2.48.1.601.g30ceb7b040-goog > > > > > Thanks, > > > >> > >>> > >>> Thanks, > >>> > >>>> > >>>>> + > >>>>> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); > >>>>> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); > >>>>> > >>>>> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > >>>>> } > >>>>> f2fs_put_page(page, 1); > >>>>> } > >>>>> + > >>>>> /* clear Orphan Flag */ > >>>>> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > >>>>> out: > >>>>> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > >>>>> set_sbi_flag(sbi, SBI_IS_RECOVERED); > >>>>> > >>>>> return err; > >>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >>>>> index 05879c6dc4d6..1c75081c0c14 100644 > >>>>> --- a/fs/f2fs/f2fs.h > >>>>> +++ b/fs/f2fs/f2fs.h > >>>>> @@ -1322,6 +1322,7 @@ enum { > >>>>> SBI_IS_CLOSE, /* specify unmounting */ > >>>>> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ > >>>>> SBI_POR_DOING, /* recovery is doing or not */ > >>>>> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ > >>>>> SBI_NEED_SB_WRITE, /* need to recover superblock */ > >>>>> SBI_NEED_CP, /* need to checkpoint */ > >>>>> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ > >>>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > >>>>> index d6ad7810df69..02f1b69d03d8 100644 > >>>>> --- a/fs/f2fs/inode.c > >>>>> +++ b/fs/f2fs/inode.c > >>>>> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) > >>>>> } > >>>>> } > >>>>> > >>>>> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { > >>>>> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", > >>>>> + __func__, inode->i_ino); > >>>>> + return false; > >>>>> + } > >>>>> + > >>>>> return true; > >>>>> } > >>>>> > >>>>> -- > >>>>> 2.48.1.502.g6dc24dfdaf-goog > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-26 3:28 ` Jaegeuk Kim @ 2025-02-26 3:49 ` Chao Yu 2025-02-26 17:15 ` Jaegeuk Kim 0 siblings, 1 reply; 10+ messages in thread From: Chao Yu @ 2025-02-26 3:49 UTC (permalink / raw) To: Jaegeuk Kim Cc: chao, linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 2/26/25 11:28, Jaegeuk Kim wrote: > On 02/24, Chao Yu wrote: >> On 2/14/25 09:44, Chao Yu wrote: >>> On 2/14/25 01:38, Jaegeuk Kim wrote: >>>> On 02/13, Chao Yu wrote: >>>>> On 2/13/25 00:47, Jaegeuk Kim wrote: >>>>>> On 02/12, Chao Yu wrote: >>>>>>> From: Leo Stone <leocstone@gmail.com> >>>>>>> >>>>>>> The syzbot reproducer mounts a f2fs image, then tries to unlink an >>>>>>> existing file. However, the unlinked file already has a link count of 0 >>>>>>> when it is read for the first time in do_read_inode(). >>>>>>> >>>>>>> Add a check to sanity_check_inode() for i_nlink == 0. >>>>>>> >>>>>>> [Chao Yu: rebase the code and fix orphan inode recovery issue] >>>>>>> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com >>>>>>> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 >>>>>>> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") >>>>>>> Signed-off-by: Leo Stone <leocstone@gmail.com> >>>>>>> Signed-off-by: Chao Yu <chao@kernel.org> >>>>>>> --- >>>>>>> fs/f2fs/checkpoint.c | 4 ++++ >>>>>>> fs/f2fs/f2fs.h | 1 + >>>>>>> fs/f2fs/inode.c | 6 ++++++ >>>>>>> 3 files changed, 11 insertions(+) >>>>>>> >>>>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >>>>>>> index bd890738b94d..ada2c548645c 100644 >>>>>>> --- a/fs/f2fs/checkpoint.c >>>>>>> +++ b/fs/f2fs/checkpoint.c >>>>>>> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>>>>> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) >>>>>>> f2fs_info(sbi, "orphan cleanup on readonly fs"); >>>>>>> >>>>>>> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>>>>> >>>>>> What about using SBI_POR_DOING? >>>>> >>>>> SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a >>>>> separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow >>>>> iget() of root_inode and all inodes during roll-forward recovery to do sanity >>>>> check nlink w/ zero. What do you think? >>>> >>>> Can we do this sanity check after f2fs_iget in the f2fs_unlink() only? >>> >>> Sure, we need to cover f2fs_rename() as well, please check this: >>> >>> https://lore.kernel.org/all/67450f9a.050a0220.21d33d.0003.GAE@google.com >> >> Hi Jaegeuk, >> >> I'm testing this, seems there is a problem, once we opened an inode that >> has zeroed nlink, in f2fs_evict_inode(), the inode and all its data will be >> deleted, then leaving its stale dir entry in parent directory. >> >> What do you think using v4? so that we may has chance to repair it w/ fsck >> rather than just deleting it? > > Do you mean v4 as the below change? I mean this v4 patch: --- fs/f2fs/checkpoint.c | 4 ++++ fs/f2fs/f2fs.h | 1 + fs/f2fs/inode.c | 6 ++++++ 3 files changed, 11 insertions(+) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index bd890738b94d..ada2c548645c 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) f2fs_info(sbi, "orphan cleanup on readonly fs"); + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); + start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) } f2fs_put_page(page, 1); } + /* clear Orphan Flag */ clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); out: + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); set_sbi_flag(sbi, SBI_IS_RECOVERED); return err; diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 05879c6dc4d6..1c75081c0c14 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -1322,6 +1322,7 @@ enum { SBI_IS_CLOSE, /* specify unmounting */ SBI_NEED_FSCK, /* need fsck.f2fs to fix */ SBI_POR_DOING, /* recovery is doing or not */ + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ SBI_NEED_SB_WRITE, /* need to recover superblock */ SBI_NEED_CP, /* need to checkpoint */ SBI_IS_SHUTDOWN, /* shutdown by ioctl */ diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c index d6ad7810df69..02f1b69d03d8 100644 --- a/fs/f2fs/inode.c +++ b/fs/f2fs/inode.c @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) } } + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", + __func__, inode->i_ino); + return false; + } + return true; } Thanks, > >> >> --- >> fs/f2fs/namei.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >> index a278c7da8177..949621bc0d07 100644 >> --- a/fs/f2fs/namei.c >> +++ b/fs/f2fs/namei.c >> @@ -547,6 +547,16 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) >> goto fail; >> } >> >> + if (unlikely(S_ISDIR(inode->i_mode) ? >> + inode->i_nlink <= 1 : inode->i_nlink == 0)) { >> + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", >> + __func__, inode->i_ino, inode->i_nlink, >> + S_ISDIR(inode->i_mode)); >> + err = -EFSCORRUPTED; >> + set_sbi_flag(sbi, SBI_NEED_FSCK); >> + goto fail; >> + } >> + >> err = f2fs_dquot_initialize(dir); >> if (err) >> goto fail; >> @@ -968,6 +978,15 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, >> } >> >> if (new_inode) { >> + if (unlikely(old_is_dir ? >> + new_inode->i_nlink <= 1 : new_inode->i_nlink == 0)) { >> + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", >> + __func__, new_inode->i_ino, new_inode->i_nlink, >> + S_ISDIR(new_inode->i_mode)); >> + err = -EFSCORRUPTED; >> + set_sbi_flag(sbi, SBI_NEED_FSCK); >> + goto out_dir; >> + } >> >> err = -ENOTEMPTY; >> if (old_is_dir && !f2fs_empty_dir(new_inode)) >> -- >> 2.48.1.601.g30ceb7b040-goog >> >>> >>> Thanks, >>> >>>> >>>>> >>>>> Thanks, >>>>> >>>>>> >>>>>>> + >>>>>>> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); >>>>>>> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); >>>>>>> >>>>>>> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>>>>> } >>>>>>> f2fs_put_page(page, 1); >>>>>>> } >>>>>>> + >>>>>>> /* clear Orphan Flag */ >>>>>>> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >>>>>>> out: >>>>>>> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>>>>>> set_sbi_flag(sbi, SBI_IS_RECOVERED); >>>>>>> >>>>>>> return err; >>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>>>>> index 05879c6dc4d6..1c75081c0c14 100644 >>>>>>> --- a/fs/f2fs/f2fs.h >>>>>>> +++ b/fs/f2fs/f2fs.h >>>>>>> @@ -1322,6 +1322,7 @@ enum { >>>>>>> SBI_IS_CLOSE, /* specify unmounting */ >>>>>>> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ >>>>>>> SBI_POR_DOING, /* recovery is doing or not */ >>>>>>> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ >>>>>>> SBI_NEED_SB_WRITE, /* need to recover superblock */ >>>>>>> SBI_NEED_CP, /* need to checkpoint */ >>>>>>> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ >>>>>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >>>>>>> index d6ad7810df69..02f1b69d03d8 100644 >>>>>>> --- a/fs/f2fs/inode.c >>>>>>> +++ b/fs/f2fs/inode.c >>>>>>> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) >>>>>>> } >>>>>>> } >>>>>>> >>>>>>> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { >>>>>>> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", >>>>>>> + __func__, inode->i_ino); >>>>>>> + return false; >>>>>>> + } >>>>>>> + >>>>>>> return true; >>>>>>> } >>>>>>> >>>>>>> -- >>>>>>> 2.48.1.502.g6dc24dfdaf-goog >>> ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-26 3:49 ` Chao Yu @ 2025-02-26 17:15 ` Jaegeuk Kim 2025-02-28 1:49 ` Chao Yu 0 siblings, 1 reply; 10+ messages in thread From: Jaegeuk Kim @ 2025-02-26 17:15 UTC (permalink / raw) To: Chao Yu Cc: linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 02/26, Chao Yu wrote: > On 2/26/25 11:28, Jaegeuk Kim wrote: > > On 02/24, Chao Yu wrote: > >> On 2/14/25 09:44, Chao Yu wrote: > >>> On 2/14/25 01:38, Jaegeuk Kim wrote: > >>>> On 02/13, Chao Yu wrote: > >>>>> On 2/13/25 00:47, Jaegeuk Kim wrote: > >>>>>> On 02/12, Chao Yu wrote: > >>>>>>> From: Leo Stone <leocstone@gmail.com> > >>>>>>> > >>>>>>> The syzbot reproducer mounts a f2fs image, then tries to unlink an > >>>>>>> existing file. However, the unlinked file already has a link count of 0 > >>>>>>> when it is read for the first time in do_read_inode(). > >>>>>>> > >>>>>>> Add a check to sanity_check_inode() for i_nlink == 0. > >>>>>>> > >>>>>>> [Chao Yu: rebase the code and fix orphan inode recovery issue] > >>>>>>> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com > >>>>>>> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 > >>>>>>> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") > >>>>>>> Signed-off-by: Leo Stone <leocstone@gmail.com> > >>>>>>> Signed-off-by: Chao Yu <chao@kernel.org> > >>>>>>> --- > >>>>>>> fs/f2fs/checkpoint.c | 4 ++++ > >>>>>>> fs/f2fs/f2fs.h | 1 + > >>>>>>> fs/f2fs/inode.c | 6 ++++++ > >>>>>>> 3 files changed, 11 insertions(+) > >>>>>>> > >>>>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > >>>>>>> index bd890738b94d..ada2c548645c 100644 > >>>>>>> --- a/fs/f2fs/checkpoint.c > >>>>>>> +++ b/fs/f2fs/checkpoint.c > >>>>>>> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > >>>>>>> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) > >>>>>>> f2fs_info(sbi, "orphan cleanup on readonly fs"); > >>>>>>> > >>>>>>> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > >>>>>> > >>>>>> What about using SBI_POR_DOING? > >>>>> > >>>>> SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a > >>>>> separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow > >>>>> iget() of root_inode and all inodes during roll-forward recovery to do sanity > >>>>> check nlink w/ zero. What do you think? > >>>> > >>>> Can we do this sanity check after f2fs_iget in the f2fs_unlink() only? > >>> > >>> Sure, we need to cover f2fs_rename() as well, please check this: > >>> > >>> https://lore.kernel.org/all/67450f9a.050a0220.21d33d.0003.GAE@google.com > >> > >> Hi Jaegeuk, > >> > >> I'm testing this, seems there is a problem, once we opened an inode that > >> has zeroed nlink, in f2fs_evict_inode(), the inode and all its data will be > >> deleted, then leaving its stale dir entry in parent directory. > >> > >> What do you think using v4? so that we may has chance to repair it w/ fsck > >> rather than just deleting it? > > > > Do you mean v4 as the below change? > > I mean this v4 patch: What about checking i_nlink right after f2fs_iget in f2fs_lookup? > > --- > fs/f2fs/checkpoint.c | 4 ++++ > fs/f2fs/f2fs.h | 1 + > fs/f2fs/inode.c | 6 ++++++ > 3 files changed, 11 insertions(+) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index bd890738b94d..ada2c548645c 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) > f2fs_info(sbi, "orphan cleanup on readonly fs"); > > + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > + > start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); > orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); > > @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > } > f2fs_put_page(page, 1); > } > + > /* clear Orphan Flag */ > clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > out: > + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > set_sbi_flag(sbi, SBI_IS_RECOVERED); > > return err; > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 05879c6dc4d6..1c75081c0c14 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -1322,6 +1322,7 @@ enum { > SBI_IS_CLOSE, /* specify unmounting */ > SBI_NEED_FSCK, /* need fsck.f2fs to fix */ > SBI_POR_DOING, /* recovery is doing or not */ > + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ > SBI_NEED_SB_WRITE, /* need to recover superblock */ > SBI_NEED_CP, /* need to checkpoint */ > SBI_IS_SHUTDOWN, /* shutdown by ioctl */ > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index d6ad7810df69..02f1b69d03d8 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) > } > } > > + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { > + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", > + __func__, inode->i_ino); > + return false; > + } > + > return true; > } > > Thanks, > > > > >> > >> --- > >> fs/f2fs/namei.c | 19 +++++++++++++++++++ > >> 1 file changed, 19 insertions(+) > >> > >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > >> index a278c7da8177..949621bc0d07 100644 > >> --- a/fs/f2fs/namei.c > >> +++ b/fs/f2fs/namei.c > >> @@ -547,6 +547,16 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) > >> goto fail; > >> } > >> > >> + if (unlikely(S_ISDIR(inode->i_mode) ? > >> + inode->i_nlink <= 1 : inode->i_nlink == 0)) { > >> + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", > >> + __func__, inode->i_ino, inode->i_nlink, > >> + S_ISDIR(inode->i_mode)); > >> + err = -EFSCORRUPTED; > >> + set_sbi_flag(sbi, SBI_NEED_FSCK); > >> + goto fail; > >> + } > >> + > >> err = f2fs_dquot_initialize(dir); > >> if (err) > >> goto fail; > >> @@ -968,6 +978,15 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, > >> } > >> > >> if (new_inode) { > >> + if (unlikely(old_is_dir ? > >> + new_inode->i_nlink <= 1 : new_inode->i_nlink == 0)) { > >> + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", > >> + __func__, new_inode->i_ino, new_inode->i_nlink, > >> + S_ISDIR(new_inode->i_mode)); > >> + err = -EFSCORRUPTED; > >> + set_sbi_flag(sbi, SBI_NEED_FSCK); > >> + goto out_dir; > >> + } > >> > >> err = -ENOTEMPTY; > >> if (old_is_dir && !f2fs_empty_dir(new_inode)) > >> -- > >> 2.48.1.601.g30ceb7b040-goog > >> > >>> > >>> Thanks, > >>> > >>>> > >>>>> > >>>>> Thanks, > >>>>> > >>>>>> > >>>>>>> + > >>>>>>> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); > >>>>>>> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); > >>>>>>> > >>>>>>> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) > >>>>>>> } > >>>>>>> f2fs_put_page(page, 1); > >>>>>>> } > >>>>>>> + > >>>>>>> /* clear Orphan Flag */ > >>>>>>> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > >>>>>>> out: > >>>>>>> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); > >>>>>>> set_sbi_flag(sbi, SBI_IS_RECOVERED); > >>>>>>> > >>>>>>> return err; > >>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > >>>>>>> index 05879c6dc4d6..1c75081c0c14 100644 > >>>>>>> --- a/fs/f2fs/f2fs.h > >>>>>>> +++ b/fs/f2fs/f2fs.h > >>>>>>> @@ -1322,6 +1322,7 @@ enum { > >>>>>>> SBI_IS_CLOSE, /* specify unmounting */ > >>>>>>> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ > >>>>>>> SBI_POR_DOING, /* recovery is doing or not */ > >>>>>>> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ > >>>>>>> SBI_NEED_SB_WRITE, /* need to recover superblock */ > >>>>>>> SBI_NEED_CP, /* need to checkpoint */ > >>>>>>> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ > >>>>>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > >>>>>>> index d6ad7810df69..02f1b69d03d8 100644 > >>>>>>> --- a/fs/f2fs/inode.c > >>>>>>> +++ b/fs/f2fs/inode.c > >>>>>>> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) > >>>>>>> } > >>>>>>> } > >>>>>>> > >>>>>>> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { > >>>>>>> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", > >>>>>>> + __func__, inode->i_ino); > >>>>>>> + return false; > >>>>>>> + } > >>>>>>> + > >>>>>>> return true; > >>>>>>> } > >>>>>>> > >>>>>>> -- > >>>>>>> 2.48.1.502.g6dc24dfdaf-goog > >>> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4] f2fs: add check for deleted inode 2025-02-26 17:15 ` Jaegeuk Kim @ 2025-02-28 1:49 ` Chao Yu 0 siblings, 0 replies; 10+ messages in thread From: Chao Yu @ 2025-02-28 1:49 UTC (permalink / raw) To: Jaegeuk Kim Cc: chao, linux-f2fs-devel, linux-kernel, Leo Stone, syzbot+b01a36acd7007e273a83 On 2025/2/27 1:15, Jaegeuk Kim wrote: > On 02/26, Chao Yu wrote: >> On 2/26/25 11:28, Jaegeuk Kim wrote: >>> On 02/24, Chao Yu wrote: >>>> On 2/14/25 09:44, Chao Yu wrote: >>>>> On 2/14/25 01:38, Jaegeuk Kim wrote: >>>>>> On 02/13, Chao Yu wrote: >>>>>>> On 2/13/25 00:47, Jaegeuk Kim wrote: >>>>>>>> On 02/12, Chao Yu wrote: >>>>>>>>> From: Leo Stone <leocstone@gmail.com> >>>>>>>>> >>>>>>>>> The syzbot reproducer mounts a f2fs image, then tries to unlink an >>>>>>>>> existing file. However, the unlinked file already has a link count of 0 >>>>>>>>> when it is read for the first time in do_read_inode(). >>>>>>>>> >>>>>>>>> Add a check to sanity_check_inode() for i_nlink == 0. >>>>>>>>> >>>>>>>>> [Chao Yu: rebase the code and fix orphan inode recovery issue] >>>>>>>>> Reported-by: syzbot+b01a36acd7007e273a83@syzkaller.appspotmail.com >>>>>>>>> Closes: https://syzkaller.appspot.com/bug?extid=b01a36acd7007e273a83 >>>>>>>>> Fixes: 39a53e0ce0df ("f2fs: add superblock and major in-memory structure") >>>>>>>>> Signed-off-by: Leo Stone <leocstone@gmail.com> >>>>>>>>> Signed-off-by: Chao Yu <chao@kernel.org> >>>>>>>>> --- >>>>>>>>> fs/f2fs/checkpoint.c | 4 ++++ >>>>>>>>> fs/f2fs/f2fs.h | 1 + >>>>>>>>> fs/f2fs/inode.c | 6 ++++++ >>>>>>>>> 3 files changed, 11 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >>>>>>>>> index bd890738b94d..ada2c548645c 100644 >>>>>>>>> --- a/fs/f2fs/checkpoint.c >>>>>>>>> +++ b/fs/f2fs/checkpoint.c >>>>>>>>> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>>>>>>> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) >>>>>>>>> f2fs_info(sbi, "orphan cleanup on readonly fs"); >>>>>>>>> >>>>>>>>> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>>>>>>> >>>>>>>> What about using SBI_POR_DOING? >>>>>>> >>>>>>> SBI_POR_DOING will cover most flow of f2fs_fill_super(), I think we can add a >>>>>>> separated flag just covering f2fs_recover_orphan_inodes(), so that we can allow >>>>>>> iget() of root_inode and all inodes during roll-forward recovery to do sanity >>>>>>> check nlink w/ zero. What do you think? >>>>>> >>>>>> Can we do this sanity check after f2fs_iget in the f2fs_unlink() only? >>>>> >>>>> Sure, we need to cover f2fs_rename() as well, please check this: >>>>> >>>>> https://lore.kernel.org/all/67450f9a.050a0220.21d33d.0003.GAE@google.com >>>> >>>> Hi Jaegeuk, >>>> >>>> I'm testing this, seems there is a problem, once we opened an inode that >>>> has zeroed nlink, in f2fs_evict_inode(), the inode and all its data will be >>>> deleted, then leaving its stale dir entry in parent directory. >>>> >>>> What do you think using v4? so that we may has chance to repair it w/ fsck >>>> rather than just deleting it? >>> >>> Do you mean v4 as the below change? >> >> I mean this v4 patch: > > What about checking i_nlink right after f2fs_iget in f2fs_lookup? Fine to me, I've updated in v5. Thanks, > >> >> --- >> fs/f2fs/checkpoint.c | 4 ++++ >> fs/f2fs/f2fs.h | 1 + >> fs/f2fs/inode.c | 6 ++++++ >> 3 files changed, 11 insertions(+) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index bd890738b94d..ada2c548645c 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -751,6 +751,8 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >> if (is_sbi_flag_set(sbi, SBI_IS_WRITABLE)) >> f2fs_info(sbi, "orphan cleanup on readonly fs"); >> >> + set_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >> + >> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); >> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); >> >> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >> } >> f2fs_put_page(page, 1); >> } >> + >> /* clear Orphan Flag */ >> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >> out: >> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >> set_sbi_flag(sbi, SBI_IS_RECOVERED); >> >> return err; >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 05879c6dc4d6..1c75081c0c14 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -1322,6 +1322,7 @@ enum { >> SBI_IS_CLOSE, /* specify unmounting */ >> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ >> SBI_POR_DOING, /* recovery is doing or not */ >> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ >> SBI_NEED_SB_WRITE, /* need to recover superblock */ >> SBI_NEED_CP, /* need to checkpoint */ >> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ >> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >> index d6ad7810df69..02f1b69d03d8 100644 >> --- a/fs/f2fs/inode.c >> +++ b/fs/f2fs/inode.c >> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) >> } >> } >> >> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { >> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", >> + __func__, inode->i_ino); >> + return false; >> + } >> + >> return true; >> } >> >> Thanks, >> >>> >>>> >>>> --- >>>> fs/f2fs/namei.c | 19 +++++++++++++++++++ >>>> 1 file changed, 19 insertions(+) >>>> >>>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >>>> index a278c7da8177..949621bc0d07 100644 >>>> --- a/fs/f2fs/namei.c >>>> +++ b/fs/f2fs/namei.c >>>> @@ -547,6 +547,16 @@ static int f2fs_unlink(struct inode *dir, struct dentry *dentry) >>>> goto fail; >>>> } >>>> >>>> + if (unlikely(S_ISDIR(inode->i_mode) ? >>>> + inode->i_nlink <= 1 : inode->i_nlink == 0)) { >>>> + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", >>>> + __func__, inode->i_ino, inode->i_nlink, >>>> + S_ISDIR(inode->i_mode)); >>>> + err = -EFSCORRUPTED; >>>> + set_sbi_flag(sbi, SBI_NEED_FSCK); >>>> + goto fail; >>>> + } >>>> + >>>> err = f2fs_dquot_initialize(dir); >>>> if (err) >>>> goto fail; >>>> @@ -968,6 +978,15 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct inode *old_dir, >>>> } >>>> >>>> if (new_inode) { >>>> + if (unlikely(old_is_dir ? >>>> + new_inode->i_nlink <= 1 : new_inode->i_nlink == 0)) { >>>> + f2fs_err_ratelimited(sbi, "%s: inode (ino=%lx) has inconsistent nlink: %u, isdir: %d", >>>> + __func__, new_inode->i_ino, new_inode->i_nlink, >>>> + S_ISDIR(new_inode->i_mode)); >>>> + err = -EFSCORRUPTED; >>>> + set_sbi_flag(sbi, SBI_NEED_FSCK); >>>> + goto out_dir; >>>> + } >>>> >>>> err = -ENOTEMPTY; >>>> if (old_is_dir && !f2fs_empty_dir(new_inode)) >>>> -- >>>> 2.48.1.601.g30ceb7b040-goog >>>> >>>>> >>>>> Thanks, >>>>> >>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>>> >>>>>>>>> + >>>>>>>>> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); >>>>>>>>> orphan_blocks = __start_sum_addr(sbi) - 1 - __cp_payload(sbi); >>>>>>>>> >>>>>>>>> @@ -778,9 +780,11 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi) >>>>>>>>> } >>>>>>>>> f2fs_put_page(page, 1); >>>>>>>>> } >>>>>>>>> + >>>>>>>>> /* clear Orphan Flag */ >>>>>>>>> clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >>>>>>>>> out: >>>>>>>>> + clear_sbi_flag(sbi, SBI_ORPHAN_RECOVERY); >>>>>>>>> set_sbi_flag(sbi, SBI_IS_RECOVERED); >>>>>>>>> >>>>>>>>> return err; >>>>>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >>>>>>>>> index 05879c6dc4d6..1c75081c0c14 100644 >>>>>>>>> --- a/fs/f2fs/f2fs.h >>>>>>>>> +++ b/fs/f2fs/f2fs.h >>>>>>>>> @@ -1322,6 +1322,7 @@ enum { >>>>>>>>> SBI_IS_CLOSE, /* specify unmounting */ >>>>>>>>> SBI_NEED_FSCK, /* need fsck.f2fs to fix */ >>>>>>>>> SBI_POR_DOING, /* recovery is doing or not */ >>>>>>>>> + SBI_ORPHAN_RECOVERY, /* orphan inodes recovery is doing */ >>>>>>>>> SBI_NEED_SB_WRITE, /* need to recover superblock */ >>>>>>>>> SBI_NEED_CP, /* need to checkpoint */ >>>>>>>>> SBI_IS_SHUTDOWN, /* shutdown by ioctl */ >>>>>>>>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c >>>>>>>>> index d6ad7810df69..02f1b69d03d8 100644 >>>>>>>>> --- a/fs/f2fs/inode.c >>>>>>>>> +++ b/fs/f2fs/inode.c >>>>>>>>> @@ -386,6 +386,12 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page) >>>>>>>>> } >>>>>>>>> } >>>>>>>>> >>>>>>>>> + if (inode->i_nlink == 0 && !is_sbi_flag_set(sbi, SBI_ORPHAN_RECOVERY)) { >>>>>>>>> + f2fs_warn(sbi, "%s: inode (ino=%lx) has a link count of 0", >>>>>>>>> + __func__, inode->i_ino); >>>>>>>>> + return false; >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> return true; >>>>>>>>> } >>>>>>>>> >>>>>>>>> -- >>>>>>>>> 2.48.1.502.g6dc24dfdaf-goog >>>>> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-02-28 1:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-12 7:27 [PATCH v4] f2fs: add check for deleted inode Chao Yu 2025-02-12 16:47 ` Jaegeuk Kim 2025-02-13 2:00 ` Chao Yu 2025-02-13 17:38 ` Jaegeuk Kim 2025-02-14 1:44 ` Chao Yu 2025-02-24 10:47 ` Chao Yu 2025-02-26 3:28 ` Jaegeuk Kim 2025-02-26 3:49 ` Chao Yu 2025-02-26 17:15 ` Jaegeuk Kim 2025-02-28 1:49 ` Chao Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox