On 8/30/23 15:28, Miklos Szeredi wrote:
On Tue, 29 Aug 2023 at 18:11, Bernd Schubert <bschubert@xxxxxxx> wrote:
Take a shared lock in fuse_cache_write_iter. This was already
done for FOPEN_DIRECT_IO in
commit 153524053bbb ("fuse: allow non-extending parallel direct
writes on the same file")
but so far missing for plain O_DIRECT. Server side needs
to set FOPEN_PARALLEL_DIRECT_WRITES in order to signal that
it supports parallel dio writes.
Hmm, I think file_remove_privs() needs exclusive lock (due to calling
notify_change()). And the fallback case also.
Need to be careful with such locking changes...
Hrmm, yeah, I missed that :( Really sorry and thanks!
I guess I can fix it if by exporting dentry_needs_remove_privs. I hope
that is acceptable. Interesting is that btrfs_direct_write seems to have
the same issue.
btrfs_direct_write
if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode))
ilock_flags |= BTRFS_ILOCK_SHARED;
...
err = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
...
err = btrfs_write_check(iocb, from, err);
...
ret = file_remove_privs(file);
I think that can be fixed as well after exporting
dentry_needs_remove_privs().
Btw, why is fuse_direct_write_iter not dropping privileges? That is
another change I need to document...
Another issue I just see is that it needs to check file size again after
taking the lock.
Thanks a lot for your review,
Bernd