Re: [PATCH v2 1/2] hfs: Validate CNIDs in hfs_read_inode

[Tetsuo Handa]

On 2025/11/25 8:46, George Anthony Vernon wrote:
> On Tue, Nov 11, 2025 at 10:42:09PM +0000, Viacheslav Dubeyko wrote:
>> On Tue, 2025-11-11 at 23:39 +0900, Tetsuo Handa wrote:
>>> On 2025/11/04 10:47, George Anthony Vernon wrote:
>>>> +	if (!is_valid_cnid(inode->i_ino,
>>>> +			   S_ISDIR(inode->i_mode) ? HFS_CDR_DIR : HFS_CDR_FIL))
>>>> +		BUG();
>>>
>>> Is it guaranteed that hfs_write_inode() and make_bad_inode() never run in parallel?
>>> If no, this check is racy because make_bad_inode() makes S_ISDIR(inode->i_mode) == false.
>>>  
>>
>> Any inode should be completely created before any hfs_write_inode() call can
>> happen. So, I don't see how hfs_write_inode() and make_bad_inode() could run in
>> parallel.
>>
> 
> Could we not read the same inode a second time, during the execution of
> hfs_write_inode()?
> 
> Then I believe we could hit make_bad_inode() in hfs_read_inode() once we
> had already entered hfs_write_inode(), and so test a cnid against the
> wrong i_mode.
> 

My "Is it guaranteed that hfs_write_inode() and make_bad_inode() never run in parallel?"
question does not assume "make_bad_inode() for HFS is called from only hfs_read_inode()".

write_inode() already checks for !is_bad_inode(inode) before calling
filesystem's write_inode callback
( https://elixir.bootlin.com/linux/v6.18-rc7/source/fs/fs-writeback.c#L1558 ).

If the reason for "ubifs is doing it in the ubifs_write_inode()" 
( https://elixir.bootlin.com/linux/v6.18-rc7/source/fs/ubifs/super.c#L299 ) is
that make_bad_inode() could be called at any moment, it is not safe for HFS to
depend on "inode->i_mode does not change during hfs_write_inode()".