[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87ms2zgt4n.fsf@mail.parknet.co.jp>
Date: Wed, 31 Dec 2025 13:41:44 +0900
From: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To: Zhiyu Zhang <zhiyuzhang999@...il.com>
Cc: viro@...iv.linux.org.uk, brauner@...nel.org, Jan Kara <jack@...e.cz>,
linux-fsdevel@...r.kernel.org, syzkaller <syzkaller@...glegroups.com>,
linux-kernel@...r.kernel.org
Subject: Re: [Kernel Bug] WARNING in vfat_rmdir
Zhiyu Zhang <zhiyuzhang999@...il.com> writes:
> Dear Linux kernel developers and maintainers,
>
> I’m sorry — the root cause analysis in my previous report was likely
> incorrect, and the patch I suggested there does not alleviate the
> issue after further testing, which means that the root cause is not on
> the errno passing.
>
> After adding debug prints in fat__get_entry(), I observed frequent
> cases of err=0, phys=0 at pos == i_size, which means the code is
> taking a "normal EOF" path (as decided by fat_bmap()) rather than
> hitting and swallowing a negative errno. As a result,
> fat_get_short_entry() still returns -ENOENT, fat_dir_empty() still
> returns 0, and the code path does not prevent drop_nlink(dir) from
> being executed even when the parent directory's i_nlink is already
> abnormal. In other words, the parent directory's i_nlink appears to be
> wrong/corrupted in the first place. Subsequent vfat_rmdir() calls then
> decrement the already-too-low link count, eventually reaching 0 and
> triggering WARN_ON(inode->i_nlink == 0) in drop_nlink() (and panicking
> if panic_on_warn is enabled).
>
> So please IGNORE my previous patch proposal. A conservative mitigation
> that I tested EFFECTIVE is similar to the UDF fix for corrupted parent
> link count handling (Jan Kara's WARNING in udf_rmdir fix:
> "https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c5566903af56dd1abb092f18dcb0c770d6cd8dcb").
It can occur on a corrupted image that didn't correctly initialized as a
directory.
> static int vfat_rmdir(struct inode *dir, struct dentry *dentry)
> err = fat_remove_entries(dir, &sinfo); /* and releases bh */
> if (err)
> goto out;
> - drop_nlink(dir);
> + if (dir->i_nlink >= 3)
> + drop_nlink(dir);
> + else
> + fat_fs_error_ratelimit(sb, "parent dir link count too
> low (%u)\n",
> + dir->i_nlink);
Looks good sanity check. However, I think it doesn't need
_ratelimit. And this should be done in msdos_namei.c too.
And fat_fs_error() should remove last "\n", and please add {} for 2
lines code (it is not necessary as c though, sorry for kind of my
preference), also looks like no "sb" here, need testing before actually
submitting the patch.
else {
fat_fs_error(sb, "parent dir link count too low (%u)",
dir->i_nlink);
}
Thanks.
> clear_nlink(inode);
> fat_truncate_time(inode, NULL, S_ATIME|S_MTIME);
--
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Powered by blists - more mailing lists