public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
	Sasha Levin <sashal@kernel.org>,
	ntfs3@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.19] fs/ntfs3: handle attr_set_size() errors when truncating files
Date: Fri, 20 Feb 2026 07:37:57 -0500	[thread overview]
Message-ID: <20260220123805.3371698-8-sashal@kernel.org> (raw)
In-Reply-To: <20260220123805.3371698-1-sashal@kernel.org>

From: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>

[ 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@paragon-software.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

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


  parent reply	other threads:[~2026-02-20 12:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-20 12:37 [PATCH AUTOSEL 6.19-5.15] libceph: define and enforce CEPH_MAX_KEY_LEN Sasha Levin
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-5.15] fs: ntfs3: fix infinite loop triggered by zero-sized ATTR_LIST Sasha Levin
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-6.6] thermal: int340x: Fix sysfs group leak on DLVR registration failure Sasha Levin
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-5.15] fs: ntfs3: check return value of indx_find to avoid infinite loop Sasha Levin
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-6.12] ACPI: x86: Force enabling of PWM2 on the Yogabook YB1-X90 Sasha Levin
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-5.15] fs/ntfs3: avoid calling run_get_entry() when run == NULL in ntfs_read_run_nb_ra() Sasha Levin
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-6.1] ceph: supply snapshot context in ceph_uninline_data() Sasha Levin
2026-02-20 12:37 ` Sasha Levin [this message]
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-6.18] ntfs3: fix circular locking dependency in run_unpack_ex Sasha Levin
2026-02-20 12:37 ` [PATCH AUTOSEL 6.19-6.1] fs/ntfs3: drop preallocated clusters for sparse and compressed files Sasha Levin
2026-02-20 12:38 ` [PATCH AUTOSEL 6.19-5.15] fs: ntfs3: fix infinite loop in attr_load_runs_range on inconsistent metadata Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260220123805.3371698-8-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ntfs3@lists.linux.dev \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox