diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index 8d6ff00c52788a..096e0953cdcbca 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -2040,6 +2040,12 @@ int fuse_flush_times(struct inode *inode, struct fuse_file *ff) inarg.valid |= FATTR_FH; inarg.fh = ff->fh; } + /* + * This is ->write_inode() flushing times the kernel owns locally, not + * a userspace utimes(); let the server tell the two apart. + */ + if (fm->fc->setattr_writeback) + inarg.valid |= FATTR_WRITEBACK; fuse_setattr_fill(fm->fc, &args, inode, &inarg, &outarg); return fuse_simple_request(fm, &args); diff --git a/fs/fuse/file.c b/fs/fuse/file.c index df8cbe9d6e435f..c30da1a6452ed6 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1899,7 +1899,31 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from) if (err) return err; - if (!fc->handle_killpriv_v2 || + /* + * A write that drops suid/sgid stays off the writeback path, + * so it holds i_rwsem exclusive. + * + * With handle_killpriv_v2 that is because the server does the + * killing from the WRITE itself. Without it, fuse_setattr() + * has to ask the server, and fuse_do_setattr() freezes + * writepages around that SETATTR: fuse_set_nowrite() asserts + * BUG_ON(fi->writectr < 0), which assumes an exclusive + * i_rwsem, and the DLM-relaxed buffered write path below holds + * it only shared. Two writers can both see the bits set + * before either has cleared them, and the second one would + * then oops inside spin_lock(&fi->lock). + * + * Only the DLM path needs the detour: everywhere else the + * buffered write already holds i_rwsem exclusive, so the two + * writers cannot overlap in the first place. + * + * The bits are read without the inode lock here, so a server + * attribute update can still set them between this test and + * file_remove_privs(). That leaves the same race, but only + * for writers whose mode changed underneath them, rather than + * for every write to a suid file. + */ + if (!(fc->handle_killpriv_v2 || fc->dlm) || !setattr_should_drop_suidgid(idmap, file_inode(file))) writeback = true; } @@ -1964,6 +1988,30 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from) goto out; } + err = count = generic_write_checks(iocb, from); + if (err <= 0) + goto out; + + /* + * Kill suid/sgid and stamp the timestamps here, before the gate, + * instead of leaving them next to the write itself. kiocb_modified() + * -> file_remove_privs() is the one that reaches the server: without + * handle_killpriv[_v2] fuse_setattr() kills the bits by asking it (a + * FUSE_GETATTR to refresh the mode, then a FUSE_SETATTR, which for a + * writeback inode first flushes and freezes writepages), and + * security_inode_killpriv() can drop the capability xattr with another + * round trip. A server may have to invalidate this inode from inside + * such a handler; its NOTIFY_INVAL_INODE then blocks in + * percpu_down_write() draining a gate reader that is itself waiting for + * the reply. Nothing held under the gate may wait for the server. + * + * This also runs before the forced-DIO re-route below, so a re-routed + * write repeats it; there is nothing left to do the second time. + */ + err = kiocb_modified(iocb); + if (err) + goto out; + wb_guard = !!wb_sem; if (wb_guard) { retry: @@ -1989,16 +2037,8 @@ static ssize_t fuse_cache_write_iter(struct kiocb *iocb, struct iov_iter *from) } } - err = count = generic_write_checks(iocb, from); - if (err <= 0) - goto out; - task_io_account_write(count); - err = kiocb_modified(iocb); - if (err) - goto out; - if (iocb->ki_flags & IOCB_DIRECT) { written = generic_file_direct_write(iocb, from); if (written < 0 || !iov_iter_count(from)) diff --git a/fs/fuse/fuse_dlm_cache.c b/fs/fuse/fuse_dlm_cache.c index b0b17cbd3c3f7a..bc6dbae2d5aeb0 100644 --- a/fs/fuse/fuse_dlm_cache.c +++ b/fs/fuse/fuse_dlm_cache.c @@ -562,6 +562,37 @@ bool fuse_dlm_range_is_locked(struct fuse_inode *inode, uint64_t start, return true; } +/** + * fuse_dlm_write_grant_exists - does the inode hold an exclusive grant anywhere + * @fi: the fuse inode + * + * Unlike fuse_dlm_range_is_locked(), which asks whether one range is fully + * covered, this asks whether any part of the file is held exclusively. A + * client that holds a write grant may be sitting on dirty page cache the + * server has not seen, so its mtime and ctime run ahead of anything the + * server can report. + * + * Return: true if at least one recorded range is held for write + */ +bool fuse_dlm_write_grant_exists(struct fuse_inode *fi) +{ + struct fuse_dlm_cache *cache = &fi->dlm_locked_areas; + struct fuse_dlm_range *range; + bool held = false; + + down_read(&cache->lock); + for (range = fuse_dlm_find_overlapping(cache, 0, U64_MAX); range; + range = fuse_page_it_iter_next(range, 0, U64_MAX)) { + if (range->mode == FUSE_PCACHE_LK_WRITE) { + held = true; + break; + } + } + up_read(&cache->lock); + + return held; +} + /** * fuse_dlm_lock_is_held - check that a byte range is covered by a granted lock * @fi: the fuse inode diff --git a/fs/fuse/fuse_dlm_cache.h b/fs/fuse/fuse_dlm_cache.h index 647a8c37c36095..30fdbb26bd3daf 100644 --- a/fs/fuse/fuse_dlm_cache.h +++ b/fs/fuse/fuse_dlm_cache.h @@ -71,6 +71,9 @@ bool fuse_dlm_range_is_locked(struct fuse_inode *inode, uint64_t start, bool fuse_dlm_lock_is_held(struct fuse_inode *inode, loff_t offset, size_t length, enum fuse_page_lock_mode mode); +/* Is any part of the file held for write? */ +bool fuse_dlm_write_grant_exists(struct fuse_inode *inode); + /* This is the interface to the filesystem */ int fuse_get_dlm_lock(struct file *file, loff_t offset, size_t length, enum fuse_page_lock_mode mode); diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 20ae3902c369b2..55fe9a5368f481 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -874,6 +874,9 @@ struct fuse_conn { /* expire inode entries when doing inode invalidation */ unsigned expire_inode_entries:1; + /* mark writeback-initiated SETATTR requests with FATTR_WRITEBACK */ + unsigned setattr_writeback:1; + /* * The following bitfields are only for optimization purposes * and hence races in setting them will not cause malfunction diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index ead6ad3e5bdbcf..2ef8dd685a4c25 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -512,6 +512,68 @@ u32 fuse_get_cache_mask(struct inode *inode) return STATX_MTIME | STATX_CTIME | STATX_SIZE; } +/* + * Which cached attributes survive a server reply. + * + * Without DLM this is fuse_get_cache_mask(): with the writeback cache on, + * writes update mtime and ctime and may extend i_size locally, the server + * knows about none of it, so the cached values win. + * + * With DLM the server is the authority (fuse_get_cache_mask() returns 0), + * because another node may have changed the file behind us and only the + * server can say so. That holds for the parts of the file we do not own. A + * write grant means no other node can touch the range until we are revoked, + * so anything the server reports about it is at best as new as what we have, + * and older if we still have unwritten data there. Keep the cached values + * for exactly what the grant covers: + * + * - size, when the server reports less than i_size and the tail it does not + * know about, [srv_size, i_size), is entirely under a write grant. Taking + * the server's answer would shrink i_size and have truncate_pagecache() + * throw the unwritten tail away. + * - mtime and ctime, while a write grant covers unwritten data: our writes + * have stamped them locally and the server's stamps predate them. Only + * while the cache is actually dirty, not for as long as the grant lives: + * a grant is held until it is revoked or the inode is evicted, and past + * the writeback the server's stamps are the newer ones. Keeping ours + * beyond that would hide a remote chown or chmod indefinitely. + * + * A remote truncate cannot slip through. It has to revoke the grant first, + * and the revoke launders the tail and drops the grant, so by the time the + * smaller size is reported neither check holds and the server's answer is + * applied as usual. A grant the server made but that could not be recorded + * (FUSE_DLM_GRANT_UNRECORDED) is invisible to the lock tree and falls back to + * trusting the server, as before. + * + * Must be called without fi->lock: the lock tree query sleeps. + */ +static u32 fuse_attr_cache_mask(struct inode *inode, struct fuse_attr *attr, + bool have_size) +{ + struct fuse_conn *fc = get_fuse_conn(inode); + struct fuse_inode *fi = get_fuse_inode(inode); + u32 cache_mask = fuse_get_cache_mask(inode); + loff_t size = i_size_read(inode); + + if (cache_mask || !fc->dlm || !fc->writeback_cache || + !S_ISREG(inode->i_mode)) + return cache_mask; + + if (!fuse_dlm_write_grant_exists(fi)) + return cache_mask; + + if (mapping_tagged(inode->i_mapping, PAGECACHE_TAG_DIRTY) || + mapping_tagged(inode->i_mapping, PAGECACHE_TAG_WRITEBACK)) + cache_mask |= STATX_MTIME | STATX_CTIME; + + if (have_size && size > (loff_t) attr->size && + fuse_dlm_lock_is_held(fi, attr->size, size - attr->size, + FUSE_PAGE_LOCK_WRITE)) + cache_mask |= STATX_SIZE; + + return cache_mask; +} + static void fuse_change_attributes_i(struct inode *inode, struct fuse_attr *attr, struct fuse_statx *sx, u64 attr_valid, u64 attr_version, u64 evict_ctr) @@ -524,15 +586,11 @@ static void fuse_change_attributes_i(struct inode *inode, struct fuse_attr *attr bool have_size = !sx || (sx->mask & STATX_SIZE); u64 srv_size; + cache_mask = fuse_attr_cache_mask(inode, attr, have_size); + spin_lock(&fi->lock); srv_size = attr->size; - /* - * In case of writeback_cache enabled, writes update mtime, ctime and - * may update i_size. In these cases trust the cached value in the - * inode. - */ - cache_mask = fuse_get_cache_mask(inode); if (cache_mask & STATX_SIZE) attr->size = i_size_read(inode); @@ -580,11 +638,16 @@ static void fuse_change_attributes_i(struct inode *inode, struct fuse_attr *attr spin_unlock(&fi->lock); /* - * Only do page cache invalidation when cache_mask is not set - * (writeback_cache disabled) AND the relevant attributes (SIZE/MTIME) - * were actually returned by the server. + * Only do page cache invalidation when the size was not served from + * the cache (writeback_cache disabled, or no grant covering the tail) + * AND the relevant attributes (SIZE/MTIME) were actually returned by + * the server. This has to key off STATX_SIZE alone: i_size_write() + * above took the server's size for any mask without that bit, and the + * cache has to be truncated to match it. The mtime branch neutralises + * itself when STATX_MTIME is set, since attr->mtime then holds the + * value old_mtime was read from. */ - if (!cache_mask && S_ISREG(inode->i_mode)) { + if (!(cache_mask & STATX_SIZE) && S_ISREG(inode->i_mode)) { bool inval = false; bool have_mtime = !sx || (sx->mask & STATX_MTIME); @@ -851,6 +914,40 @@ static void fuse_dlm_revoke_inval_range(struct fuse_inode *fi, loff_t offset, fuse_dlm_unlock_range(fi, start, end); } +/* + * Drop a page-cache range on behalf of a NOTIFY invalidate. + * + * invalidate_inode_pages2_range() waits out folios under writeback and + * launders dirty ones, both of which need a FUSE_WRITE reply. While + * writepages are frozen (fuse_set_nowrite(): truncate, O_TRUNC open, fsync, + * pre-SETATTR flush) no reply can arrive, because fuse_flush_writepages() + * parks the request on fi->queued_writes until fuse_release_nowrite(). A + * server that revokes from inside the handler it is revoking for then + * deadlocks against its own reply. fuse_do_setattr() states the same rule + * for its own invalidate. + * + * So while frozen use invalidate_mapping_pages(), which skips dirty and + * under-writeback folios and never blocks. The stale clean folios still + * go, and the freezes that span a request drop the cache themselves once + * they complete: fuse_do_setattr() invalidates the mapping after releasing + * the freeze, the O_TRUNC open path calls truncate_pagecache(). + */ +static void fuse_notify_invalidate_range(struct inode *inode, pgoff_t start, + pgoff_t end) +{ + struct fuse_inode *fi = get_fuse_inode(inode); + bool frozen; + + spin_lock(&fi->lock); + frozen = fi->writectr < 0; + spin_unlock(&fi->lock); + + if (frozen) + invalidate_mapping_pages(inode->i_mapping, start, end); + else + invalidate_inode_pages2_range(inode->i_mapping, start, end); +} + int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, loff_t offset, loff_t len) { @@ -984,10 +1081,10 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, * notified range. */ if (fuse_inode_force_dio(inode)) - invalidate_inode_pages2(inode->i_mapping); + fuse_notify_invalidate_range(inode, 0, -1); else - invalidate_inode_pages2_range(inode->i_mapping, - pg_start, pg_end); + fuse_notify_invalidate_range(inode, pg_start, + pg_end); percpu_up_write(wb_sem); @@ -1000,8 +1097,7 @@ int fuse_reverse_inval_inode(struct fuse_conn *fc, u64 nodeid, * range unserialized (best-effort), as before. */ if (fc->dlm && fc->writeback_cache) fuse_dlm_revoke_inval_range(fi, offset, len); - invalidate_inode_pages2_range(inode->i_mapping, - pg_start, pg_end); + fuse_notify_invalidate_range(inode, pg_start, pg_end); } } iput(inode); @@ -1912,6 +2008,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args, fc->inval_inode_entries = 1; if (flags & FUSE_EXPIRE_INODE_ENTRY) fc->expire_inode_entries = 1; + if (flags & FUSE_SETATTR_WRITEBACK) + fc->setattr_writeback = 1; } else { ra_pages = fc->max_read / PAGE_SIZE; fc->no_lock = 1; @@ -1965,7 +2063,8 @@ static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm) FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP | FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_ALLOW_IDMAP | FUSE_REQUEST_TIMEOUT | FUSE_INVAL_INODE_ENTRY | - FUSE_EXPIRE_INODE_ENTRY | FUSE_URING_REDUCED_Q; + FUSE_EXPIRE_INODE_ENTRY | FUSE_URING_REDUCED_Q | + FUSE_SETATTR_WRITEBACK; #ifdef CONFIG_FUSE_DAX if (fm->fc->dax) flags |= FUSE_MAP_ALIGNMENT; diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h index 23374de18e2fd9..eb31ec25dad1f7 100644 --- a/include/uapi/linux/fuse.h +++ b/include/uapi/linux/fuse.h @@ -366,6 +366,17 @@ struct fuse_file_lock { #define FATTR_LOCKOWNER (1 << 9) #define FATTR_CTIME (1 << 10) #define FATTR_KILL_SUIDGID (1 << 11) +/* + * Not an attribute selector: marks the request as a kernel-initiated + * writeback of locally owned attributes rather than a userspace-initiated + * change. Only sent if the server negotiated FUSE_SETATTR_WRITEBACK. + * + * The bit is deliberately far above the sequentially allocated FATTR_* + * range: libfuse mirrors these bits into its own FUSE_SET_ATTR_* space, + * which has its own allocations from bit 12 upwards, and only a value that + * is free on both sides can be passed through without translation. + */ +#define FATTR_WRITEBACK (1 << 30) /** * Flags returned by the OPEN request @@ -449,6 +460,8 @@ struct fuse_file_lock { * optimal io-size alignment * FUSE_URING_REDUCED_Q: Client (kernel) supports less queues - Server is free * to register between 1 and nr-core io-uring queues + * FUSE_SETATTR_WRITEBACK: kernel marks writeback-initiated SETATTR requests + * with FATTR_WRITEBACK */ #define FUSE_ASYNC_READ (1 << 0) #define FUSE_POSIX_LOCKS (1 << 1) @@ -498,6 +511,7 @@ struct fuse_file_lock { #define FUSE_REQUEST_TIMEOUT (1ULL << 42) #define FUSE_ALIGN_PG_ORDER (1ULL << 50) +#define FUSE_SETATTR_WRITEBACK (1ULL << 58) #define FUSE_URING_REDUCED_Q (1ULL << 59) #define FUSE_INVAL_INODE_ENTRY (1ULL << 60) #define FUSE_EXPIRE_INODE_ENTRY (1ULL << 61)