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>,
	kernel test robot <lkp@intel.com>,
	Dan Carpenter <dan.carpenter@linaro.org>,
	Sasha Levin <sashal@kernel.org>,
	ntfs3@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.19-5.15] fs/ntfs3: avoid calling run_get_entry() when run == NULL in ntfs_read_run_nb_ra()
Date: Fri, 20 Feb 2026 07:37:55 -0500	[thread overview]
Message-ID: <20260220123805.3371698-6-sashal@kernel.org> (raw)
In-Reply-To: <20260220123805.3371698-1-sashal@kernel.org>

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

[ Upstream commit c5226b96c08a010ebef5fdf4c90572bcd89e4299 ]

When ntfs_read_run_nb_ra() is invoked with run == NULL the code later
assumes run is valid and may call run_get_entry(NULL, ...), and also
uses clen/idx without initializing them. Smatch reported uninitialized
variable warnings and this can lead to undefined behaviour. This patch
fixes it.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202512230646.v5hrYXL0-lkp@intel.com/
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 all the information needed for a thorough analysis.

## Analysis

### 1. Commit Message Analysis

The commit clearly describes a bug fix: when `ntfs_read_run_nb()`
(renamed to `ntfs_read_run_nb_ra()` in the commit being analyzed) is
invoked with `run == NULL`, the code can later call `run_get_entry(NULL,
...)` and use uninitialized variables `clen` and `idx`. This was
reported by the kernel test robot and Dan Carpenter (a well-known static
analysis expert using Smatch).

### 2. Code Change Analysis

**The bug mechanism:**

At `fs/ntfs3/fsntfs.c:1186-1207`, when `run == NULL`, the function uses
the absolute boot MFT location:
- `lbo` and `len` are set from boot info (lines 1194-1195)
- **But `clen`, `idx`, and `vcn` are NOT initialized** — they are only
  set via `run_lookup_entry()` on the `run != NULL` path (line 1196)

The function then enters a `for(;;)` loop (line 1215) containing an
inner `do { } while (len32)` loop. If the requested `bytes` exceeds the
initial `len` (i.e., the read spans more than one record), the inner
loop completes, and execution falls through to line 1255 where:

1. `vcn_next = vcn + clen;` — **uses uninitialized `clen`** (undefined
   behavior)
2. `run_get_entry(run, ++idx, ...)` — **dereferences NULL `run`** (crash
   at `run->count` in `run.c:608`)
3. `++idx` — **uses uninitialized `idx`** (undefined behavior)

**What `run_get_entry()` does with NULL:**
Looking at `fs/ntfs3/run.c:608`, the very first thing it does is `if
(index >= run->count)`, which immediately dereferences the NULL pointer
→ **kernel oops/panic**.

**The fix:** Adds a simple `if (!run)` check before reaching the
`run_get_entry()` call, returning `-EINVAL` and going to error cleanup.
This is exactly 4 lines of code and is obviously correct — if we entered
the loop via the `run == NULL` path and need another fragment, we can't
get one, so returning an error is the right behavior.

### 3. Classification

This is a **bug fix** that prevents:
- NULL pointer dereference (crash/oops)
- Use of uninitialized variables (undefined behavior, potential data
  corruption)

### 4. Scope and Risk

- **Lines changed:** 4 lines of actual code added (NULL check + goto)
- **Files touched:** 1 (`fs/ntfs3/fsntfs.c`)
- **Risk:** Extremely low — adds a defensive check that can only trigger
  on an error path, returning `-EINVAL` which callers already handle
- **No behavioral change** for the normal case where `run != NULL`

### 5. User Impact

NTFS3 is used by anyone mounting NTFS filesystems on Linux (common for
dual-boot systems, external drives). While the `run == NULL` path is
specific to early MFT reading during mount, the uninitialized variable
use means the code is technically invoking undefined behavior, which
compilers can exploit in unexpected ways.

### 6. Stability Indicators

- Reported by kernel test robot and Dan Carpenter (Smatch static
  analysis) — two highly reputable sources
- The fix is authored by the NTFS3 maintainer (Konstantin Komarov)
- Extremely small and contained change

### 7. Dependency Check

The fix is self-contained — it just adds a NULL check. It applies to the
existing `ntfs_read_run_nb()` function which has been in the kernel
since NTFS3 was merged in v5.15.

## Verification

- **Read `ntfs_read_run_nb()` at fsntfs.c:1171-1282**: Confirmed `clen`,
  `idx` are only initialized via `run_lookup_entry()` on line 1196,
  which is skipped when `run == NULL`
- **Read `run_get_entry()` at run.c:603-623**: Confirmed line 608
  immediately dereferences `run->count`, which would crash if `run ==
  NULL`
- **Searched callers of `ntfs_read_run_nb()`**: Found 12 call sites; the
  `run == NULL` path is designed for early MFT reading (per comment at
  line 1187)
- **Checked `oa->run1` initialization in fslog.c**: All paths through
  `fake_attr` (line 4749) and normal flow (line 4801) set `oa->run1`, so
  the fslog caller is unlikely to pass NULL — but the function's design
  explicitly handles `run == NULL` at its entry
- **Confirmed ntfs3 present since v5.15**: The filesystem was merged in
  Linux 5.15, present in all active stable trees
- **git log confirmed recent ntfs3 activity**: Active maintenance by the
  NTFS3 maintainer

This is a textbook stable backport candidate: small, surgical fix for a
NULL pointer dereference and uninitialized variable use, authored by the
subsystem maintainer, reported by trusted automated tools.

**YES**

 fs/ntfs3/fsntfs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ntfs3/fsntfs.c b/fs/ntfs3/fsntfs.c
index 5f138f7158357..ac99c5613284a 100644
--- a/fs/ntfs3/fsntfs.c
+++ b/fs/ntfs3/fsntfs.c
@@ -1252,6 +1252,12 @@ int ntfs_read_run_nb(struct ntfs_sb_info *sbi, const struct runs_tree *run,
 
 		} while (len32);
 
+		if (!run) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		/* Get next fragment to read. */
 		vcn_next = vcn + clen;
 		if (!run_get_entry(run, ++idx, &vcn, &lcn, &clen) ||
 		    vcn != vcn_next) {
-- 
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 ` Sasha Levin [this message]
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 ` [PATCH AUTOSEL 6.19] fs/ntfs3: handle attr_set_size() errors when truncating files Sasha Levin
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-6-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=dan.carpenter@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --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