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

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Mon, 2025-11-24 at 23:46 +0000, 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.
> 
> 

Maybe, I am missing something in your point. But your explanation sounds
slightly strange to me.

At first, VFS code tries to find inode object in the cache of already created
inodes. So, if we created inode, then it should be in inode cache. It means that
we don't need to execute any read operations for such inode object. So, we can
call hfs_write_inode() only if inode object completely created and it is in
inode cache.

Potentially, it is possible to imagine situation that two or more threads try to
access the same inode ID in parallel while inode object is not created yet. But,
usually, newly created inode is locked until the end of creation operation. So,
only one thread will call hfs_read_inode() and others will get inode object from
the cache.

So, hfs_write_inode() could happen if we took the inode from inode cache. And
this inode could be good or bad object. But I still don't see how
hfs_read_inode() and hfs_write_inode() can be called in parallel for the same
inode object.

Thanks,
Slava.