Re: [PATCH 1/2] hfsplus: update timestamps on truncate()

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Mon, 2018-10-15 at 22:24 +0100, Al Viro wrote:
> On Mon, Oct 15, 2018 at 02:02:01PM -0700, Viacheslav Dubeyko wrote:
> > > 1) unlink() finds the victim and locks it
> > > 
> > > 2) in hfsplus_file_release():
> > >         if (atomic_dec_and_test(&HFSPLUS_I(inode)->opencnt)) {
> > > got to 0
> > >                 inode_lock(inode);
> > > block waiting for unlink
> > > 
> > > 3) open() finds the sucker in dcache and hits hfsplus_file_open(), where
> > > we do
> > >         atomic_inc(&HFSPLUS_I(inode)->opencnt);
> > > and now opencnt is 1.
> > > 
> > > 4) on the unlink side:
> > >         if (inode->i_ino == cnid &&
> > >             atomic_read(&HFSPLUS_I(inode)->opencnt)) {
> > >                 str.name = name;
> > >                 str.len = sprintf(name, "temp%lu", inode->i_ino);
> > >                 res = hfsplus_rename_cat(inode->i_ino,
> > >                                          dir, &dentry->d_name,
> > >                                          sbi->hidden_dir, &str);
> > >                 if (!res) {
> > >                         inode->i_flags |= S_DEAD;
> > >                         drop_nlink(inode);
> > >                 }
> > >                 goto out;
> > >         }
> > > nlink is zero now, the sucker got renamed and marked S_DEAD
> > > 
> > > 5) ->release() finally got through inode_lock() and
> > >                 hfsplus_file_truncate(inode);
> > >                 if (inode->i_flags & S_DEAD) {
> > >                         hfsplus_delete_cat(inode->i_ino,
> > >                                            HFSPLUS_SB(sb)->hidden_dir, NULL);
> > >                         hfsplus_delete_inode(inode);
> > >                 }
> > >                 inode_unlock(inode);
> > > ... and now we have killed everything we used to have associated with that
> > > inode on disk.  While it's still open.  What's to stop CNID to be reused,
> > > etc. and what's to preserve sanity in that situation?
> > > 
> > > What am I missing there?
> > 
> > OK. If you believe that the patch is in the bad shape then what's your
> > suggestion for improving the patch?
> 
> Tha patch is OK; the problem, AFAICS, is neither introduced nor fixed by it.
> I might be wrong regarding the locking problem I described, but it does
> appear to be real and I'd like somebody more familiar with HFS+ to comment
> on it.

Yes, it looks like a mess. Let me take a deeper look in the code.

Thanks,
Vyacheslav Dubeyko.