Re: [PATCH] vfat: fix uninitialized i_pos error

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

zhoumin <teczm@foxmail.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@mail.parknet.co.jp>