[PATCH AUTOSEL 6.19] fs/ntfs3: handle attr_set_size() errors when truncating files

[Sasha Levin]

From: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>

[ Upstream commit 576248a34b927e93b2fd3fff7df735ba73ad7d01 ]

If attr_set_size() fails while truncating down, the error is silently
ignored and the inode may be left in an inconsistent state.

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

Now I have a clear picture of the bug. Let me analyze it:

## Analysis

### 1. COMMIT MESSAGE ANALYSIS

The commit message is clear: "handle attr_set_size() errors when
truncating files." It states that if `attr_set_size()` fails while
truncating down, the error is silently ignored and the inode may be left
in an inconsistent state. The author is Konstantin Komarov, the NTFS3
maintainer.

### 2. CODE CHANGE ANALYSIS

**The bug:** In the original `ntfs_truncate()` function (line 504-554 of
`fs/ntfs3/file.c`), `attr_set_size()` is called at line 531-533 and its
return value is stored in `err`. However, after `ni_unlock(ni)`, the
code does **not** check `err` — it proceeds to modify the inode metadata
(setting FILE_ATTRIBUTE_ARCHIVE, updating timestamps) and then either:
- Sets `dirty = 1` and later calls `mark_inode_dirty()`, OR
- Calls `ntfs_sync_inode()` which **overwrites** `err` (line 545-546)

In both paths, the original error from `attr_set_size()` is lost. The
function returns `0` (line 554) even when the underlying attribute size
change failed.

**The fix adds** (after line 539, after `ni_unlock(ni)`):
```c
if (unlikely(err))
    return err;
```

This ensures that if `attr_set_size()` failed, the error is propagated
to the caller immediately, before marking the inode dirty with
potentially inconsistent metadata.

**Additional cleanup:** The `dirty` variable is removed and
`mark_inode_dirty()` is called directly in the `!IS_DIRSYNC` branch,
which is a trivial simplification.

### 3. IMPACT ASSESSMENT

**The caller** (`ntfs_setattr` at line 851) checks the return value of
`ntfs_truncate()`:
```c
err = ntfs_truncate(inode, newsize);
...
if (err)
    goto out;
```

So if `attr_set_size()` fails (e.g., due to disk I/O error, out of
space, corrupted MFT records), the old code would:
1. Silently ignore the failure
2. Proceed to update inode metadata as if truncation succeeded
3. Return success to the VFS layer

This means the in-memory inode state (already updated by
`truncate_setsize()`) could diverge from the on-disk NTFS attribute
state, leading to **filesystem inconsistency**. The inode would think
the file is the new size while the NTFS data attribute still has the old
size.

This is a **data corruption** bug. On an NTFS filesystem, this could
lead to:
- Stale data being visible after truncation
- Incorrect file sizes reported
- Further I/O errors when accessing the inconsistent inode

### 4. SCOPE AND RISK

- **2 lines of actual logic change** (the `if (unlikely(err)) return
  err;` check), plus trivial cleanup of the `dirty` variable
- Affects only the NTFS3 truncate path
- Very low risk of regression — it only changes behavior when
  `attr_set_size()` already failed
- The code has existed since the initial NTFS3 commit `4342306f0f0d5f`
  (v5.15, 2021-08-13), so this bug affects all stable trees containing
  NTFS3

### 5. STABLE CRITERIA

- **Obviously correct**: The fix simply checks an already-captured error
  return value that was being silently ignored
- **Fixes a real bug**: Silently ignoring errors from `attr_set_size()`
  can lead to filesystem inconsistency
- **Important issue**: Filesystem data corruption/inconsistency
- **Small and contained**: ~5 lines changed, single file, single
  function
- **No new features**: Pure error handling fix

### Verification

- `git blame` confirmed the buggy code (missing error check after
  `attr_set_size()`) has been present since commit `4342306f0f0d5f`
  (Konstantin Komarov, 2021-08-13), which is the initial NTFS3 import in
  v5.15
- Verified the caller `ntfs_setattr` at line 851 checks
  `ntfs_truncate()` return value — so propagating the error has
  meaningful effect
- Verified that line 554 (`return 0`) means the error is unconditionally
  discarded in the original code — confirmed by reading lines 539-554
- Verified `attr_set_size()` can return errors (e.g., `-ENOENT` at line
  430, propagated errors from `attr_set_size_res` at line 437)
- Verified the `dirty` variable removal is safe — `mark_inode_dirty()`
  is now called directly in the `!IS_DIRSYNC` branch, which is
  functionally identical
- Could NOT verify whether this specific patch has been discussed on
  lore.kernel.org (not checked via WebFetch), but the author is the
  NTFS3 maintainer which provides confidence

This is a clear, small, obviously-correct error handling fix for a
filesystem data consistency bug that has existed since NTFS3 was first
introduced. It meets all stable kernel criteria.

**YES**

 fs/ntfs3/file.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c
index 2e7b2e566ebe1..362396968ab69 100644
--- a/fs/ntfs3/file.c
+++ b/fs/ntfs3/file.c
@@ -505,8 +505,8 @@ static int ntfs_truncate(struct inode *inode, loff_t new_size)
 {
 	struct super_block *sb = inode->i_sb;
 	struct ntfs_inode *ni = ntfs_i(inode);
-	int err, dirty = 0;
 	u64 new_valid;
+	int err;
 
 	if (!S_ISREG(inode->i_mode))
 		return 0;
@@ -522,7 +522,6 @@ static int ntfs_truncate(struct inode *inode, loff_t new_size)
 	}
 
 	new_valid = ntfs_up_block(sb, min_t(u64, ni->i_valid, new_size));
-
 	truncate_setsize(inode, new_size);
 
 	ni_lock(ni);
@@ -536,20 +535,19 @@ static int ntfs_truncate(struct inode *inode, loff_t new_size)
 		ni->i_valid = new_valid;
 
 	ni_unlock(ni);
+	if (unlikely(err))
+		return err;
 
 	ni->std_fa |= FILE_ATTRIBUTE_ARCHIVE;
 	inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
 	if (!IS_DIRSYNC(inode)) {
-		dirty = 1;
+		mark_inode_dirty(inode);
 	} else {
 		err = ntfs_sync_inode(inode);
 		if (err)
 			return err;
 	}
 
-	if (dirty)
-		mark_inode_dirty(inode);
-
 	return 0;
 }
 
-- 
2.51.0