From: Bernd Schubert bschubert@ddn.com
[ Upstream commit 1ce120dcefc056ce8af2486cebbb77a458aad4c3 ]
This was done as condition on direct_io_allow_mmap, but I believe this is not right, as a file might be open two times - once with write-back enabled another time with FOPEN_DIRECT_IO.
Signed-off-by: Bernd Schubert bschubert@ddn.com Signed-off-by: Miklos Szeredi mszeredi@redhat.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
## Analysis Summary
### 1. WHAT THE COMMIT FIXES
This commit addresses a **data coherency bug** in FUSE when a file can be opened twice with different modes: - **File descriptor A**: opened normally with page cache (write-back caching enabled) - **File descriptor B**: opened with `FOPEN_DIRECT_IO` flag
**The Bug:** The original code only flushed dirty pages before direct I/O when `fc->direct_io_allow_mmap` was enabled. However, dirty pages can exist regardless of this flag - they can come from a write-back cached file handle to the same file.
**Data corruption scenario:** 1. Write data via normal cached file handle → creates dirty pages in page cache 2. Write to same location via FOPEN_DIRECT_IO handle → goes directly to backend storage 3. Later, dirty pages from step 1 flush to disk → **OVERWRITE** the direct IO data
This causes **data loss/corruption** where writes via direct I/O are silently overwritten.
### 2. CODE CHANGE ANALYSIS
The change is minimal: ```c - if (fopen_direct_io && fc->direct_io_allow_mmap) { + if (fopen_direct_io) { ```
Simply removes the `&& fc->direct_io_allow_mmap` condition, making the `filemap_write_and_wait_range()` call happen for **all** `FOPEN_DIRECT_IO` operations, not just when `direct_io_allow_mmap` is enabled.
### 3. STABLE CRITERIA CHECK
| Criterion | Assessment | |-----------|------------| | Obviously correct | ✅ The fix is logically sound - always flush dirty pages before direct IO | | Fixes real bug | ✅ Data corruption/loss in specific multi-fd scenarios | | Important issue | ✅ Data corruption is severe | | Small and contained | ✅ Single condition removal, one file | | No new features | ✅ Pure correctness fix | | Clean application | ✅ Should apply cleanly |
### 4. RISK ASSESSMENT
**Risk: LOW** - The change is **conservative** - it flushes *more* often, not less - Worst case: slight performance regression from additional sync operations - Best case: prevents data corruption in mixed cached/direct IO scenarios - The affected code path (`fuse_direct_io`) is well-established
### 5. BACKPORT CONCERNS
**Negative signals:** - No `Cc: stable@vger.kernel.org` tag - No `Fixes:` tag pointing to `b5a2a3a0b7766` (which introduced the incomplete logic)
**Positive signals:** - Small, surgical fix - Fixes data corruption (high severity) - Maintainer-reviewed (signed off by Miklos Szeredi, FUSE maintainer) - Safe direction of change
### 6. VERSION APPLICABILITY
The original buggy commit (`b5a2a3a0b7766`) was merged in v6.10, so this fix applies to: - v6.10.y through current stable trees
### 7. USER IMPACT
Affects FUSE users who: - Use FOPEN_DIRECT_IO feature - Have the same file opened through multiple file descriptors with different caching modes - This is not a theoretical scenario - FUSE servers like libfuse can legitimately have files accessed this way
### CONCLUSION
Despite the absence of explicit `Cc: stable` tags, this commit **should be backported**. It fixes a real data corruption bug with a minimal, obviously correct change. The fix is safe (more synchronization, not less), small, and addresses a serious issue. The lack of stable tags appears to be an oversight rather than an indication the fix isn't needed in stable. Data corruption bugs in filesystems are exactly the type of issue stable trees exist to fix.
**YES**
fs/fuse/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c index f1ef77a0be05b..c5c82b3807911 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -1607,7 +1607,7 @@ ssize_t fuse_direct_io(struct fuse_io_priv *io, struct iov_iter *iter, if (!ia) return -ENOMEM;
- if (fopen_direct_io && fc->direct_io_allow_mmap) { + if (fopen_direct_io) { res = filemap_write_and_wait_range(mapping, pos, pos + count - 1); if (res) { fuse_io_free(ia);