On Fri, Oct 12, 2018 at 02:57:21PM -0700, Viacheslav Dubeyko wrote:
> Looks good.
>
> Reviewed-by: Vyacheslav Dubeyko <slava@xxxxxxxxxxx>
Looking at the vicinity of that code has brought something that looks
fishy: suppose we have the file opened and close() races with unlink()
and open()
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?
