* [PATCH] f2fs: reject invalid recovered filename lengths
@ 2026-07-29 11:41 Wenjie Qi
2026-08-03 8:52 ` Chao Yu
0 siblings, 1 reply; 3+ messages in thread
From: Wenjie Qi @ 2026-07-29 11:41 UTC (permalink / raw)
To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-kernel, qiwenjie, qwjhust
Recovery uses raw_inode->i_namelen directly when rebuilding fsynced
dentries. A zero-length name uses no dentry slots, so recovery can
report success without recreating the dentry.
Treat zero-length and oversized recovered names as corruption, mark
NEED_FSCK, and stop recovery with -EFSCORRUPTED.
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
fs/f2fs/recovery.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 89af8407b667..70cf99efcb3c 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -126,8 +126,8 @@ static int init_recovered_filename(const struct inode *dir,
fname->disk_name.len = le32_to_cpu(raw_inode->i_namelen);
fname->disk_name.name = raw_inode->i_name;
- if (WARN_ON(fname->disk_name.len > F2FS_NAME_LEN))
- return -ENAMETOOLONG;
+ if (!fname->disk_name.len || fname->disk_name.len > F2FS_NAME_LEN)
+ return -EFSCORRUPTED;
if (!IS_ENCRYPTED(dir)) {
usr_fname->name = fname->disk_name.name;
@@ -185,8 +185,16 @@ static int recover_dentry(struct inode *inode, struct folio *ifolio,
dir = entry->inode;
err = init_recovered_filename(dir, raw_inode, &fname, &usr_fname);
- if (err)
+ if (err) {
+ if (err == -EFSCORRUPTED) {
+ f2fs_err(F2FS_I_SB(inode),
+ "invalid recovered filename length %u for ino %llu",
+ le32_to_cpu(raw_inode->i_namelen), inode->i_ino);
+ set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
+ f2fs_handle_error(F2FS_I_SB(inode), ERROR_CORRUPTED_INODE);
+ }
goto out;
+ }
retry:
de = __f2fs_find_entry(dir, &fname, &folio);
if (de && inode->i_ino == le32_to_cpu(de->ino))
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] f2fs: reject invalid recovered filename lengths
2026-07-29 11:41 [PATCH] f2fs: reject invalid recovered filename lengths Wenjie Qi
@ 2026-08-03 8:52 ` Chao Yu
2026-08-03 11:41 ` Wenjie Qi
0 siblings, 1 reply; 3+ messages in thread
From: Chao Yu @ 2026-08-03 8:52 UTC (permalink / raw)
To: Wenjie Qi, jaegeuk; +Cc: chao, linux-f2fs-devel, linux-kernel, qiwenjie
On 7/29/26 19:41, Wenjie Qi wrote:
> Recovery uses raw_inode->i_namelen directly when rebuilding fsynced
> dentries. A zero-length name uses no dentry slots, so recovery can
> report success without recreating the dentry.
>
> Treat zero-length and oversized recovered names as corruption, mark
> NEED_FSCK, and stop recovery with -EFSCORRUPTED.
Do you have a reproducer for this? maybe using inject.f2fs? It will be
better to cover those bugs w/ xfsqa testcase, could you please consider
to add it?
>
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> ---
> fs/f2fs/recovery.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index 89af8407b667..70cf99efcb3c 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -126,8 +126,8 @@ static int init_recovered_filename(const struct inode *dir,
> fname->disk_name.len = le32_to_cpu(raw_inode->i_namelen);
> fname->disk_name.name = raw_inode->i_name;
>
> - if (WARN_ON(fname->disk_name.len > F2FS_NAME_LEN))
> - return -ENAMETOOLONG;
> + if (!fname->disk_name.len || fname->disk_name.len > F2FS_NAME_LEN)
unlikely?
relocate error handling here?
Thanks,
> + return -EFSCORRUPTED;
>
> if (!IS_ENCRYPTED(dir)) {
> usr_fname->name = fname->disk_name.name;
> @@ -185,8 +185,16 @@ static int recover_dentry(struct inode *inode, struct folio *ifolio,
>
> dir = entry->inode;
> err = init_recovered_filename(dir, raw_inode, &fname, &usr_fname);
> - if (err)
> + if (err) {
> + if (err == -EFSCORRUPTED) {
> + f2fs_err(F2FS_I_SB(inode),
> + "invalid recovered filename length %u for ino %llu",
> + le32_to_cpu(raw_inode->i_namelen), inode->i_ino);
> + set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> + f2fs_handle_error(F2FS_I_SB(inode), ERROR_CORRUPTED_INODE);
> + }
> goto out;
> + }
> retry:
> de = __f2fs_find_entry(dir, &fname, &folio);
> if (de && inode->i_ino == le32_to_cpu(de->ino))
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] f2fs: reject invalid recovered filename lengths
2026-08-03 8:52 ` Chao Yu
@ 2026-08-03 11:41 ` Wenjie Qi
0 siblings, 0 replies; 3+ messages in thread
From: Wenjie Qi @ 2026-08-03 11:41 UTC (permalink / raw)
To: Chao Yu; +Cc: jaegeuk, linux-f2fs-devel, linux-kernel, qiwenjie
Yes, I have a reproducer.
This one is not naturally triggered by inject.f2fs, since the bug
comes
from malformed on-disk recovery metadata rather than a runtime fault
site.
The reproducer I used is:
1. create a file with a 255-byte name;
2. fsync the file only, without fsyncing the parent directory;
3. crash before checkpoint so the fsynced inode stays in roll-forward data;
4. patch the recovery log entry on the image and change i_namelen from
255 to 0;
5. mount and let recovery run.
On current code, recovery reports success, but the fsynced file is
not
recreated after mount.
I agree this would be better covered by an xfstests case. I can look at
turning the reproducer into one, though it needs an image-corruption
step between crash and remount, so it is a bit different from
inject.f2fs-based tests.
For the code, I can add unlikely() to the zero/oversized length check in
the next version.
On Mon, Aug 3, 2026 at 4:53 PM Chao Yu <chao@kernel.org> wrote:
>
> On 7/29/26 19:41, Wenjie Qi wrote:
> > Recovery uses raw_inode->i_namelen directly when rebuilding fsynced
> > dentries. A zero-length name uses no dentry slots, so recovery can
> > report success without recreating the dentry.
> >
> > Treat zero-length and oversized recovered names as corruption, mark
> > NEED_FSCK, and stop recovery with -EFSCORRUPTED.
>
> Do you have a reproducer for this? maybe using inject.f2fs? It will be
> better to cover those bugs w/ xfsqa testcase, could you please consider
> to add it?
>
> >
> > Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> > ---
> > fs/f2fs/recovery.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> > index 89af8407b667..70cf99efcb3c 100644
> > --- a/fs/f2fs/recovery.c
> > +++ b/fs/f2fs/recovery.c
> > @@ -126,8 +126,8 @@ static int init_recovered_filename(const struct inode *dir,
> > fname->disk_name.len = le32_to_cpu(raw_inode->i_namelen);
> > fname->disk_name.name = raw_inode->i_name;
> >
> > - if (WARN_ON(fname->disk_name.len > F2FS_NAME_LEN))
> > - return -ENAMETOOLONG;
> > + if (!fname->disk_name.len || fname->disk_name.len > F2FS_NAME_LEN)
>
> unlikely?
>
> relocate error handling here?
>
> Thanks,
>
> > + return -EFSCORRUPTED;
> >
> > if (!IS_ENCRYPTED(dir)) {
> > usr_fname->name = fname->disk_name.name;
> > @@ -185,8 +185,16 @@ static int recover_dentry(struct inode *inode, struct folio *ifolio,
> >
> > dir = entry->inode;
> > err = init_recovered_filename(dir, raw_inode, &fname, &usr_fname);
> > - if (err)
> > + if (err) {
> > + if (err == -EFSCORRUPTED) {
> > + f2fs_err(F2FS_I_SB(inode),
> > + "invalid recovered filename length %u for ino %llu",
> > + le32_to_cpu(raw_inode->i_namelen), inode->i_ino);
> > + set_sbi_flag(F2FS_I_SB(inode), SBI_NEED_FSCK);
> > + f2fs_handle_error(F2FS_I_SB(inode), ERROR_CORRUPTED_INODE);
> > + }
> > goto out;
> > + }
> > retry:
> > de = __f2fs_find_entry(dir, &fname, &folio);
> > if (de && inode->i_ino == le32_to_cpu(de->ino))
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-03 11:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 11:41 [PATCH] f2fs: reject invalid recovered filename lengths Wenjie Qi
2026-08-03 8:52 ` Chao Yu
2026-08-03 11:41 ` Wenjie Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox