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

Tetsuo Handa
2026-01-06T10:22:07Z

On 2025/11/30 19:07, Tetsuo Handa wrote:
> 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()".

In a different thread, it turned out that make_bad_inode() must not be called
once the inode is fully visible
( https://lkml.kernel.org/r/wqkxevwtev5p77czk2com5zvbbwcpxxeucrt7zbgjciqxjyivx@c7624klburuh ),
requiring "make_bad_inode() for HFS is called from only hfs_read_inode()" assumption being true.

When can we expect next version of this patch?

>
> 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()".
>