linux-kernel - Re: [PATCH] vfat: fix uninitialized i

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87jz2zz1t9.fsf@mail.parknet.co.jp>
Date: Wed, 20 Aug 2025 01:57:54 +0900
From: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
To: zhoumin <teczm@...mail.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfat: fix uninitialized i_pos error

zhoumin <teczm@...mail.com> writes:

>>> The i_pos field remains uninitialized when fat_fs_error_ratelimit() reports
>>> error, e.g.,
>>>
>>> 	[ 1642.703550] FAT-fs (loop0): error, fat_get_cluster: invalid
>>> 	cluster chain (i_pos 0)
>>>
>>> Since i_pos is assigned in fat_attach after fat_fill_inode, the error
>>> message lacks useful debug info.
>>>
>>> Path:
>>> vfat_lookup
>>> 	fat_build_inode
>>> 		fat_fill_inode
>>> 			fat_calc_dir_size
>>>           			fat_get_cluster /* report error */
>>>       	fat_attach  /* i_pos assigned here */
> 
>> No. It is initialized as 0, and it must be unavailable outside
>> between fat_attach and fat_detach.
> 
> I see that it was initialized to 0. What I meant is that when
> fat_fs_error_ratelimit uses i_pos for debugging output, it doesn't get the
> correct value.

Not big issue. IOW, 0 is still a valid state.

>> IOW, this is introducing the race.
> I'm not quite clear about the race condition you mentioned here. Could you
> give an example to explain it?
> 
> Actually, the modification I initially considered was as follows. I'm not
> sure if this approach carries any risks. If it's acceptable, I'm ready to
> submit a new version.

When you try to change logic, you should read and understand the current
logic before.

For example, fat_iget() and __fat_write_inode() have to grab only valid
and live inode. And the orphaned inode must not be written back to disk,
because same entry can be reused already.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>