lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ