* [PATCH AUTOSEL 6.19-5.15] gfs2: fiemap page fault fix
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] dlm: fix recovery pending middle conversion Sasha Levin
` (22 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable; +Cc: Andreas Gruenbacher, Sasha Levin, gfs2
From: Andreas Gruenbacher <agruenba@redhat.com>
[ Upstream commit e411d74cc5ba290f85d0dd5e4d1df8f1d6d975d2 ]
In gfs2_fiemap(), we are calling iomap_fiemap() while holding the inode
glock. This can lead to recursive glock taking if the fiemap buffer is
memory mapped to the same inode and accessing it triggers a page fault.
Fix by disabling page faults for iomap_fiemap() and faulting in the
buffer by hand if necessary.
Fixes xfstest generic/742.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have the complete picture. Let me write up the full analysis.
---
## Detailed Analysis
### 1. Commit Message Analysis
The commit subject is "gfs2: fiemap page fault fix" — a clear bug fix.
The body explains a concrete deadlock scenario: calling `iomap_fiemap()`
while holding the inode glock can lead to **recursive glock taking** if
the fiemap buffer is memory-mapped to the same inode. It also says it
"Fixes xfstest generic/742," which is a test specifically designed to
catch this class of bug.
The author is Andreas Gruenbacher, the GFS2 maintainer and the same
person who wrote the original mmap + page fault deadlock fixes for
GFS2's buffered I/O (commit `00bfe02f47968`) and direct I/O (commit
`b01b2d72da25c`) paths — both merged in v5.16.
### 2. Code Change Analysis — The Deadlock Mechanism
The deadlock path is clear and concrete:
1. **`gfs2_fiemap()`** acquires the inode glock `ip->i_gl` in
`LM_ST_SHARED` mode via `gfs2_glock_nq_init()`
2. **`iomap_fiemap()`** iterates over file extents and calls
**`iomap_to_fiemap()`** → **`fiemap_fill_next_extent()`**
3. `fiemap_fill_next_extent()` uses **`copy_to_user()`** to write extent
data to the user-supplied buffer (line 144 of `fs/ioctl.c`)
4. If that user buffer is **mmap'd to the same file**, `copy_to_user()`
triggers a page fault
5. The page fault enters **`gfs2_page_mkwrite()`** (for write faults) or
**`gfs2_fault()`** (for read faults)
6. **`gfs2_page_mkwrite()`** tries to acquire `ip->i_gl` in
`LM_ST_EXCLUSIVE` mode — **deadlock**: it conflicts with the already-
held `LM_ST_SHARED`. Even `gfs2_fault()` would attempt another
`LM_ST_SHARED`, which in GFS2's distributed locking model can also
cause issues (the glock holder tracks are per-process, and in a
clustered scenario this can cause distributed deadlocks).
### 3. The Fix — Well-Established Pattern
The fix follows the **exact same pattern** already used in 4+ locations
within `fs/gfs2/file.c`:
```2195:2212:fs/gfs2/inode.c
retry:
ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
if (ret)
goto out;
pagefault_disable();
ret = iomap_fiemap(inode, fieinfo, start, len, &gfs2_iomap_ops);
pagefault_enable();
gfs2_glock_dq_uninit(&gh);
if (ret == -EFAULT && fault_in_fiemap(fieinfo)) {
fieinfo->fi_extents_mapped = 0;
goto retry;
}
```
The pattern is:
- **`pagefault_disable()`** before the operation that touches user
memory while holding the glock
- **`pagefault_enable()`** after
- If `-EFAULT` is returned (page fault was needed but prevented), **drop
the glock**, **manually fault in** the user pages, and **retry**
The helper `fault_in_fiemap()` calls `fault_in_safe_writeable()` on the
destination buffer, which is a non-destructive way to ensure pages are
resident without holding any filesystem locks.
### 4. Cross-Subsystem Validation
The **exact same bug class** was found and fixed in OCFS2
(`04100f775c2ea`), reported by syzbot, and explicitly tagged `Cc:
stable@vger.kernel.org`. That fix had a stack trace showing the deadlock
happening during `copy_to_user` in `fiemap_fill_next_extent` → recursive
semaphore acquisition in `ocfs2_page_mkwrite`. The GFS2 case is
structurally identical.
### 5. Scope and Risk Assessment
- **Lines changed**: +20 lines in a single file (`fs/gfs2/inode.c`)
- **New helper**: `fault_in_fiemap()` — 6 lines, trivial wrapper around
`fault_in_safe_writeable()`
- **Dependencies**: `fault_in_safe_writeable()` exists in all current
LTS trees (v6.1, v6.6, v6.12)
- **Clean apply**: The `gfs2_fiemap()` function is byte-for-byte
identical in v6.1, v6.6, v6.12, and mainline. The patch applies
cleanly.
- **Risk**: Very low. The fix uses a well-established pattern from the
same subsystem. The only behavioral change is adding retry logic for
the (previously impossible) case where the user buffer is mmap'd to
the same file. Normal fiemap calls are unaffected (no page faults, no
retry).
### 6. User Impact
- **Severity**: **Deadlock** — system hang for the process, and on
clustered GFS2, potential cluster-wide impact
- **Trigger**: User-space can trigger this by mmap'ing a GFS2 file and
passing the mmap'd buffer to `FS_IOC_FIEMAP`
- **Affected users**: All GFS2 users, especially those running xfstests
or using fiemap on clustered filesystems
- **Test coverage**: xfstest generic/742 specifically tests this
scenario
### 7. Stability and Quality Indicators
- Author is the GFS2 subsystem maintainer (Andreas Gruenbacher)
- Pattern is proven and battle-tested in the same codebase since v5.16
- The analogous OCFS2 fix was syzbot-reported and stable-tagged
- The fix has a dedicated xfstest (generic/742)
### 8. Dependency Check
No external dependencies. The fix is self-contained:
- `pagefault_disable/enable` — core kernel functions, always available
- `fault_in_safe_writeable` — available since v5.16, present in all
current LTS trees
- The `gfs2_fiemap` function is unchanged across all LTS trees
### Conclusion
This commit fixes a **real deadlock** in GFS2's fiemap path that can be
triggered from user space. The fix is small (20 lines), self-contained,
follows a well-established pattern already used in the same subsystem,
applies cleanly to all current stable trees, and has a dedicated
xfstest. The same bug class in OCFS2 was considered important enough for
syzbot reporting and explicit stable tagging. This is a textbook stable
backport candidate.
**YES**
fs/gfs2/inode.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 36618e3531996..ea03c23bd147b 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -2182,6 +2182,14 @@ static int gfs2_getattr(struct mnt_idmap *idmap,
return 0;
}
+static bool fault_in_fiemap(struct fiemap_extent_info *fi)
+{
+ struct fiemap_extent __user *dest = fi->fi_extents_start;
+ size_t size = sizeof(*dest) * fi->fi_extents_max;
+
+ return fault_in_safe_writeable((char __user *)dest, size) == 0;
+}
+
static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
u64 start, u64 len)
{
@@ -2191,14 +2199,22 @@ static int gfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
inode_lock_shared(inode);
+retry:
ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
if (ret)
goto out;
+ pagefault_disable();
ret = iomap_fiemap(inode, fieinfo, start, len, &gfs2_iomap_ops);
+ pagefault_enable();
gfs2_glock_dq_uninit(&gh);
+ if (ret == -EFAULT && fault_in_fiemap(fieinfo)) {
+ fieinfo->fi_extents_mapped = 0;
+ goto retry;
+ }
+
out:
inode_unlock_shared(inode);
return ret;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.18] dlm: fix recovery pending middle conversion
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-5.15] gfs2: fiemap page fault fix Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.6] smb: client: prevent races in ->query_interfaces() Sasha Levin
` (21 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable; +Cc: Alexander Aring, David Teigland, Sasha Levin, gfs2
From: Alexander Aring <aahringo@redhat.com>
[ Upstream commit 1416bd508c78bdfdb9ae0b4511369e5581f348ea ]
During a workload involving conversions between lock modes PR and CW,
lock recovery can create a "conversion deadlock" state between locks
that have been recovered. When this occurs, kernel warning messages
are logged, e.g.
"dlm: WARN: pending deadlock 1e node 0 2 1bf21"
"dlm: receive_rcom_lock_args 2e middle convert gr 3 rq 2 remote 2 1e"
After this occurs, the deadlocked conversions both appear on the convert
queue of the resource being locked, and the conversion requests do not
complete.
Outside of recovery, conversions that would produce a deadlock are
resolved immediately, and return -EDEADLK. The locks are not placed
on the convert queue in the deadlocked state.
To fix this problem, an lkb under conversion between PR/CW is rebuilt
during recovery on a new master's granted queue, with the currently
granted mode, rather than being rebuilt on the new master's convert
queue, with the currently granted mode and the newly requested mode.
The in-progress convert is then resent to the new master after
recovery, so the conversion deadlock will be processed outside of
the recovery context and handled as described above.
Signed-off-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: David Teigland <teigland@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me provide my detailed analysis.
---
## Detailed Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit subject clearly says "fix" and the message describes a
concrete, reproducible bug:
- **Symptom**: During a workload involving conversions between lock
modes PR (Protected Read) and CW (Concurrent Write), lock recovery
creates a "conversion deadlock" state between recovered locks.
- **Visible effects**: Kernel warning messages (`"dlm: WARN: pending
deadlock..."`) and permanently stuck lock conversions on the convert
queue that never complete.
- **This affects clustered filesystem users** (GFS2, OCFS2) running on
systems where DLM node recovery occurs during PR/CW conversion
workloads.
The authors are **Alexander Aring** (Red Hat, DLM subsystem developer)
and **David Teigland** (the DLM maintainer who signs off all DLM
patches). This is authoritative.
### 2. CODE CHANGE ANALYSIS
The change is in `recover_convert_waiter()` in `fs/dlm/lock.c`. Here's
what changed:
**Before (buggy code introduced by f74dacb4):**
```5017:5035:fs/dlm/lock.c
if (middle_conversion(lkb)) {
log_rinfo(ls, "%s %x middle convert in progress",
__func__,
lkb->lkb_id);
/* We sent this lock to the new master. The new master
will
- tell us when it's granted. We no longer need a reply, so
- use a fake reply to put the lkb into the right state.
*/
hold_lkb(lkb);
memset(ms_local, 0, sizeof(struct dlm_message));
ms_local->m_type = cpu_to_le32(DLM_MSG_CONVERT_REPLY);
ms_local->m_result =
cpu_to_le32(to_dlm_errno(-EINPROGRESS));
ms_local->m_header.h_nodeid =
cpu_to_le32(lkb->lkb_nodeid);
_receive_convert_reply(lkb, ms_local, true);
unhold_lkb(lkb);
} else if (lkb->lkb_rqmode >= lkb->lkb_grmode) {
set_bit(DLM_IFL_RESEND_BIT, &lkb->lkb_iflags);
}
```
**After (the fix):**
```c
if (middle_conversion(lkb) || lkb->lkb_rqmode >=
lkb->lkb_grmode)
set_bit(DLM_IFL_RESEND_BIT, &lkb->lkb_iflags);
```
### 3. BUG MECHANISM
The recovery sequence in `fs/dlm/recoverd.c` runs these steps in order:
1. **Line 218**: `dlm_recover_waiters_pre()` — handles lkbs waiting for
replies from failed nodes
2. **Line 247**: `dlm_recover_locks()` — sends locks to new masters (via
rcom)
3. **Line 270**: `dlm_recover_rsbs()` — calls `recover_conversion()` on
flagged resources
4. **Line 320**: `dlm_recover_waiters_post()` — resends operations
marked with RESEND bit
**The bug in the old code**: When `recover_convert_waiter` faked an
`-EINPROGRESS` reply for a middle conversion:
- `_receive_convert_reply()` → `__receive_convert_reply()` would handle
the `-EINPROGRESS` case by calling `del_lkb(r, lkb)` + `add_lkb(r,
lkb, DLM_LKSTS_CONVERT)`, moving the lkb to the **local convert
queue**
- The lkb was also removed from the waiters list via
`remove_from_waiters_ms()`
- When the lock was subsequently sent to the **new master** via rcom
(step 2), it was sent as a converting lock
- `receive_rcom_lock_args()` on the new master placed it on the convert
queue and set `RSB_RECOVER_CONVERT`
- If **two locks** had middle conversions (e.g., A: PR→CW, B: CW→PR),
both ended up on the convert queue of the new master resource in a
**deadlocked state**
- `recover_conversion()` couldn't resolve this; the normal `-EDEADLK`
detection doesn't run during recovery
- Result: permanent deadlock, kernel warnings, stuck I/O
**How the fix resolves it**: By setting `DLM_IFL_RESEND_BIT` instead of
faking `-EINPROGRESS`:
- The lkb stays on the waiters list with its original granted status
unchanged
- When sent to the new master via rcom, it's sent as a **granted lock**
(not converting), placed on the **granted queue**
- No conversion deadlock can form during recovery
- After recovery, `dlm_recover_waiters_post()` finds the lkb (via
`find_resend_waiter`), resets its state, and calls `_convert_lock()`
to resend the conversion to the new master through **normal channels**
- If the conversion would deadlock, the normal code returns `-EDEADLK`
immediately, preventing the stuck state
### 4. CLASSIFICATION
This is a **clear bug fix** — it fixes a conversion deadlock in DLM lock
recovery. There are no new features, APIs, or behavioral changes. The
fix actually **removes** complex code and replaces it with the simpler,
correct approach.
### 5. SCOPE AND RISK ASSESSMENT
- **Lines changed**: Net removal of ~17 lines; replaces ~20 lines with 2
lines
- **Files changed**: 1 (`fs/dlm/lock.c`)
- **Function modified**: Only `recover_convert_waiter()`
- **Risk**: **Very low** — the fix simplifies the code and uses the same
`DLM_IFL_RESEND_BIT` mechanism that is already used for normal up-
conversions (REQUEST, LOOKUP, and non-middle CONVERT cases). The
RESEND path via `dlm_recover_waiters_post()` → `_convert_lock()` is
well-tested existing code.
- **Subsystem**: DLM (fs/dlm/) — critical for clustered filesystems
### 6. USER IMPACT
- **Who is affected**: Users of DLM-based clustered filesystems (GFS2,
OCFS2) in production clusters
- **Severity**: **High** — when the bug triggers, lock conversions
permanently hang, which would block I/O on clustered filesystems
- **Trigger**: PR↔CW conversions during node recovery (when a cluster
node fails). This is a realistic scenario in production clusters.
### 7. DEPENDENCY CHECK
- **Requires f74dacb4** ("dlm: fix recovery of middle conversions"):
This commit introduced the code being modified (the fake -EINPROGRESS
approach). f74dacb4 entered mainline in v6.13-rc1 and is present in
v6.13+ stable trees.
- **Does NOT require a8abcff1** ("dlm: move to rinfo for all middle
conversion cases"): a8abcff1 only changed `log_limit` → `log_rinfo` in
`receive_rcom_lock_args` and `recover_conversion`, not in
`recover_convert_waiter`. The code being removed already used
`log_rinfo` since f74dacb4.
- The patch should apply cleanly on any tree containing f74dacb4.
### 8. STABILITY INDICATORS
- Authored and signed-off by the DLM maintainers (Aring + Teigland)
- The fix simplifies code (reduction in complexity = lower regression
risk)
- Uses existing well-tested RESEND mechanism
- The commit message thoroughly explains the problem and solution
### Summary
This is a clear-cut bug fix for a conversion deadlock in DLM lock
recovery that causes permanent hangs and kernel warnings. The fix is
small, surgical (net code removal), contained to a single function in a
single file, and uses existing well-tested mechanisms. It affects
production clustered filesystem users. The prerequisite commit
(f74dacb4) is in v6.13+ stable trees, so this fix is needed in those
same trees. The risk is very low — the fix actually simplifies the
recovery path.
**YES**
fs/dlm/lock.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index be938fdf17d96..c01a291db401b 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -5014,25 +5014,8 @@ void dlm_receive_buffer(const union dlm_packet *p, int nodeid)
static void recover_convert_waiter(struct dlm_ls *ls, struct dlm_lkb *lkb,
struct dlm_message *ms_local)
{
- if (middle_conversion(lkb)) {
- log_rinfo(ls, "%s %x middle convert in progress", __func__,
- lkb->lkb_id);
-
- /* We sent this lock to the new master. The new master will
- * tell us when it's granted. We no longer need a reply, so
- * use a fake reply to put the lkb into the right state.
- */
- hold_lkb(lkb);
- memset(ms_local, 0, sizeof(struct dlm_message));
- ms_local->m_type = cpu_to_le32(DLM_MSG_CONVERT_REPLY);
- ms_local->m_result = cpu_to_le32(to_dlm_errno(-EINPROGRESS));
- ms_local->m_header.h_nodeid = cpu_to_le32(lkb->lkb_nodeid);
- _receive_convert_reply(lkb, ms_local, true);
- unhold_lkb(lkb);
-
- } else if (lkb->lkb_rqmode >= lkb->lkb_grmode) {
+ if (middle_conversion(lkb) || lkb->lkb_rqmode >= lkb->lkb_grmode)
set_bit(DLM_IFL_RESEND_BIT, &lkb->lkb_iflags);
- }
/* lkb->lkb_rqmode < lkb->lkb_grmode shouldn't happen since down
conversions are async; there's no reply from the remote master */
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.6] smb: client: prevent races in ->query_interfaces()
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-5.15] gfs2: fiemap page fault fix Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] dlm: fix recovery pending middle conversion Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Ensure proper bus clean-up Sasha Levin
` (20 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Henrique Carvalho, Steve French, Sasha Levin, sfrench, linux-cifs,
samba-technical
From: Henrique Carvalho <henrique.carvalho@suse.com>
[ Upstream commit c3c06e42e1527716c54f3ad2ced6a034b5f3a489 ]
It was possible for two query interface works to be concurrently trying
to update the interfaces.
Prevent this by checking and updating iface_last_update under
iface_lock.
Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me summarize my analysis.
---
## Detailed Analysis
### 1. Commit Message Analysis
The commit message clearly describes a **race condition**: "It was
possible for two query interface works to be concurrently trying to
update the interfaces." The fix is to check and update
`iface_last_update` atomically under `iface_lock`. Authored by Henrique
Carvalho (SUSE) and signed off by Steve French (CIFS maintainer) - both
trusted SMB subsystem contributors.
### 2. The Bug: TOCTOU Race on `iface_last_update`
The `iface_last_update` field is explicitly documented in `cifsglob.h`
as **protected by `iface_lock`**:
```1119:1123:fs/smb/client/cifsglob.h
/* ========= begin: protected by iface_lock ======== */
struct list_head iface_list;
size_t iface_count;
unsigned long iface_last_update; /* jiffies */
/* ========= end: protected by iface_lock ======== */
```
And confirmed in the lock ordering documentation:
```2000:2002:fs/smb/client/cifsglob.h
- cifs_ses->iface_lock cifs_ses->iface_list
sesInfoAlloc
- ->iface_count
- ->iface_last_update
```
**Before the fix**, the code violated this contract in two ways:
1. **`SMB3_request_interfaces()` (line 828-831)**: Read
`iface_last_update` **without any lock**:
```827:831:fs/smb/client/smb2ops.c
/* do not query too frequently */
if (ses->iface_last_update &&
time_before(jiffies, ses->iface_last_update +
(SMB_INTERFACE_POLL_INTERVAL * HZ)))
return 0;
```
2. **`parse_server_interfaces()` (line 798)**: Wrote `iface_last_update
= jiffies` **without the lock** at the end of processing.
The second check inside `parse_server_interfaces()` (line 641-646, under
`iface_lock`) was meant to catch races, but it came **after** the
expensive `SMB2_ioctl()` network call had already been made.
### 3. Race Scenario
`SMB3_request_interfaces()` can be called from **multiple concurrent
paths**:
- `smb2_query_server_interfaces()` - periodic delayed work (every 600s)
- `smb2_reconnect()` (via `query_server_interfaces` function pointer) -
reconnection path
- `smb3_qfs_tcon()` - during mount
**Race sequence**:
1. Thread A reads `iface_last_update` (no lock) → timer expired →
proceeds to `SMB2_ioctl()`
2. Thread B reads `iface_last_update` (no lock) → same stale value →
also proceeds to `SMB2_ioctl()`
3. Both threads enter `parse_server_interfaces()` concurrently
4. Thread A takes `iface_lock`, marks ALL interfaces as `is_active = 0`,
drops lock
5. Thread B takes `iface_lock`, marks ALL interfaces as `is_active = 0`
again, drops lock
6. Both threads independently iterate the buffer, both try to match/add
interfaces
7. In the cleanup (`out:` label), both threads iterate the list and call
`kref_put()` on inactive entries
**Consequences**:
- **Double `kref_put()` / use-after-free**: Both threads may identify
the same interface as inactive and call `kref_put()` on it,
potentially dropping the refcount below zero or causing UAF
- **Corrupted `iface_count`**: Both threads decrement `ses->iface_count`
for the same interface
- **List corruption**: While individual list operations are under
`iface_lock`, the mark-inactive → process → cleanup sequence is NOT
atomic, leading to incorrect state
- **Unnecessary duplicate network I/O**: Both threads perform
`SMB2_ioctl()` when only one is needed
### 4. The Fix
The fix is elegant and minimal (8 insertions, 11 deletions, net -3
lines):
1. **In `SMB3_request_interfaces()`**: Wraps the `iface_last_update`
check **and** update in a single `spin_lock/spin_unlock` critical
section. Crucially, it sets `iface_last_update = jiffies` **before**
doing any work (optimistic update), ensuring the second thread will
see the updated timestamp and bail out immediately.
2. **In `parse_server_interfaces()`**: Removes the now-redundant
duplicate time check and the two `iface_last_update = jiffies`
assignments (one in the empty-list error path, one at end of
processing).
This converts the race-prone check-then-act sequence into an atomic
check-and-update.
### 5. Scope and Risk Assessment
- **Files changed**: 1 (`fs/smb/client/smb2ops.c`)
- **Lines changed**: 8 insertions, 11 deletions (net -3)
- **Subsystem**: SMB/CIFS client - filesystem, important for enterprise
users
- **Risk**: Very low - the change only adds proper locking around an
already-documented protected field
- **Side effect consideration**: Setting `iface_last_update` before the
work means that if `SMB2_ioctl()` fails, the timestamp is still
updated. However, this is acceptable because: (a) the old code also
set it on error paths (the `-EOPNOTSUPP` case set it), and (b) the
600-second polling interval means it will retry relatively soon
### 6. Stable Tree Applicability
The vulnerable code exists in:
- **v6.12**: Identical structure to mainline, should apply cleanly
- **v6.6**: Similar structure but has a slightly different
`iface_last_update` placement (inside the while loop, not at end of
function - due to commit 78e727e58e54e being a v6.8 era change). The
core race (unlocked check in `SMB3_request_interfaces`) exists and the
fix concept applies, but minor adaptation may be needed.
### 7. Dependencies
The fix is self-contained. It doesn't depend on any other commits. The
only context difference for older stable trees (v6.6) is the location of
`iface_last_update = jiffies` inside `parse_server_interfaces()`, but
the fix's approach (move checking+setting to the caller under lock,
remove from callee) works regardless.
### 8. Summary
This is a clear race condition fix in the SMB/CIFS client that:
- Fixes a documented locking violation (`iface_last_update` accessed
without `iface_lock`)
- Prevents potential use-after-free via double `kref_put()` on
interfaces
- Prevents interface list state corruption affecting SMB multichannel
- Is small (net -3 lines), surgical, single-file, and self-contained
- Affects an important subsystem (filesystem client) used widely in
enterprise environments
- Has very low regression risk
**YES**
fs/smb/client/smb2ops.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index c1aaf77e187b6..edfd6a4e87e8b 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -637,13 +637,6 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
p = buf;
spin_lock(&ses->iface_lock);
- /* do not query too frequently, this time with lock held */
- if (ses->iface_last_update &&
- time_before(jiffies, ses->iface_last_update +
- (SMB_INTERFACE_POLL_INTERVAL * HZ))) {
- spin_unlock(&ses->iface_lock);
- return 0;
- }
/*
* Go through iface_list and mark them as inactive
@@ -666,7 +659,6 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
"Empty network interface list returned by server %s\n",
ses->server->hostname);
rc = -EOPNOTSUPP;
- ses->iface_last_update = jiffies;
goto out;
}
@@ -795,8 +787,6 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
+ sizeof(p->Next) && p->Next))
cifs_dbg(VFS, "%s: incomplete interface info\n", __func__);
- ses->iface_last_update = jiffies;
-
out:
/*
* Go through the list again and put the inactive entries
@@ -825,10 +815,17 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_
struct TCP_Server_Info *pserver;
/* do not query too frequently */
+ spin_lock(&ses->iface_lock);
if (ses->iface_last_update &&
time_before(jiffies, ses->iface_last_update +
- (SMB_INTERFACE_POLL_INTERVAL * HZ)))
+ (SMB_INTERFACE_POLL_INTERVAL * HZ))) {
+ spin_unlock(&ses->iface_lock);
return 0;
+ }
+
+ ses->iface_last_update = jiffies;
+
+ spin_unlock(&ses->iface_lock);
rc = SMB2_ioctl(xid, tcon, NO_FILE_ID, NO_FILE_ID,
FSCTL_QUERY_NETWORK_INTERFACE_INFO,
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Ensure proper bus clean-up
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (2 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.6] smb: client: prevent races in ->query_interfaces() Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-11 7:56 ` Adrian Hunter
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-5.10] audit: add fchmodat2() to change attributes class Sasha Levin
` (19 subsequent siblings)
23 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Adrian Hunter, Frank Li, Alexandre Belloni, Sasha Levin,
jarkko.nikula, billy_tsai, quic_msavaliy, wsa+renesas
From: Adrian Hunter <adrian.hunter@intel.com>
[ Upstream commit 8bb96575883d3b201ce37046b3903ea1d2d50bbc ]
Wait for the bus to fully disable before proceeding, ensuring that no
operations are still in progress. Synchronize the IRQ handler only after
interrupt signals have been disabled. This approach also handles cases
where bus disable might fail, preventing race conditions and ensuring a
consistent shutdown sequence.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260113072702.16268-3-adrian.hunter@intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Good. `readx_poll_timeout` has been used in this file since its
introduction. The new `i3c_hci_bus_disable()` function uses the same
pattern. No compatibility issue.
---
## Detailed Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message clearly states the intent: "ensuring that no
operations are still in progress", "Synchronize the IRQ handler only
after interrupt signals have been disabled", "preventing race conditions
and ensuring a consistent shutdown sequence." This is unambiguously a
race condition and shutdown ordering fix.
**Author**: Adrian Hunter (Intel), a prolific and trusted kernel
developer.
**Reviewer**: Frank Li (NXP), an I3C subsystem reviewer.
**Maintainer**: Merged by Alexandre Belloni, the I3C subsystem
maintainer.
### 2. CODE CHANGE ANALYSIS - The Race Conditions
**Bug 1: Bus not fully disabled before cleanup**
The OLD `i3c_hci_bus_cleanup()` does:
```155:165:/home/sasha/linux-
autosel/drivers/i3c/master/mipi-i3c-hci/core.c
static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)
{
struct i3c_hci *hci = to_i3c_hci(m);
struct platform_device *pdev =
to_platform_device(m->dev.parent);
reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
synchronize_irq(platform_get_irq(pdev, 0));
hci->io->cleanup(hci);
if (hci->cmd == &mipi_i3c_hci_cmd_v1)
mipi_i3c_hci_dat_v1.cleanup(hci);
}
```
`reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE)` merely writes to the
register. The hardware may still be mid-operation. Without waiting for
hardware acknowledgment, the subsequent cleanup proceeds on a bus that
is still active, which can cause undefined hardware behavior.
The new `i3c_hci_bus_disable()` uses `readx_poll_timeout()` with a
generous 500ms timeout to wait until the `HC_CONTROL_BUS_ENABLE` bit is
confirmed cleared by the hardware, ensuring the bus has truly stopped.
**Bug 2: Use-after-free race in DMA cleanup path**
This is the critical bug. The old flow is:
1. `reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE)` - request bus disable
2. `synchronize_irq()` - waits for any currently-running IRQ handler to
finish
3. `hci_dma_cleanup()` starts - iterates rings, disabling per-ring
interrupt signals and freeing DMA buffers in the **same loop**
The race: `synchronize_irq()` only guarantees the *currently running*
handler has finished. It does **not** prevent new interrupts from
arriving. Since the top-level `INTR_SIGNAL_ENABLE` register is still
armed (it's only cleared inside `io->cleanup()`), the hardware can
deliver a new interrupt *after* `synchronize_irq()` returns.
If that happens, `i3c_hci_irq_handler()` runs, calls
`hci_dma_irq_handler()`, which accesses `rings->headers[i]` (including
`rh->ibi_status`, `rh->resp`, `rh->xfer`) - DMA memory that
`hci_dma_cleanup()` may be concurrently freeing with
`dma_free_coherent()`. This is a **use-after-free**.
The fix splits the DMA cleanup loop: first loop disables all ring
interrupt signals and ring control, then calls
`i3c_hci_sync_irq_inactive()` (which disables top-level interrupt
signals AND synchronize_irq), and only then the second loop frees DMA
resources.
**Bug 3: Same race in PIO cleanup path**
The `hci_pio_irq_handler()` accesses `pio->lock`, `pio->enabled_irqs`,
and other `hci_pio_data` fields. If an interrupt fires after
`synchronize_irq()` but before `hci_pio_cleanup()` disables PIO signals
and frees `pio`, the handler will access freed memory.
The fix inserts `i3c_hci_sync_irq_inactive()` after disabling PIO
interrupt signals and before freeing the `pio` structure.
### 3. CLASSIFICATION
This is a **race condition fix** preventing **use-after-free** during
driver cleanup/shutdown. It falls squarely in the category of fixes that
should be backported to stable.
### 4. SCOPE AND RISK ASSESSMENT
- **Files changed**: 4 files, all within the same driver
- **Lines added**: ~40 lines of new code (two helper functions + loop
restructuring)
- **Lines removed**: ~5 lines
- **Complexity**: Moderate but well-contained
- **Risk**: LOW - changes only affect the cleanup/shutdown path, using
established kernel patterns (`readx_poll_timeout`, `synchronize_irq`)
- **Subsystem**: I3C bus driver used by Intel and AMD hardware (active,
maintained)
### 5. USER IMPACT
- Affects users with MIPI I3C HCI hardware (Intel, AMD platforms via
ACPI)
- The bug could trigger during driver removal (module unload), system
shutdown, or suspend/resume cycles
- Consequence if triggered: kernel crash, memory corruption, or
undefined behavior from use-after-free
- The AMD ACPI entry `AMDI5017` shows this driver is used on real
production hardware
### 6. STABILITY INDICATORS
- **Reviewed-by**: Frank Li (NXP) - I3C expert
- **Merged by**: Alexandre Belloni - I3C maintainer
- Already in mainline (v6.19)
### 7. DEPENDENCY CHECK
The commit is **self-contained**:
- New functions `i3c_hci_bus_disable()` and
`i3c_hci_sync_irq_inactive()` are introduced and used only within this
commit
- Uses `readx_poll_timeout()` and `synchronize_irq()` which exist in all
stable kernels
- `<linux/iopoll.h>` is already included in `core.c`
- No dependency on other patches from the series (the other patches in
the series appear to be PCI-related, not core driver changes)
### 8. BACKPORT CONSIDERATIONS
For older stable trees that lack `9e23897bca622` ("Use physical device
pointer with DMA API"), the `dma_free_coherent` calls in
`hci_dma_cleanup` use `&hci->master.dev` instead of `rings->sysdev`.
However, this is in the *unchanged* part of the code (the second loop),
so it should apply cleanly or with minimal context adjustment.
### Final Assessment
This commit fixes a real race condition that can cause use-after-free
during driver shutdown/cleanup in the MIPI I3C HCI driver. The bug
mechanism is clear: interrupt handlers can fire after
`synchronize_irq()` returns because interrupt signal enable registers
weren't disabled first, leading to concurrent access of memory being
freed. The fix is well-structured, properly orders the shutdown sequence
(disable signals -> synchronize IRQ -> free resources), and is self-
contained with low regression risk.
**YES**
drivers/i3c/master/mipi-i3c-hci/core.c | 32 +++++++++++++++++++++++---
drivers/i3c/master/mipi-i3c-hci/dma.c | 7 ++++++
drivers/i3c/master/mipi-i3c-hci/hci.h | 1 +
drivers/i3c/master/mipi-i3c-hci/pio.c | 2 ++
4 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 607d77ab0e546..0a4d8c9968c9b 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -152,13 +152,39 @@ static int i3c_hci_bus_init(struct i3c_master_controller *m)
return 0;
}
+/* Bus disable should never fail, so be generous with the timeout */
+#define BUS_DISABLE_TIMEOUT_US (500 * USEC_PER_MSEC)
+
+static int i3c_hci_bus_disable(struct i3c_hci *hci)
+{
+ u32 regval;
+ int ret;
+
+ reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
+
+ /* Ensure controller is disabled */
+ ret = readx_poll_timeout(reg_read, HC_CONTROL, regval,
+ !(regval & HC_CONTROL_BUS_ENABLE), 0, BUS_DISABLE_TIMEOUT_US);
+ if (ret)
+ dev_err(&hci->master.dev, "%s: Failed to disable bus\n", __func__);
+
+ return ret;
+}
+
+void i3c_hci_sync_irq_inactive(struct i3c_hci *hci)
+{
+ struct platform_device *pdev = to_platform_device(hci->master.dev.parent);
+ int irq = platform_get_irq(pdev, 0);
+
+ reg_write(INTR_SIGNAL_ENABLE, 0x0);
+ synchronize_irq(irq);
+}
+
static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)
{
struct i3c_hci *hci = to_i3c_hci(m);
- struct platform_device *pdev = to_platform_device(m->dev.parent);
- reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
- synchronize_irq(platform_get_irq(pdev, 0));
+ i3c_hci_bus_disable(hci);
hci->io->cleanup(hci);
if (hci->cmd == &mipi_i3c_hci_cmd_v1)
mipi_i3c_hci_dat_v1.cleanup(hci);
diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index 951abfea5a6fd..7061c44243424 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -162,6 +162,13 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
rh_reg_write(INTR_SIGNAL_ENABLE, 0);
rh_reg_write(RING_CONTROL, 0);
+ }
+
+ i3c_hci_sync_irq_inactive(hci);
+
+ for (i = 0; i < rings->total; i++) {
+ rh = &rings->headers[i];
+
rh_reg_write(CR_SETUP, 0);
rh_reg_write(IBI_SETUP, 0);
diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
index 249ccb13c9092..5add9c68434bf 100644
--- a/drivers/i3c/master/mipi-i3c-hci/hci.h
+++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
@@ -147,5 +147,6 @@ void mipi_i3c_hci_pio_reset(struct i3c_hci *hci);
void mipi_i3c_hci_dct_index_reset(struct i3c_hci *hci);
void amd_set_od_pp_timing(struct i3c_hci *hci);
void amd_set_resp_buf_thld(struct i3c_hci *hci);
+void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
#endif
diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
index 710faa46a00fa..9bf6c3ba6bce9 100644
--- a/drivers/i3c/master/mipi-i3c-hci/pio.c
+++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
@@ -212,6 +212,8 @@ static void hci_pio_cleanup(struct i3c_hci *hci)
pio_reg_write(INTR_SIGNAL_ENABLE, 0x0);
+ i3c_hci_sync_irq_inactive(hci);
+
if (pio) {
dev_dbg(&hci->master.dev, "status = %#x/%#x",
pio_reg_read(INTR_STATUS), pio_reg_read(INTR_SIGNAL_ENABLE));
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Ensure proper bus clean-up
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Ensure proper bus clean-up Sasha Levin
@ 2026-02-11 7:56 ` Adrian Hunter
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Hunter @ 2026-02-11 7:56 UTC (permalink / raw)
To: Sasha Levin, patches, stable
Cc: Frank Li, Alexandre Belloni, billy_tsai, quic_msavaliy,
wsa+renesas
On 11/02/2026 01:30, Sasha Levin wrote:
> From: Adrian Hunter <adrian.hunter@intel.com>
>
> [ Upstream commit 8bb96575883d3b201ce37046b3903ea1d2d50bbc ]
>
> Wait for the bus to fully disable before proceeding, ensuring that no
> operations are still in progress. Synchronize the IRQ handler only after
> interrupt signals have been disabled. This approach also handles cases
> where bus disable might fail, preventing race conditions and ensuring a
> consistent shutdown sequence.
To expose the issues raised by AI, it would require either a misbehaving
I3C target device, or an I3C target device that attempts hot-join.
However the driver does not currently support hot-join.
Also, as I mentioned before:
This patch was part of a larger patch set to enable support for
Runtime PM. There is also no support for System Suspend at this
point. In my opinion, someone serious about I3C usage would want
power management support, and would therefore back port the
entire patch set (and the subsequent System PM patch set) to their
kernel.
Consequently, I doubt back porting this patch separately helps
anyone.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Link: https://patch.msgid.link/20260113072702.16268-3-adrian.hunter@intel.com
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>
> LLM Generated explanations, may be completely bogus:
>
> Good. `readx_poll_timeout` has been used in this file since its
> introduction. The new `i3c_hci_bus_disable()` function uses the same
> pattern. No compatibility issue.
>
> ---
>
> ## Detailed Analysis
>
> ### 1. COMMIT MESSAGE ANALYSIS
>
> The commit message clearly states the intent: "ensuring that no
> operations are still in progress", "Synchronize the IRQ handler only
> after interrupt signals have been disabled", "preventing race conditions
> and ensuring a consistent shutdown sequence." This is unambiguously a
> race condition and shutdown ordering fix.
>
> **Author**: Adrian Hunter (Intel), a prolific and trusted kernel
> developer.
> **Reviewer**: Frank Li (NXP), an I3C subsystem reviewer.
> **Maintainer**: Merged by Alexandre Belloni, the I3C subsystem
> maintainer.
>
> ### 2. CODE CHANGE ANALYSIS - The Race Conditions
>
> **Bug 1: Bus not fully disabled before cleanup**
>
> The OLD `i3c_hci_bus_cleanup()` does:
>
> ```155:165:/home/sasha/linux-
> autosel/drivers/i3c/master/mipi-i3c-hci/core.c
> static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)
> {
> struct i3c_hci *hci = to_i3c_hci(m);
> struct platform_device *pdev =
> to_platform_device(m->dev.parent);
>
> reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
> synchronize_irq(platform_get_irq(pdev, 0));
> hci->io->cleanup(hci);
> if (hci->cmd == &mipi_i3c_hci_cmd_v1)
> mipi_i3c_hci_dat_v1.cleanup(hci);
> }
> ```
>
> `reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE)` merely writes to the
> register. The hardware may still be mid-operation. Without waiting for
> hardware acknowledgment, the subsequent cleanup proceeds on a bus that
> is still active, which can cause undefined hardware behavior.
>
> The new `i3c_hci_bus_disable()` uses `readx_poll_timeout()` with a
> generous 500ms timeout to wait until the `HC_CONTROL_BUS_ENABLE` bit is
> confirmed cleared by the hardware, ensuring the bus has truly stopped.
>
> **Bug 2: Use-after-free race in DMA cleanup path**
>
> This is the critical bug. The old flow is:
>
> 1. `reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE)` - request bus disable
> 2. `synchronize_irq()` - waits for any currently-running IRQ handler to
> finish
> 3. `hci_dma_cleanup()` starts - iterates rings, disabling per-ring
> interrupt signals and freeing DMA buffers in the **same loop**
>
> The race: `synchronize_irq()` only guarantees the *currently running*
> handler has finished. It does **not** prevent new interrupts from
> arriving. Since the top-level `INTR_SIGNAL_ENABLE` register is still
> armed (it's only cleared inside `io->cleanup()`), the hardware can
> deliver a new interrupt *after* `synchronize_irq()` returns.
>
> If that happens, `i3c_hci_irq_handler()` runs, calls
> `hci_dma_irq_handler()`, which accesses `rings->headers[i]` (including
> `rh->ibi_status`, `rh->resp`, `rh->xfer`) - DMA memory that
> `hci_dma_cleanup()` may be concurrently freeing with
> `dma_free_coherent()`. This is a **use-after-free**.
>
> The fix splits the DMA cleanup loop: first loop disables all ring
> interrupt signals and ring control, then calls
> `i3c_hci_sync_irq_inactive()` (which disables top-level interrupt
> signals AND synchronize_irq), and only then the second loop frees DMA
> resources.
>
> **Bug 3: Same race in PIO cleanup path**
>
> The `hci_pio_irq_handler()` accesses `pio->lock`, `pio->enabled_irqs`,
> and other `hci_pio_data` fields. If an interrupt fires after
> `synchronize_irq()` but before `hci_pio_cleanup()` disables PIO signals
> and frees `pio`, the handler will access freed memory.
>
> The fix inserts `i3c_hci_sync_irq_inactive()` after disabling PIO
> interrupt signals and before freeing the `pio` structure.
>
> ### 3. CLASSIFICATION
>
> This is a **race condition fix** preventing **use-after-free** during
> driver cleanup/shutdown. It falls squarely in the category of fixes that
> should be backported to stable.
>
> ### 4. SCOPE AND RISK ASSESSMENT
>
> - **Files changed**: 4 files, all within the same driver
> - **Lines added**: ~40 lines of new code (two helper functions + loop
> restructuring)
> - **Lines removed**: ~5 lines
> - **Complexity**: Moderate but well-contained
> - **Risk**: LOW - changes only affect the cleanup/shutdown path, using
> established kernel patterns (`readx_poll_timeout`, `synchronize_irq`)
> - **Subsystem**: I3C bus driver used by Intel and AMD hardware (active,
> maintained)
>
> ### 5. USER IMPACT
>
> - Affects users with MIPI I3C HCI hardware (Intel, AMD platforms via
> ACPI)
> - The bug could trigger during driver removal (module unload), system
> shutdown, or suspend/resume cycles
> - Consequence if triggered: kernel crash, memory corruption, or
> undefined behavior from use-after-free
> - The AMD ACPI entry `AMDI5017` shows this driver is used on real
> production hardware
>
> ### 6. STABILITY INDICATORS
>
> - **Reviewed-by**: Frank Li (NXP) - I3C expert
> - **Merged by**: Alexandre Belloni - I3C maintainer
> - Already in mainline (v6.19)
>
> ### 7. DEPENDENCY CHECK
>
> The commit is **self-contained**:
> - New functions `i3c_hci_bus_disable()` and
> `i3c_hci_sync_irq_inactive()` are introduced and used only within this
> commit
> - Uses `readx_poll_timeout()` and `synchronize_irq()` which exist in all
> stable kernels
> - `<linux/iopoll.h>` is already included in `core.c`
> - No dependency on other patches from the series (the other patches in
> the series appear to be PCI-related, not core driver changes)
>
> ### 8. BACKPORT CONSIDERATIONS
>
> For older stable trees that lack `9e23897bca622` ("Use physical device
> pointer with DMA API"), the `dma_free_coherent` calls in
> `hci_dma_cleanup` use `&hci->master.dev` instead of `rings->sysdev`.
> However, this is in the *unchanged* part of the code (the second loop),
> so it should apply cleanly or with minimal context adjustment.
>
> ### Final Assessment
>
> This commit fixes a real race condition that can cause use-after-free
> during driver shutdown/cleanup in the MIPI I3C HCI driver. The bug
> mechanism is clear: interrupt handlers can fire after
> `synchronize_irq()` returns because interrupt signal enable registers
> weren't disabled first, leading to concurrent access of memory being
> freed. The fix is well-structured, properly orders the shutdown sequence
> (disable signals -> synchronize IRQ -> free resources), and is self-
> contained with low regression risk.
>
> **YES**
>
> drivers/i3c/master/mipi-i3c-hci/core.c | 32 +++++++++++++++++++++++---
> drivers/i3c/master/mipi-i3c-hci/dma.c | 7 ++++++
> drivers/i3c/master/mipi-i3c-hci/hci.h | 1 +
> drivers/i3c/master/mipi-i3c-hci/pio.c | 2 ++
> 4 files changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> index 607d77ab0e546..0a4d8c9968c9b 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> @@ -152,13 +152,39 @@ static int i3c_hci_bus_init(struct i3c_master_controller *m)
> return 0;
> }
>
> +/* Bus disable should never fail, so be generous with the timeout */
> +#define BUS_DISABLE_TIMEOUT_US (500 * USEC_PER_MSEC)
> +
> +static int i3c_hci_bus_disable(struct i3c_hci *hci)
> +{
> + u32 regval;
> + int ret;
> +
> + reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
> +
> + /* Ensure controller is disabled */
> + ret = readx_poll_timeout(reg_read, HC_CONTROL, regval,
> + !(regval & HC_CONTROL_BUS_ENABLE), 0, BUS_DISABLE_TIMEOUT_US);
> + if (ret)
> + dev_err(&hci->master.dev, "%s: Failed to disable bus\n", __func__);
> +
> + return ret;
> +}
> +
> +void i3c_hci_sync_irq_inactive(struct i3c_hci *hci)
> +{
> + struct platform_device *pdev = to_platform_device(hci->master.dev.parent);
> + int irq = platform_get_irq(pdev, 0);
> +
> + reg_write(INTR_SIGNAL_ENABLE, 0x0);
> + synchronize_irq(irq);
> +}
> +
> static void i3c_hci_bus_cleanup(struct i3c_master_controller *m)
> {
> struct i3c_hci *hci = to_i3c_hci(m);
> - struct platform_device *pdev = to_platform_device(m->dev.parent);
>
> - reg_clear(HC_CONTROL, HC_CONTROL_BUS_ENABLE);
> - synchronize_irq(platform_get_irq(pdev, 0));
> + i3c_hci_bus_disable(hci);
> hci->io->cleanup(hci);
> if (hci->cmd == &mipi_i3c_hci_cmd_v1)
> mipi_i3c_hci_dat_v1.cleanup(hci);
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index 951abfea5a6fd..7061c44243424 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -162,6 +162,13 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
>
> rh_reg_write(INTR_SIGNAL_ENABLE, 0);
> rh_reg_write(RING_CONTROL, 0);
> + }
> +
> + i3c_hci_sync_irq_inactive(hci);
> +
> + for (i = 0; i < rings->total; i++) {
> + rh = &rings->headers[i];
> +
> rh_reg_write(CR_SETUP, 0);
> rh_reg_write(IBI_SETUP, 0);
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h
> index 249ccb13c9092..5add9c68434bf 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h
> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h
> @@ -147,5 +147,6 @@ void mipi_i3c_hci_pio_reset(struct i3c_hci *hci);
> void mipi_i3c_hci_dct_index_reset(struct i3c_hci *hci);
> void amd_set_od_pp_timing(struct i3c_hci *hci);
> void amd_set_resp_buf_thld(struct i3c_hci *hci);
> +void i3c_hci_sync_irq_inactive(struct i3c_hci *hci);
>
> #endif
> diff --git a/drivers/i3c/master/mipi-i3c-hci/pio.c b/drivers/i3c/master/mipi-i3c-hci/pio.c
> index 710faa46a00fa..9bf6c3ba6bce9 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/pio.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/pio.c
> @@ -212,6 +212,8 @@ static void hci_pio_cleanup(struct i3c_hci *hci)
>
> pio_reg_write(INTR_SIGNAL_ENABLE, 0x0);
>
> + i3c_hci_sync_irq_inactive(hci);
> +
> if (pio) {
> dev_dbg(&hci->master.dev, "status = %#x/%#x",
> pio_reg_read(INTR_STATUS), pio_reg_read(INTR_SIGNAL_ENABLE));
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH AUTOSEL 6.19-5.10] audit: add fchmodat2() to change attributes class
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (3 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Ensure proper bus clean-up Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] btrfs: fallback to buffered IO if the data profile has duplication Sasha Levin
` (18 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Jeffrey Bencteux, Paul Moore, Sasha Levin, eparis, audit,
linux-arch
From: Jeffrey Bencteux <jeff@bencteux.fr>
[ Upstream commit 4f493a6079b588cf1f04ce5ed6cdad45ab0d53dc ]
fchmodat2(), introduced in version 6.6 is currently not in the change
attribute class of audit. Calling fchmodat2() to change a file
attribute in the same fashion than chmod() or fchmodat() will bypass
audit rules such as:
-w /tmp/test -p rwa -k test_rwa
The current patch adds fchmodat2() to the change attributes class.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me write up the full analysis.
---
## Detailed Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message is clear and explicit about the bug:
- **Problem**: `fchmodat2()`, introduced in kernel v6.6, was never added
to the audit subsystem's "change attributes" (chattr) class. This
means that any process calling `fchmodat2()` to change a file's
permissions **completely bypasses audit rules** that monitor attribute
changes (the `-p a` permission in audit rules, e.g., `-w /tmp/test -p
rwa -k test_rwa`).
- **Impact**: This is a **security audit bypass**. Users who deploy
audit rules to track attribute changes on sensitive files (a standard
security and compliance practice) will miss `fchmodat2()` calls
entirely. An attacker or any process using `fchmodat2()` instead of
`chmod()`/`fchmodat()` can change file permissions without triggering
any audit record.
- **Author**: Jeffrey Bencteux, signed off by Paul Moore (the audit
subsystem maintainer), which is a strong indicator of correctness.
### 2. CODE CHANGE ANALYSIS
The patch is **3 lines** in a single file:
```c
#ifdef __NR_fchmodat2
__NR_fchmodat2,
#endif
```
Added to `include/asm-generic/audit_change_attr.h`, right after the
existing `__NR_fchmodat` entry. This is the header file that is
`#include`d by the `chattr_class[]` arrays across all architectures
(x86, sparc, s390, powerpc, parisc, alpha, and generic lib/audit.c +
compat_audit.c). These arrays define which syscall numbers are in the
"change attributes" audit class.
The mechanism is straightforward:
- The `audit_match_perm()` function in `kernel/auditsc.c` (line 167-169)
checks if a syscall number (`ctx->major`) matches `AUDIT_CLASS_CHATTR`
when the audit rule has `AUDIT_PERM_ATTR` set.
- Without `fchmodat2` in the `chattr_class[]` array,
`audit_match_class(AUDIT_CLASS_CHATTR, __NR_fchmodat2)` returns 0, so
the rule never fires for `fchmodat2()`.
Critically, `fchmodat2()` (defined in `fs/open.c` line 704-708) calls
the same `do_fchmodat()` function as `fchmodat()` (line 710-714). They
are functionally identical for attribute changes - the only difference
is `fchmodat2` adds a `flags` parameter. So there's no question that
`fchmodat2` belongs in the chattr class.
### 3. CLASSIFICATION
This is a **security bug fix** — specifically, an audit bypass. It is
NOT a new feature. The `fchmodat2` syscall already exists; this merely
ensures the audit subsystem correctly classifies it alongside the
equivalent `fchmodat` syscall.
This fits squarely into the category of fixes that are critical for
stable:
- Environments running Linux audit (virtually all enterprise
deployments, compliance-regulated systems) are silently missing
attribute change events
- This is a **compliance gap** (PCI-DSS, HIPAA, SOX, FedRAMP all require
file integrity monitoring via audit)
- The `#ifdef __NR_fchmodat2` guard ensures it compiles safely on older
architectures where the syscall might not be defined
### 4. SCOPE AND RISK ASSESSMENT
- **Size**: 3 lines added, 1 file changed — the smallest possible change
- **Risk**: Near zero. The change only adds a syscall number to an
existing list. The `#ifdef` guard protects against the case where
`__NR_fchmodat2` is not defined. There is no behavioral change to any
existing code path.
- **Could this break anything?**: No. Adding a new entry to the
chattr_class array only means that `fchmodat2` syscalls will now
correctly trigger audit rules with attribute-change permissions.
There's no way this causes a regression — at worst, users would see
additional (correct) audit records they were previously missing.
### 5. USER IMPACT
- **Who is affected**: Every single system running Linux audit with
attribute-change monitoring rules on kernels 6.6+. This includes
enterprise distributions (RHEL, SLES, Ubuntu), compliance-critical
infrastructure, and security-sensitive deployments.
- **Severity**: HIGH — this is a silent security audit bypass. Users
believe they are monitoring attribute changes, but `fchmodat2()` slips
through undetected.
- **Real-world trigger**: glibc and other C libraries may route
`chmod()`-like calls through `fchmodat2()` if available. The commit
message explicitly gives an example of the audit rule being bypassed.
### 6. DEPENDENCY CHECK
- **fchmodat2 syscall exists in v6.6**: Confirmed — `__NR_fchmodat2` is
defined as syscall 452 in `include/uapi/asm-generic/unistd.h` since
v6.6.
- **Clean application to v6.6**: Confirmed — the context lines
(`__NR_fchownat`, `__NR_fchmodat`, `#endif`, `#ifdef __NR_chown32`)
are identical in v6.6's version of the file.
- **No other dependencies**: The patch is completely self-contained. It
doesn't depend on any other commit. The `setxattrat`/`removexattrat`
additions from commit 6140be90ec70c are in different positions in the
file and don't affect the context.
- **Applicable to**: 6.6.y, 6.1.y (if fchmodat2 was backported there —
it wasn't since fchmodat2 was introduced in 6.6), and all stable trees
≥6.6.
### 7. STABILITY INDICATORS
- Signed off by Paul Moore, the audit subsystem maintainer
- The pattern is well-established — the same file has entries for every
attribute-changing syscall, each guarded by `#ifdef __NR_*`
- The commit is in the `security-next` tree, in the pipeline for
mainline
### 8. COMPARISON WITH PRECEDENT
The same file has been modified before for identical reasons:
- `262fd3aad6110` ("audit: Modify a set of system calls in audit class
definitions") — added syscalls to audit classes
- `af2951325bd6c` ("audit: make link()/linkat() match 'attribute change'
predicate") — added `link`/`linkat` to the same class
- `6140be90ec70c` ("fs/xattr: add *at family syscalls") — added
`setxattrat`/`removexattrat` to the same header
All of these follow the exact same pattern of adding a syscall number to
an audit class list.
### Summary
This is a textbook stable backport candidate:
1. **Obviously correct**: It adds 3 lines that are structurally
identical to the existing `fchmodat` entry, guarded by `#ifdef`
2. **Fixes a real bug**: Silent audit bypass of attribute changes via
`fchmodat2()`
3. **Security impact**: Allows processes to bypass audit monitoring —
this is a security-relevant gap
4. **Small and contained**: 3 lines, 1 file, no behavioral side effects
5. **No new features**: Just ensures existing audit infrastructure
correctly covers an existing syscall
6. **Applies cleanly to stable**: Context matches v6.6 exactly; no
dependencies
7. **Signed off by subsystem maintainer**: Paul Moore (audit maintainer)
The fix addresses a security audit bypass that has existed since v6.6
(when `fchmodat2` was introduced) — roughly 2+ years of every stable
kernel silently failing to audit attribute changes made via
`fchmodat2()`. This is particularly critical for enterprise and
compliance-regulated environments.
**YES**
include/asm-generic/audit_change_attr.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/asm-generic/audit_change_attr.h b/include/asm-generic/audit_change_attr.h
index cc840537885fb..ddd90bbe40dfc 100644
--- a/include/asm-generic/audit_change_attr.h
+++ b/include/asm-generic/audit_change_attr.h
@@ -26,6 +26,9 @@ __NR_fremovexattr,
__NR_fchownat,
__NR_fchmodat,
#endif
+#ifdef __NR_fchmodat2
+__NR_fchmodat2,
+#endif
#ifdef __NR_chown32
__NR_chown32,
__NR_fchown32,
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.12] btrfs: fallback to buffered IO if the data profile has duplication
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (4 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-5.10] audit: add fchmodat2() to change attributes class Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19] btrfs: don't BUG() on unexpected delayed ref type in run_one_delayed_ref() Sasha Levin
` (17 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable; +Cc: Qu Wenruo, David Sterba, Sasha Levin, clm, linux-btrfs
From: Qu Wenruo <wqu@suse.com>
[ Upstream commit 7c2830f00c3e086292c1ee9f27b61efaf8e76c9a ]
[BACKGROUND]
Inspired by a recent kernel bug report, which is related to direct IO
buffer modification during writeback, that leads to contents mismatch of
different RAID1 mirrors.
[CAUSE AND PROBLEMS]
The root cause is exactly the same explained in commit 968f19c5b1b7
("btrfs: always fallback to buffered write if the inode requires
checksum"), that we can not trust direct IO buffer which can be modified
halfway during writeback.
Unlike data checksum verification, if this happened on inodes without
data checksum but has the data has extra mirrors, it will lead to
stealth data mismatch on different mirrors.
This will be way harder to detect without data checksum.
Furthermore for RAID56, we can even have data without checksum and data
with checksum mixed inside the same full stripe.
In that case if the direct IO buffer got changed halfway for the
nodatasum part, the data with checksum immediately lost its ability to
recover, e.g.:
" " = Good old data or parity calculated using good old data
"X" = Data modified during writeback
0 32K 64K
Data 1 | | Has csum
Data 2 |XXXXXXXXXXXXXXXX | No csum
Parity | |
In above case, the parity is calculated using data 1 (has csum, from
page cache, won't change during writeback), and old data 2 (has no csum,
direct IO write).
After parity is calculated, but before submission to the storage, direct
IO buffer of data 2 is modified, causing the range [0, 32K) of data 2
has a different content.
Now all data is submitted to the storage, and the fs got fully synced.
Then the device of data 1 is lost, has to be rebuilt from data 2 and
parity. But since the data 2 has some modified data, and the parity is
calculated using old data, the recovered data is no the same for data 1,
causing data checksum mismatch.
[FIX]
Fix the problem by checking the data allocation profile.
If our data allocation profile is either RAID0 or SINGLE, we can allow
true zero-copy direct IO and the end user is fully responsible for any
race.
However this is not going to fix all situations, as it's still possible
to race with balance where the fs got a new data profile after the data
allocation profile check.
But this fix should still greatly reduce the window of the original bug.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=99171
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Here is my detailed analysis:
---
## Commit Analysis: "btrfs: fallback to buffered IO if the data profile
has duplication"
### 1. COMMIT MESSAGE ANALYSIS
The commit has an extremely detailed and well-structured message with
[BACKGROUND], [CAUSE AND PROBLEMS], and [FIX] sections. Key points:
- **Root cause**: Direct IO buffers can be modified by userspace during
writeback. This is a known issue — the same root cause was addressed
for checksummed inodes by commit 968f19c5b1b7.
- **Impact**: On filesystems with redundant data profiles (RAID1,
RAID5/6, DUP, RAID10, RAID1C3/C4), modified DIO buffers cause **silent
data mismatch across mirrors**. This is *data corruption* — different
mirrors/copies hold different content.
- **RAID56 amplification**: For RAID56, the problem is worse — corrupted
parity renders checksummed data unrecoverable, since parity is
calculated from pre-modification data but the actual written data may
differ.
- **Referenced bug**: [bugzilla.kernel.org
#99171](https://bugzilla.kernel.org/show_bug.cgi?id=99171) — a long-
standing issue dating back to 2015.
- **Authors**: Qu Wenruo (key btrfs developer), signed-off by David
Sterba (btrfs maintainer).
### 2. CODE CHANGE ANALYSIS
The patch is **12 lines, single file** (`fs/btrfs/direct-io.c`),
touching only `btrfs_direct_write()`:
**Variable declaration added:**
```c
const u64 data_profile = btrfs_data_alloc_profile(fs_info) &
BTRFS_BLOCK_GROUP_PROFILE_MASK;
```
**Early bailout check added before `relock:` label:**
```c
if (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0)
goto buffered;
```
The logic is:
- `btrfs_data_alloc_profile()` returns the current data allocation
profile, which includes type + profile bits
- After masking with `BTRFS_BLOCK_GROUP_PROFILE_MASK`, the result is:
- `0` for SINGLE (no profile bits)
- `BTRFS_BLOCK_GROUP_RAID0` for RAID0
- Other values for RAID1/DUP/RAID5/RAID6/RAID10/RAID1C3/RAID1C4
- Only SINGLE and RAID0 are allowed through to direct IO — these have no
data duplication
- All other profiles fall back to `buffered:` where the kernel controls
the page cache
The `goto buffered` jumps to the existing `buffered:` label (line 965 in
current code) that handles the buffered write fallback, including NOWAIT
handling. This is safe because the inode lock has NOT been taken yet at
this point.
### 3. THE BUG MECHANISM IN DETAIL
The bug occurs in this sequence:
1. Application opens a file with O_DIRECT on a btrfs filesystem with
RAID1/RAID5/RAID6/DUP data profile
2. Application issues a direct IO write
3. btrfs begins writing the data to multiple mirrors/parity
4. During writeback (between when the data is read from the user buffer
and when it's written to disk), another thread or the application
itself modifies the DIO buffer
5. Different mirrors receive different content — some get the original
data, some get the modified data
6. For NODATASUM inodes, there's **no checksum to detect this mismatch**
7. If a drive fails and rebuild is needed, the wrong data may be
reconstructed
This is *silent data corruption* — the most insidious kind, since the
user doesn't know it happened.
### 4. CLASSIFICATION
This is a **data corruption fix**. It prevents silent data mismatch on
RAID mirrors caused by DIO buffer modification during writeback. This is
firmly in the "data corruption" category of stable-worthy fixes.
### 5. SCOPE AND RISK ASSESSMENT
- **Lines changed**: 12 added, 0 removed — extremely small
- **Files touched**: 1 (`fs/btrfs/direct-io.c`)
- **Risk of regression**: LOW. The worst case is a performance
regression — direct IO falls back to buffered IO on RAID
configurations. But:
- The buffered IO path is well-tested and reliable
- Users who need zero-copy DIO on RAID can still use NODATASUM (it was
already the common practice)
- Data integrity is more important than performance
- **Subsystem maturity**: btrfs is mature; the `btrfs_direct_write()`
function is well-understood
### 6. DEPENDENCY ANALYSIS
- **`btrfs_data_alloc_profile()`**: Exists since v5.4-rc1 (commit
878d7b679491). Available in ALL active stable trees (5.15.y, 6.1.y,
6.6.y, 6.12.y).
- **`BTRFS_BLOCK_GROUP_PROFILE_MASK`**: Exists in
`include/uapi/linux/btrfs_tree.h` since very early btrfs — available
everywhere.
- **`buffered:` label**: Exists in `btrfs_direct_write()` since the
function was written — available everywhere.
- **Commit 968f19c5b1b7**: IS an ancestor of this commit (confirmed).
However, this patch does NOT strictly depend on it — the new check is
placed BEFORE `relock:`, before the inode lock is taken, and uses
`goto buffered` which existed before 968f. The two fixes are
complementary but independent:
- 968f handles checksummed inodes (checksum mismatch)
- This commit handles redundant profiles (mirror mismatch)
- **File location**: On v6.12+, code is in `fs/btrfs/direct-io.c`. On
v6.6 and earlier, the direct IO code was in `fs/btrfs/inode.c` (moved
by commit 9aa29a20b7009 in v6.7). Backports to v6.6 and earlier would
need path adjustment.
### 7. USER IMPACT
- **Who is affected**: Anyone using btrfs with RAID1, RAID10, RAID5,
RAID6, or DUP data profiles and direct IO writes. This is a **very
common scenario** — VM images on btrfs RAID1 is a standard deployment
pattern.
- **Severity**: **Data corruption** — silent mirror mismatch that can
lead to data loss on rebuild.
- **How long the bug has existed**: Since btrfs RAID support was added.
The bugzilla (#99171) was filed in 2015.
- **Acknowledgment in commit**: The author acknowledges this is an
incomplete fix (race with balance can still occur), but it "greatly
reduces the window."
### 8. STABILITY INDICATORS
- Author: Qu Wenruo — prolific btrfs developer, deeply understands the
subsystem
- Committer: David Sterba — btrfs maintainer, reviewed and accepted
- Committed 2026-01-29, authored 2025-11-01 — has had time for review
- The commit is already on multiple autosel candidate branches,
confirming interest in backporting
### Concerns
1. **Performance impact**: Users on RAID configurations who previously
got zero-copy DIO will now get buffered IO. This is a behavioral
change, though documented and deliberate.
2. **Incomplete fix**: The commit acknowledges a remaining race with
`balance` changing profiles. But a partial fix that "greatly reduces
the window" is better than no fix.
3. **Needs 968f19c5b1b7 for clean apply**: The diff context includes
code from 968f. However, the fix can be adapted to apply without it —
only the surrounding context differs. The core logic (profile check +
goto buffered) is self-contained.
### Verdict
This commit fixes **silent data corruption** on btrfs RAID
configurations — one of the most serious categories of bugs. The fix is
minimal (12 lines), self-contained, obviously correct, and has zero risk
of introducing new crashes or instability (worst case: performance
regression from buffered IO fallback). The bug has been known since 2015
(bugzilla #99171) and affects a common use case. The author and
committer are the primary btrfs maintainers.
**YES**
fs/btrfs/direct-io.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
index 07e19e88ba4b3..5443d69efe956 100644
--- a/fs/btrfs/direct-io.c
+++ b/fs/btrfs/direct-io.c
@@ -814,6 +814,8 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
ssize_t ret;
unsigned int ilock_flags = 0;
struct iomap_dio *dio;
+ const u64 data_profile = btrfs_data_alloc_profile(fs_info) &
+ BTRFS_BLOCK_GROUP_PROFILE_MASK;
if (iocb->ki_flags & IOCB_NOWAIT)
ilock_flags |= BTRFS_ILOCK_TRY;
@@ -827,6 +829,16 @@ ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode) && IS_NOSEC(inode))
ilock_flags |= BTRFS_ILOCK_SHARED;
+ /*
+ * If our data profile has duplication (either extra mirrors or RAID56),
+ * we can not trust the direct IO buffer, the content may change during
+ * writeback and cause different contents written to different mirrors.
+ *
+ * Thus only RAID0 and SINGLE can go true zero-copy direct IO.
+ */
+ if (data_profile != BTRFS_BLOCK_GROUP_RAID0 && data_profile != 0)
+ goto buffered;
+
relock:
ret = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
if (ret < 0)
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19] btrfs: don't BUG() on unexpected delayed ref type in run_one_delayed_ref()
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (5 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] btrfs: fallback to buffered IO if the data profile has duplication Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci-pci: Add System Suspend support Sasha Levin
` (16 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Filipe Manana, Boris Burkov, Qu Wenruo, David Sterba, Sasha Levin,
clm, linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
[ Upstream commit c7d1d4ff56744074e005771aff193b927392d51f ]
There is no need to BUG(), we can just return an error and log an error
message.
Reviewed-by: Boris Burkov <boris@bur.io>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Those `BUG()` calls in the sub-functions (`run_delayed_data_ref` line
1602 and `run_delayed_tree_ref` line 1752) are for unexpected
`node->action` values, not `node->type` values. The commit we're
analyzing only addresses the `node->type` BUG() in
`run_one_delayed_ref`. These are separate concerns and independently
addressable.
## Analysis Summary
### What the commit does
This commit replaces a `BUG()` (unconditional kernel panic) with proper
error handling (`-EUCLEAN` return + `btrfs_err()` log message) in
`run_one_delayed_ref()` in `fs/btrfs/extent-tree.c`. The `BUG()`
triggers when a delayed ref node has an unexpected type value that
doesn't match any of the known reference key types
(`BTRFS_TREE_BLOCK_REF_KEY`, `BTRFS_SHARED_BLOCK_REF_KEY`,
`BTRFS_EXTENT_DATA_REF_KEY`, `BTRFS_SHARED_DATA_REF_KEY`, or
`BTRFS_EXTENT_OWNER_REF_KEY`).
### Why it matters
- **BUG() causes a complete kernel crash/panic.** The system becomes
unresponsive, potentially causing data loss if writes were in
progress, and requires a reboot.
- **With the fix**, the error returns -EUCLEAN, which propagates through
`btrfs_run_delayed_refs_for_head()` -> `__btrfs_run_delayed_refs()` ->
`btrfs_run_delayed_refs()`, where `btrfs_abort_transaction(trans,
ret)` is called. This gracefully aborts the transaction and puts the
filesystem into an error state, allowing the system to continue
operating (possibly in read-only mode for that filesystem) without a
full kernel crash.
### Trigger conditions
While the `node->type` field is typically populated correctly by
`btrfs_ref_type()` (which can only return 4 valid values), this BUG()
can be triggered by:
1. **Memory corruption** (e.g., hardware memory errors, other kernel
bugs corrupting memory)
2. **Future code bugs** that introduce a new type without handling it
here
3. **Unusual filesystem states** or logic bugs in the delayed ref
infrastructure
### Stable kernel criteria assessment
1. **Obviously correct and tested**: Yes. The patch is reviewed by 3
btrfs developers (Boris Burkov, Qu Wenruo, David Sterba). The error
path already exists and is exercised by other error codes. The change
is straightforward: `BUG()` -> `return -EUCLEAN` + error log.
2. **Fixes a real bug**: Yes. BUG() in a code path that can be triggered
is a bug - it causes an unnecessary kernel crash when graceful error
handling is possible. The entire call chain already handles errors
from this function correctly.
3. **Fixes an important issue**: Yes. A kernel panic/crash is a serious
issue. Converting to graceful error handling prevents data loss and
system downtime.
4. **Small and contained**: Yes. The change is approximately 10 lines in
a single function, in a single file. It only modifies error handling
behavior.
5. **No new features**: Correct. No new features or APIs are added. This
only changes how an error condition is handled.
6. **Applies cleanly**: For 6.12.y and later stable trees, it should
apply cleanly. For 6.6.y, the `BTRFS_EXTENT_OWNER_REF_KEY` branch
needs to be removed (it was introduced in v6.7), requiring a minor
backport adaptation.
### Precedent
Similar BUG_ON()/BUG() to -EUCLEAN conversions in the same file
(`fs/btrfs/extent-tree.c`) by the same author have been backported to
stable:
- `28cb13f29faf` ("btrfs: don't BUG_ON() when 0 reference count at
btrfs_lookup_extent_info()") - backported to 6.6.y
- `bb3868033a4c` ("btrfs: do not BUG_ON() when freeing tree block after
error") - backported to 6.6.y
### Risk assessment
**Very low risk.** The change only affects an error path that was
previously causing a kernel panic. The new behavior (returning -EUCLEAN)
is strictly better than the old behavior (BUG/crash). The error
propagation chain is already well-tested since other errors from
`run_one_delayed_ref` follow the exact same path.
### Dependencies
The commit is **self-contained**. It has no dependencies on other
commits. For older stable trees (pre-6.7), the
`BTRFS_EXTENT_OWNER_REF_KEY` branch wouldn't exist, requiring a trivial
adaptation.
**YES**
fs/btrfs/extent-tree.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index e4cae34620d19..1bf081243efb2 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1761,32 +1761,36 @@ static int run_one_delayed_ref(struct btrfs_trans_handle *trans,
struct btrfs_delayed_extent_op *extent_op,
bool insert_reserved)
{
+ struct btrfs_fs_info *fs_info = trans->fs_info;
int ret = 0;
if (TRANS_ABORTED(trans)) {
if (insert_reserved) {
btrfs_pin_extent(trans, node->bytenr, node->num_bytes);
- free_head_ref_squota_rsv(trans->fs_info, href);
+ free_head_ref_squota_rsv(fs_info, href);
}
return 0;
}
if (node->type == BTRFS_TREE_BLOCK_REF_KEY ||
- node->type == BTRFS_SHARED_BLOCK_REF_KEY)
+ node->type == BTRFS_SHARED_BLOCK_REF_KEY) {
ret = run_delayed_tree_ref(trans, href, node, extent_op,
insert_reserved);
- else if (node->type == BTRFS_EXTENT_DATA_REF_KEY ||
- node->type == BTRFS_SHARED_DATA_REF_KEY)
+ } else if (node->type == BTRFS_EXTENT_DATA_REF_KEY ||
+ node->type == BTRFS_SHARED_DATA_REF_KEY) {
ret = run_delayed_data_ref(trans, href, node, extent_op,
insert_reserved);
- else if (node->type == BTRFS_EXTENT_OWNER_REF_KEY)
+ } else if (node->type == BTRFS_EXTENT_OWNER_REF_KEY) {
ret = 0;
- else
- BUG();
+ } else {
+ ret = -EUCLEAN;
+ btrfs_err(fs_info, "unexpected delayed ref node type: %u", node->type);
+ }
+
if (ret && insert_reserved)
btrfs_pin_extent(trans, node->bytenr, node->num_bytes);
if (ret < 0)
- btrfs_err(trans->fs_info,
+ btrfs_err(fs_info,
"failed to run delayed ref for logical %llu num_bytes %llu type %u action %u ref_mod %d: %d",
node->bytenr, node->num_bytes, node->type,
node->action, node->ref_mod, ret);
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci-pci: Add System Suspend support
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (6 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19] btrfs: don't BUG() on unexpected delayed ref type in run_one_delayed_ref() Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-11 7:57 ` Adrian Hunter
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] hfsplus: fix volume corruption issue for generic/480 Sasha Levin
` (15 subsequent siblings)
23 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Adrian Hunter, Frank Li, Alexandre Belloni, Sasha Levin,
jarkko.nikula
From: Adrian Hunter <adrian.hunter@intel.com>
[ Upstream commit 4280197d154cae1d1d5acb54484da26da04eac32 ]
Assign the driver PM operations pointer, which is necessary for the PCI
subsystem to put the device into a low power state. Refer to
pci_pm_suspend_noirq() which bails out if the pointer is NULL, before
it has the opportunity to call pci_prepare_to_sleep().
No other actions are necessary as the mipi-i3c-hci driver takes care of
controller state.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260123063325.8210-4-adrian.hunter@intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me compile my detailed analysis.
---
## Comprehensive Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit subject "Add System Suspend support" makes this sound like a
new feature, but the body reveals the real story: it's fixing a bug
where the PCI subsystem **cannot put the device into a low power state**
during system suspend. The commit message specifically references
`pci_pm_suspend_noirq()` which "bails out if the pointer is NULL, before
it has the opportunity to call `pci_prepare_to_sleep()`."
### 2. CODE CHANGE ANALYSIS
The change is extremely minimal:
1. Adds an **empty** `struct dev_pm_ops` (no callbacks):
```c
static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
};
```
2. Assigns it to the PCI driver structure:
```c
.driver = {
.pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
},
```
### 3. THE BUG MECHANISM
The critical code path is in `pci_pm_suspend_noirq()`:
```863:866:drivers/pci/pci-driver.c
if (!pm) {
pci_save_state(pci_dev);
goto Fixup;
}
```
When `dev->driver->pm` is NULL (as in the current driver),
`pci_pm_suspend_noirq()` takes the early exit at line 863-866: it saves
PCI state but jumps to `Fixup`, **completely skipping** the call to
`pci_prepare_to_sleep()` at line 895:
```886:896:drivers/pci/pci-driver.c
if (!pci_dev->state_saved) {
pci_save_state(pci_dev);
/*
- If the device is a bridge with a child in D0 below it,
- it needs to stay in D0, so check skip_bus_pm to avoid
- putting it into a low-power state in that case.
*/
if (!pci_dev->skip_bus_pm &&
pci_power_manageable(pci_dev))
pci_prepare_to_sleep(pci_dev);
}
```
With the fix (non-NULL but empty `pm` ops), the code:
- Skips the NULL-pm early exit
- Skips the `pm->suspend_noirq` block (callback is NULL)
- Falls through to `pci_save_state()` AND `pci_prepare_to_sleep()`,
which transitions the device to D3hot/D3cold
### 4. USER IMPACT
Without this fix, the Intel I3C HCI PCI controller stays in **D0 (full
power)** during system suspend. This has real consequences:
- **Higher power consumption during suspend** - device draws power when
it shouldn't
- **May prevent platform from reaching deeper sleep states** (S0ix/S3)
on Intel platforms
- Affects Intel Panther Lake, Wildcat Lake, and Nova Lake platforms
This is a known pattern - other PCI drivers like `surface_dtx` and
`aggregator/core.c` use the same empty `dev_pm_ops` trick for exactly
this reason.
### 5. DRIVER EXISTENCE IN STABLE TREES
- **v6.14**: Driver first added (148 lines). No `.driver.pm` pointer.
- **v6.15, v6.16**: No changes to this file.
- **v6.17**: 1 commit (Wildcat Lake-U device IDs). Still no PM.
- **v6.18**: No changes.
- **v6.19**: 13 commits (major refactoring). Still no PM.
The bug exists in **ALL versions** of this driver. However, the diff
from the analyzed commit targets a codebase significantly different from
any released version (references `intel_mi_1_info`, `intel_si_2_info`,
`mfd_remove_devices`, `mipi_i3c_hci_pci_rpm_forbid` - none of which
exist in v6.14-v6.19). This means:
- The exact diff **will NOT apply cleanly** to any stable tree
- A manual adaptation would be needed, but it's **trivially simple**
(just add the empty struct and pointer to the existing `pci_driver`
struct)
### 6. CLASSIFICATION
This is a **bug fix**, not a new feature. System suspend is expected to
work correctly for any PCI device. The driver was missing a required
piece (`dev_pm_ops` pointer) that the PCI subsystem needs to properly
power-manage the device. The comment in the code even says "PM ops must
exist for PCI to put a device to a low power state."
### 7. RISK ASSESSMENT
- **Risk: Essentially zero.** The `dev_pm_ops` structure is completely
empty - no callbacks are defined. Its sole purpose is to not be NULL,
so the PCI core doesn't take the early-exit path.
- **Lines changed**: ~7 lines added
- **Files changed**: 1
- **Reviewed-by**: Frank Li (NXP)
### 8. CONCERNS
- **Dependency**: The exact diff won't apply cleanly to stable trees
(context mismatch). A trivial manual adaptation is needed.
- **Hardware newness**: The supported platforms (Panther Lake, etc.) are
new Intel hardware with potentially limited user base.
- **Not a crash/security bug**: This is a power management regression,
not a crash or data corruption.
### 9. VERDICT
The fix resolves a real power management bug that affects every user
with Intel I3C HCI hardware doing system suspend. The change is
trivially small, zero-risk, and follows an established pattern used by
other PCI drivers. While the hardware is new and the issue isn't a
crash, broken system suspend is a significant user-visible issue - users
with these platforms will see higher battery drain during suspend, and
the platform may not reach its desired sleep state.
The fix meets stable criteria: it's obviously correct, fixes a real bug,
is small and contained, and doesn't introduce new features. The empty PM
ops structure adds no new behavior - it simply allows the existing PCI
power management infrastructure to work as designed.
**YES**
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index dc8ede0f8ad8d..f8ac1ae371cc9 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -265,6 +265,10 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
ida_free(&mipi_i3c_hci_pci_ida, dev_id);
}
+/* PM ops must exist for PCI to put a device to a low power state */
+static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
+};
+
static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
/* Wildcat Lake-U */
{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_info},
@@ -287,6 +291,9 @@ static struct pci_driver mipi_i3c_hci_pci_driver = {
.id_table = mipi_i3c_hci_pci_devices,
.probe = mipi_i3c_hci_pci_probe,
.remove = mipi_i3c_hci_pci_remove,
+ .driver = {
+ .pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
+ },
};
module_pci_driver(mipi_i3c_hci_pci_driver);
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci-pci: Add System Suspend support
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci-pci: Add System Suspend support Sasha Levin
@ 2026-02-11 7:57 ` Adrian Hunter
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Hunter @ 2026-02-11 7:57 UTC (permalink / raw)
To: Sasha Levin, patches, stable; +Cc: Frank Li, Alexandre Belloni
On 11/02/2026 01:30, Sasha Levin wrote:
> From: Adrian Hunter <adrian.hunter@intel.com>
>
> [ Upstream commit 4280197d154cae1d1d5acb54484da26da04eac32 ]
>
> Assign the driver PM operations pointer, which is necessary for the PCI
> subsystem to put the device into a low power state. Refer to
> pci_pm_suspend_noirq() which bails out if the pointer is NULL, before
> it has the opportunity to call pci_prepare_to_sleep().
>
> No other actions are necessary as the mipi-i3c-hci driver takes care of
> controller state.
Back porting this patch by itself makes no sense. The entire patch set
and previous patch sets are needed to support power management.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Link: https://patch.msgid.link/20260123063325.8210-4-adrian.hunter@intel.com
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>
> LLM Generated explanations, may be completely bogus:
>
> Now I have a complete picture. Let me compile my detailed analysis.
>
> ---
>
> ## Comprehensive Analysis
>
> ### 1. COMMIT MESSAGE ANALYSIS
>
> The commit subject "Add System Suspend support" makes this sound like a
> new feature, but the body reveals the real story: it's fixing a bug
> where the PCI subsystem **cannot put the device into a low power state**
> during system suspend. The commit message specifically references
> `pci_pm_suspend_noirq()` which "bails out if the pointer is NULL, before
> it has the opportunity to call `pci_prepare_to_sleep()`."
>
> ### 2. CODE CHANGE ANALYSIS
>
> The change is extremely minimal:
> 1. Adds an **empty** `struct dev_pm_ops` (no callbacks):
> ```c
> static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
> };
> ```
> 2. Assigns it to the PCI driver structure:
> ```c
> .driver = {
> .pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
> },
> ```
>
> ### 3. THE BUG MECHANISM
>
> The critical code path is in `pci_pm_suspend_noirq()`:
>
> ```863:866:drivers/pci/pci-driver.c
> if (!pm) {
> pci_save_state(pci_dev);
> goto Fixup;
> }
> ```
>
> When `dev->driver->pm` is NULL (as in the current driver),
> `pci_pm_suspend_noirq()` takes the early exit at line 863-866: it saves
> PCI state but jumps to `Fixup`, **completely skipping** the call to
> `pci_prepare_to_sleep()` at line 895:
>
> ```886:896:drivers/pci/pci-driver.c
> if (!pci_dev->state_saved) {
> pci_save_state(pci_dev);
>
> /*
> - If the device is a bridge with a child in D0 below it,
> - it needs to stay in D0, so check skip_bus_pm to avoid
> - putting it into a low-power state in that case.
> */
> if (!pci_dev->skip_bus_pm &&
> pci_power_manageable(pci_dev))
> pci_prepare_to_sleep(pci_dev);
> }
> ```
>
> With the fix (non-NULL but empty `pm` ops), the code:
> - Skips the NULL-pm early exit
> - Skips the `pm->suspend_noirq` block (callback is NULL)
> - Falls through to `pci_save_state()` AND `pci_prepare_to_sleep()`,
> which transitions the device to D3hot/D3cold
>
> ### 4. USER IMPACT
>
> Without this fix, the Intel I3C HCI PCI controller stays in **D0 (full
> power)** during system suspend. This has real consequences:
> - **Higher power consumption during suspend** - device draws power when
> it shouldn't
> - **May prevent platform from reaching deeper sleep states** (S0ix/S3)
> on Intel platforms
> - Affects Intel Panther Lake, Wildcat Lake, and Nova Lake platforms
>
> This is a known pattern - other PCI drivers like `surface_dtx` and
> `aggregator/core.c` use the same empty `dev_pm_ops` trick for exactly
> this reason.
>
> ### 5. DRIVER EXISTENCE IN STABLE TREES
>
> - **v6.14**: Driver first added (148 lines). No `.driver.pm` pointer.
> - **v6.15, v6.16**: No changes to this file.
> - **v6.17**: 1 commit (Wildcat Lake-U device IDs). Still no PM.
> - **v6.18**: No changes.
> - **v6.19**: 13 commits (major refactoring). Still no PM.
>
> The bug exists in **ALL versions** of this driver. However, the diff
> from the analyzed commit targets a codebase significantly different from
> any released version (references `intel_mi_1_info`, `intel_si_2_info`,
> `mfd_remove_devices`, `mipi_i3c_hci_pci_rpm_forbid` - none of which
> exist in v6.14-v6.19). This means:
> - The exact diff **will NOT apply cleanly** to any stable tree
> - A manual adaptation would be needed, but it's **trivially simple**
> (just add the empty struct and pointer to the existing `pci_driver`
> struct)
>
> ### 6. CLASSIFICATION
>
> This is a **bug fix**, not a new feature. System suspend is expected to
> work correctly for any PCI device. The driver was missing a required
> piece (`dev_pm_ops` pointer) that the PCI subsystem needs to properly
> power-manage the device. The comment in the code even says "PM ops must
> exist for PCI to put a device to a low power state."
>
> ### 7. RISK ASSESSMENT
>
> - **Risk: Essentially zero.** The `dev_pm_ops` structure is completely
> empty - no callbacks are defined. Its sole purpose is to not be NULL,
> so the PCI core doesn't take the early-exit path.
> - **Lines changed**: ~7 lines added
> - **Files changed**: 1
> - **Reviewed-by**: Frank Li (NXP)
>
> ### 8. CONCERNS
>
> - **Dependency**: The exact diff won't apply cleanly to stable trees
> (context mismatch). A trivial manual adaptation is needed.
> - **Hardware newness**: The supported platforms (Panther Lake, etc.) are
> new Intel hardware with potentially limited user base.
> - **Not a crash/security bug**: This is a power management regression,
> not a crash or data corruption.
>
> ### 9. VERDICT
>
> The fix resolves a real power management bug that affects every user
> with Intel I3C HCI hardware doing system suspend. The change is
> trivially small, zero-risk, and follows an established pattern used by
> other PCI drivers. While the hardware is new and the issue isn't a
> crash, broken system suspend is a significant user-visible issue - users
> with these platforms will see higher battery drain during suspend, and
> the platform may not reach its desired sleep state.
>
> The fix meets stable criteria: it's obviously correct, fixes a real bug,
> is small and contained, and doesn't introduce new features. The empty PM
> ops structure adds no new behavior - it simply allows the existing PCI
> power management infrastructure to work as designed.
>
> **YES**
>
> drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> index dc8ede0f8ad8d..f8ac1ae371cc9 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> @@ -265,6 +265,10 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
> ida_free(&mipi_i3c_hci_pci_ida, dev_id);
> }
>
> +/* PM ops must exist for PCI to put a device to a low power state */
> +static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
> +};
> +
> static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
> /* Wildcat Lake-U */
> { PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_info},
> @@ -287,6 +291,9 @@ static struct pci_driver mipi_i3c_hci_pci_driver = {
> .id_table = mipi_i3c_hci_pci_devices,
> .probe = mipi_i3c_hci_pci_probe,
> .remove = mipi_i3c_hci_pci_remove,
> + .driver = {
> + .pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
> + },
> };
>
> module_pci_driver(mipi_i3c_hci_pci_driver);
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH AUTOSEL 6.19-6.18] hfsplus: fix volume corruption issue for generic/480
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (7 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci-pci: Add System Suspend support Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] kselftest/kublk: include message in _Static_assert for C11 compatibility Sasha Levin
` (14 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Viacheslav Dubeyko, John Paul Adrian Glaubitz, Yangtao Li,
linux-fsdevel, Sasha Levin
From: Viacheslav Dubeyko <slava@dubeyko.com>
[ Upstream commit bea4429eb30190c59b5ac7c8ff6c90176c7c110f ]
The xfstests' test-case generic/480 leaves HFS+ volume
in corrupted state:
sudo ./check generic/480
FSTYP -- hfsplus
PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.17.0-rc1+ #4 SMP PREEMPT_DYNAMIC Wed Oct 1 15:02:44 PDT 2025
MKFS_OPTIONS -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
generic/480 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent
(see XFSTESTS-2/xfstests-dev/results//generic/480.full for details)
Ran: generic/480
Failures: generic/480
Failed 1 of 1 tests
sudo fsck.hfsplus -d /dev/loop51
** /dev/loop51
Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K.
Executing fsck_hfs (version 540.1-Linux).
** Checking non-journaled HFS Plus Volume.
The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
** Checking multi-linked files.
CheckHardLinks: found 1 pre-Leopard file inodes.
Incorrect number of file hard links
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
invalid VHB nextCatalogID
Volume header needs minor repair
(2, 0)
Verify Status: VIStat = 0x8000, ABTStat = 0x0000 EBTStat = 0x0000
CBTStat = 0x0000 CatStat = 0x00000002
** Repairing volume.
Incorrect flags for file hard link (id = 19)
(It should be 0x22 instead of 0x2)
Incorrect flags for file inode (id = 18)
(It should be 0x22 instead of 0x2)
first link ID=0 is < 16 for fileinode=18
Error getting first link ID for inode = 18 (result=2)
Invalid first link in hard link chain (id = 18)
(It should be 19 instead of 0)
Indirect node 18 needs link count adjustment
(It should be 1 instead of 2)
** Rechecking volume.
** Checking non-journaled HFS Plus Volume.
The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
** Checking multi-linked files.
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
** The volume untitled was repaired successfully.
The generic/480 test executes such steps on final phase:
"Now remove of the links of our file and create
a new file with the same name and in the same
parent directory, and finally fsync this new file."
unlink $SCRATCH_MNT/testdir/bar
touch $SCRATCH_MNT/testdir/bar
$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/testdir/bar
"Simulate a power failure and mount the filesystem
to check that replaying the fsync log/journal
succeeds, that is the mount operation does not fail."
_flakey_drop_and_remount
The key issue in HFS+ logic is that hfsplus_link(),
hfsplus_unlink(), hfsplus_rmdir(), hfsplus_symlink(),
and hfsplus_mknod() methods don't call
hfsplus_cat_write_inode() for the case of modified
inode objects. As a result, even if hfsplus_file_fsync()
is trying to flush the dirty Catalog File, but because of
not calling hfsplus_cat_write_inode() not all modified
inodes save the new state into Catalog File's records.
Finally, simulation of power failure results in inconsistent
state of Catalog File and FSCK tool reports about
volume corruption.
This patch adds calling of hfsplus_cat_write_inode()
method for modified inodes in hfsplus_link(),
hfsplus_unlink(), hfsplus_rmdir(), hfsplus_symlink(),
and hfsplus_mknod() methods. Also, it adds debug output
in several methods.
sudo ./check generic/480
FSTYP -- hfsplus
PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc1+ #18 SMP PREEMPT_DYNAMIC Thu Dec 4 12:24:45 PST 2025
MKFS_OPTIONS -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
generic/480 16s ... 16s
Ran: generic/480
Passed all 1 tests
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Link: https://lore.kernel.org/r/20251205000054.3670326-1-slava@dubeyko.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Here is my complete analysis:
---
## Commit Analysis: "hfsplus: fix volume corruption issue for
generic/480"
### 1. COMMIT MESSAGE ANALYSIS
The subject line explicitly says "fix volume corruption" - this is a
high-severity filesystem integrity bug. The commit message is
exceptionally detailed:
- Provides full xfstests output showing the corruption
- Includes `fsck.hfsplus` output documenting the specific corruption
found: "Incorrect number of file hard links", "invalid VHB
nextCatalogID", "Incorrect flags for file hard link"
- Describes the exact test scenario: unlink a hard link, create new file
with same name, fsync, simulate power failure
- Explains the root cause clearly: directory operations modify inodes
in-memory but don't persist changes to the HFS+ Catalog File B-tree
- Shows "after" test results proving the fix works
- Author is Viacheslav Dubeyko, the HFS+ subsystem maintainer
### 2. CODE CHANGE ANALYSIS
**The root cause**: HFS+ has a two-level write system:
1. VFS `mark_inode_dirty()` -> deferred writeback ->
`hfsplus_write_inode()` -> `hfsplus_cat_write_inode()` (asynchronous)
2. Direct `hfsplus_cat_write_inode()` call (synchronous, writes
immediately to catalog B-tree)
Five directory operations — `hfsplus_link()`, `hfsplus_unlink()`,
`hfsplus_rmdir()`, `hfsplus_symlink()`, `hfsplus_mknod()` — modify
multiple inodes (directory entries, link counts, timestamps) but only
called `mark_inode_dirty()` without directly writing to the Catalog
File. If a power failure occurs between the directory operation and the
deferred writeback, the on-disk catalog tree becomes inconsistent.
**The fix adds explicit `hfsplus_cat_write_inode()` calls**:
- `hfsplus_link()`: Writes `src_dir`, `dst_dir`, `sbi->hidden_dir`, and
`inode` to catalog
- `hfsplus_unlink()`: Writes `dir`, `sbi->hidden_dir`, and `inode` to
catalog (only on success)
- `hfsplus_rmdir()`: Writes `dir` to catalog
- `hfsplus_symlink()`: Writes `dir` and `inode` to catalog
- `hfsplus_mknod()`: Writes `dir` and `inode` to catalog
**This pattern already existed in `hfsplus_rename()`** (added by commit
24e17a29cf753 which fixed the same class of bug for generic/073):
```532:563:fs/hfsplus/dir.c
static int hfsplus_rename(struct mnt_idmap *idmap,
struct inode *old_dir, struct dentry
*old_dentry,
struct inode *new_dir, struct dentry
*new_dentry,
unsigned int flags)
{
// ...
if (!res) {
new_dentry->d_fsdata = old_dentry->d_fsdata;
res = hfsplus_cat_write_inode(old_dir);
if (!res)
res = hfsplus_cat_write_inode(new_dir);
}
return res;
}
```
The current commit extends this same proven pattern to the five other
directory operations that were missing it.
**Additional changes**: The commit adds `hfs_dbg()` debug output in
multiple functions. The `hfs_dbg` macro maps to `pr_debug()` (compiled
out by default), so this is harmless at runtime. One minor whitespace
alignment change in `hfsplus_link()`.
### 3. CLASSIFICATION
- **Bug type**: Filesystem data corruption / inconsistency on power
failure
- **Category**: Missing write-through of inode metadata to on-disk
B-tree
- **Severity**: HIGH - filesystem corruption detected by fsck, incorrect
hard link counts, flags, catalog IDs
- **Reproducibility**: Reliably reproducible with xfstests generic/480
### 4. SCOPE AND RISK ASSESSMENT
- **Files changed**: 2 (fs/hfsplus/dir.c, fs/hfsplus/inode.c)
- **Functional change**: ~33 lines of `hfsplus_cat_write_inode()` calls
+ error handling
- **Debug noise**: ~10 lines of `hfs_dbg()` additions
- **Risk**: LOW - `hfsplus_cat_write_inode()` is already called from the
VFS writeback path (`hfsplus_write_inode()`) and from
`hfsplus_rename()`, so it's thoroughly tested. The new calls add it at
the same logical points in other directory operations.
- **Subsystem**: HFS+ filesystem - relatively narrow impact (only
affects HFS+ users)
### 5. USER IMPACT
HFS+ is used for:
- Mac OS X / macOS compatibility
- External storage devices formatted as HFS+
- Dual-boot Linux/Mac systems
Without this fix, operations involving hard links, file deletion,
symlinks, or device node creation can lead to volume corruption after
power failure. The corruption manifests as incorrect link counts, wrong
catalog IDs, and incorrect file flags - requiring fsck repair.
### 6. DEPENDENCY CHECK
**The core fix is self-contained.** The `hfsplus_cat_write_inode()`
function exists in all kernel versions with HFS+ support. The additions
to `hfsplus_link()`, `hfsplus_unlink()`, `hfsplus_rmdir()`,
`hfsplus_symlink()`, and `hfsplus_mknod()` are independent of the
earlier `hfsplus_rename()` fix (24e17a29cf753).
**Backport concern**: The `hfs_dbg()` macro signature changed in v6.18
(commit f32a26fab3672). Before 6.18, `hfs_dbg` required a flag argument
as the first parameter (e.g., `hfs_dbg(INODE, "...")`). The current
commit uses the new format (no flag argument). For stable trees before
6.18, the `hfs_dbg()` additions would need to be stripped from the
backport, but the core fix applies cleanly without them.
### 7. STABILITY INDICATORS
- Author is the HFS+ subsystem maintainer
- This is the third in a series of similar corruption fixes
(generic/073, generic/101, generic/480), demonstrating the author has
deep understanding of the issue class
- Verified with xfstests before and after
- The pattern used (calling `hfsplus_cat_write_inode()` from directory
operations) is already proven in `hfsplus_rename()`
### CONCLUSION
This commit fixes a real, reproducible filesystem corruption bug in HFS+
that manifests during power failure scenarios. The fix is conceptually
simple - ensuring that all modified inodes are persisted to the catalog
B-tree during directory operations, matching the pattern already
established in `hfsplus_rename()`. The risk is low because the called
function is well-tested and the change pattern is proven. The debug
output additions are harmless noise that may need stripping for older
stable trees but don't affect the core fix.
Filesystem corruption is one of the highest-priority categories for
stable backporting, as it directly impacts data integrity.
**YES**
fs/hfsplus/dir.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
fs/hfsplus/inode.c | 5 +++++
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c
index cadf0b5f93422..ca5f74a140ec1 100644
--- a/fs/hfsplus/dir.c
+++ b/fs/hfsplus/dir.c
@@ -313,6 +313,9 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
if (!S_ISREG(inode->i_mode))
return -EPERM;
+ hfs_dbg("src_dir->i_ino %lu, dst_dir->i_ino %lu, inode->i_ino %lu\n",
+ src_dir->i_ino, dst_dir->i_ino, inode->i_ino);
+
mutex_lock(&sbi->vh_mutex);
if (inode->i_ino == (u32)(unsigned long)src_dentry->d_fsdata) {
for (;;) {
@@ -332,7 +335,7 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
cnid = sbi->next_cnid++;
src_dentry->d_fsdata = (void *)(unsigned long)cnid;
res = hfsplus_create_cat(cnid, src_dir,
- &src_dentry->d_name, inode);
+ &src_dentry->d_name, inode);
if (res)
/* panic? */
goto out;
@@ -350,6 +353,21 @@ static int hfsplus_link(struct dentry *src_dentry, struct inode *dst_dir,
mark_inode_dirty(inode);
sbi->file_count++;
hfsplus_mark_mdb_dirty(dst_dir->i_sb);
+
+ res = hfsplus_cat_write_inode(src_dir);
+ if (res)
+ goto out;
+
+ res = hfsplus_cat_write_inode(dst_dir);
+ if (res)
+ goto out;
+
+ res = hfsplus_cat_write_inode(sbi->hidden_dir);
+ if (res)
+ goto out;
+
+ res = hfsplus_cat_write_inode(inode);
+
out:
mutex_unlock(&sbi->vh_mutex);
return res;
@@ -367,6 +385,9 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
if (HFSPLUS_IS_RSRC(inode))
return -EPERM;
+ hfs_dbg("dir->i_ino %lu, inode->i_ino %lu\n",
+ dir->i_ino, inode->i_ino);
+
mutex_lock(&sbi->vh_mutex);
cnid = (u32)(unsigned long)dentry->d_fsdata;
if (inode->i_ino == cnid &&
@@ -408,6 +429,15 @@ static int hfsplus_unlink(struct inode *dir, struct dentry *dentry)
inode_set_ctime_current(inode);
mark_inode_dirty(inode);
out:
+ if (!res) {
+ res = hfsplus_cat_write_inode(dir);
+ if (!res) {
+ res = hfsplus_cat_write_inode(sbi->hidden_dir);
+ if (!res)
+ res = hfsplus_cat_write_inode(inode);
+ }
+ }
+
mutex_unlock(&sbi->vh_mutex);
return res;
}
@@ -429,6 +459,8 @@ static int hfsplus_rmdir(struct inode *dir, struct dentry *dentry)
inode_set_ctime_current(inode);
hfsplus_delete_inode(inode);
mark_inode_dirty(inode);
+
+ res = hfsplus_cat_write_inode(dir);
out:
mutex_unlock(&sbi->vh_mutex);
return res;
@@ -465,6 +497,12 @@ static int hfsplus_symlink(struct mnt_idmap *idmap, struct inode *dir,
hfsplus_instantiate(dentry, inode, inode->i_ino);
mark_inode_dirty(inode);
+
+ res = hfsplus_cat_write_inode(dir);
+ if (res)
+ goto out;
+
+ res = hfsplus_cat_write_inode(inode);
goto out;
out_err:
@@ -506,6 +544,12 @@ static int hfsplus_mknod(struct mnt_idmap *idmap, struct inode *dir,
hfsplus_instantiate(dentry, inode, inode->i_ino);
mark_inode_dirty(inode);
+
+ res = hfsplus_cat_write_inode(dir);
+ if (res)
+ goto out;
+
+ res = hfsplus_cat_write_inode(inode);
goto out;
failed_mknod:
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 7ae6745ca7ae1..c762bf909d1aa 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -328,6 +328,9 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end,
struct hfsplus_vh *vhdr = sbi->s_vhdr;
int error = 0, error2;
+ hfs_dbg("inode->i_ino %lu, start %llu, end %llu\n",
+ inode->i_ino, start, end);
+
error = file_write_and_wait_range(file, start, end);
if (error)
return error;
@@ -616,6 +619,8 @@ int hfsplus_cat_write_inode(struct inode *inode)
hfsplus_cat_entry entry;
int res = 0;
+ hfs_dbg("inode->i_ino %lu\n", inode->i_ino);
+
if (HFSPLUS_IS_RSRC(inode))
main_inode = HFSPLUS_I(inode)->rsrc_inode;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.18] kselftest/kublk: include message in _Static_assert for C11 compatibility
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (8 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] hfsplus: fix volume corruption issue for generic/480 Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] dlm: validate length in dlm_search_rsb_tree Sasha Levin
` (13 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Clint George, Ming Lei, Shuah Khan, Sasha Levin, nathan,
linux-block, llvm
From: Clint George <clintbgeorge@gmail.com>
[ Upstream commit 3e6ad272bb8b3199bad952e7b077102af2d8df03 ]
Add descriptive message in the _Static_assert to comply with the C11
standard requirement to prevent compiler from throwing out error. The
compiler throws an error when _Static_assert is used without a message as
that is a C23 extension.
[] Testing:
The diff between before and after of running the kselftest test of the
module shows no regression on system with x86 architecture
[] Error log:
~/Desktop/kernel-dev/linux-v1/tools/testing/selftests/ublk$ make LLVM=1 W=1
CC kublk
In file included from kublk.c:6:
./kublk.h:220:43: error: '_Static_assert' with no message is a C23 extension [-Werror,-Wc23-extensions]
220 | _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
| ^
| , ""
1 error generated.
In file included from null.c:3:
./kublk.h:220:43: error: '_Static_assert' with no message is a C23 extension [-Werror,-Wc23-extensions]
220 | _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
| ^
| , ""
1 error generated.
In file included from file_backed.c:3:
./kublk.h:220:43: error: '_Static_assert' with no message is a C23 extension [-Werror,-Wc23-extensions]
220 | _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
| ^
| , ""
1 error generated.
In file included from common.c:3:
./kublk.h:220:43: error: '_Static_assert' with no message is a C23 extension [-Werror,-Wc23-extensions]
220 | _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
| ^
| , ""
1 error generated.
In file included from stripe.c:3:
./kublk.h:220:43: error: '_Static_assert' with no message is a C23 extension [-Werror,-Wc23-extensions]
220 | _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
| ^
| , ""
1 error generated.
In file included from fault_inject.c:11:
./kublk.h:220:43: error: '_Static_assert' with no message is a C23 extension [-Werror,-Wc23-extensions]
220 | _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
| ^
| , ""
1 error generated.
make: *** [../lib.mk:225: ~/Desktop/kernel-dev/linux-v1/tools/testing/selftests/ublk/kublk] Error 1
Link: https://lore.kernel.org/r/20251215085022.7642-1-clintbgeorge@gmail.com
Signed-off-by: Clint George <clintbgeorge@gmail.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
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.
---
## Detailed Analysis
### 1. Commit Message Analysis
The commit fixes a C11/C23 compatibility issue in
`tools/testing/selftests/ublk/kublk.h`. The `_Static_assert()` macro is
used without a message string, which is only valid in C23. When
compiling with Clang/LLVM, this triggers the `-Wc23-extensions` warning,
which is promoted to a hard error by `-Werror`.
The commit message includes the full error log showing compilation
failure across 6 source files that include `kublk.h`. The fix is
reviewed by Ming Lei (ublk maintainer) and signed off by Shuah Khan
(kselftest maintainer).
### 2. Code Change Analysis
The change is a single-line modification:
```diff
- _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
+ _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7,
"UBLK_MAX_QUEUES_SHIFT must be <= 7");
```
This is the most trivial kind of fix possible - adding a string literal
parameter to comply with C11 standard syntax. The runtime behavior is
completely unchanged since `_Static_assert` is a compile-time construct
that generates zero code.
### 3. Impact Assessment
**The build failure is worse than the error log suggests.** The ublk
selftest Makefile includes `-Werror` **by default** (`ifneq
($(WERROR),0)` means it's always on unless explicitly disabled). This
means **any user building the ublk selftest with Clang/LLVM will hit a
hard build error** - they don't need `W=1` to trigger it; that just adds
additional warning flags.
### 4. Affected Stable Trees
The buggy `_Static_assert` line was introduced by commit `bf098d72696db`
("selftests: ublk: kublk: plumb q_id in io_uring user_data") which first
appeared in the 6.16 cycle. The unfixed line exists in:
- **6.16.y** (line 252)
- **6.17.y** (line 223)
- **6.18.y** (line 223)
- **6.19.y** (line 226)
It does NOT exist in 6.15.y or earlier (6.12.y LTS, 6.6.y LTS, etc.).
### 5. Stable Criteria Evaluation
| Criterion | Assessment |
|-----------|-----------|
| Obviously correct and tested | YES - trivially obvious, tested by
author |
| Fixes a real bug | YES - hard compilation error |
| Important issue | YES - **build fix** (explicitly listed exception
category) |
| Small and contained | YES - one line, one file |
| No new features | YES - purely a build fix |
| Can apply cleanly | Likely YES with minor context adjustment per tree
|
### 6. Risk Assessment
**Risk: Essentially zero.** `_Static_assert` is a compile-time
construct. Adding a message string parameter cannot change runtime
behavior. The assertion condition itself is unchanged. This is the
safest possible type of change.
### 7. Dependency Check
No dependencies. The fix is entirely self-contained - it only modifies
the syntax of an existing assertion. No new includes, no new functions,
no changed behavior.
### 8. Classification
This is a **build fix** for a kselftest. Build fixes are explicitly
listed as an allowed exception category for stable ("These are critical
for users who need to build the kernel"). While this specifically
affects selftest infrastructure rather than the kernel itself, selftests
are integral to verifying stable kernel correctness, and CI systems
(KernelCI, 0-day) regularly run these tests on stable trees.
The fix prevents a **hard build failure** with the Clang/LLVM toolchain
(widely used, especially in Android and ChromeOS kernel development).
The `-Werror` flag is enabled by default in the ublk selftest Makefile,
making this a default-configuration failure, not an edge case.
### Conclusion
This is a trivial, zero-risk build fix that prevents a default-
configuration compilation error in the ublk kselftest when using
Clang/LLVM. It meets all stable kernel criteria: obviously correct,
fixes a real build failure, is tiny and self-contained, and introduces
no new features. The only limitation is that it only applies to stable
trees 6.16.y and later where the buggy `_Static_assert` line exists.
**YES**
tools/testing/selftests/ublk/kublk.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ublk/kublk.h b/tools/testing/selftests/ublk/kublk.h
index 8a83b90ec603a..cae2e30f0cdd5 100644
--- a/tools/testing/selftests/ublk/kublk.h
+++ b/tools/testing/selftests/ublk/kublk.h
@@ -223,7 +223,7 @@ static inline __u64 build_user_data(unsigned tag, unsigned op,
unsigned tgt_data, unsigned q_id, unsigned is_target_io)
{
/* we only have 7 bits to encode q_id */
- _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7);
+ _Static_assert(UBLK_MAX_QUEUES_SHIFT <= 7, "UBLK_MAX_QUEUES_SHIFT must be <= 7");
assert(!(tag >> 16) && !(op >> 8) && !(tgt_data >> 16) && !(q_id >> 7));
return tag | (op << 16) | (tgt_data << 24) |
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.12] dlm: validate length in dlm_search_rsb_tree
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (9 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] kselftest/kublk: include message in _Static_assert for C11 compatibility Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0 Sasha Levin
` (12 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Ezrak1e, Alexander Aring, David Teigland, Sasha Levin, gfs2
From: Ezrak1e <ezrakiez@gmail.com>
[ Upstream commit 080e5563f878c64e697b89e7439d730d0daad882 ]
The len parameter in dlm_dump_rsb_name() is not validated and comes
from network messages. When it exceeds DLM_RESNAME_MAXLEN, it can
cause out-of-bounds write in dlm_search_rsb_tree().
Add length validation to prevent potential buffer overflow.
Signed-off-by: Ezrak1e <ezrakiez@gmail.com>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
Signed-off-by: David Teigland <teigland@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Indeed, `dlm_copy_master_names` calls `find_rsb_root` with `inlen` that
comes from network data (`receive_rcom_names` →
`dlm_copy_master_names`). `inlen` is derived from
`rc_in->rc_header.h_length - sizeof(struct dlm_rcom)` with no bounds
check. And `find_rsb_root` doesn't validate it before passing to
`dlm_search_rsb_tree`.
So there are **at least two** unvalidated network-
to-`dlm_search_rsb_tree` paths:
1. `receive_rcom_lookup` → `dlm_dump_rsb_name` → `dlm_search_rsb_tree`
(debug dump path)
2. `receive_rcom_names` → `dlm_copy_master_names` → `find_rsb_root` →
`dlm_search_rsb_tree`
Both can pass `len > DLM_RESNAME_MAXLEN` (64), causing `memcpy(key,
name, len)` to overflow the stack buffer `key[DLM_RESNAME_MAXLEN]`.
Now let me verify what `DLM_RESNAME_MAXLEN` is:
The value is 64 (from `include/uapi/linux/dlmconstants.h`). The stack
buffer `key` is 64 bytes. If `len` > 64, `memcpy(key, name, len)` writes
beyond the buffer — a classic **stack buffer overflow**.
This is exploitable from a malicious DLM cluster node sending a crafted
network message.
## Detailed Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message clearly states:
- The `len` parameter comes from **network messages** (untrusted input)
- When `len` exceeds `DLM_RESNAME_MAXLEN` (64), it causes an **out-of-
bounds write** in `dlm_search_rsb_tree()`
- The fix adds length validation to prevent the buffer overflow
### 2. CODE CHANGE ANALYSIS
The change is extremely small and surgical — just 2 lines added:
```625:636:fs/dlm/lock.c
int dlm_search_rsb_tree(struct rhashtable *rhash, const void *name, int
len,
struct dlm_rsb **r_ret)
{
char key[DLM_RESNAME_MAXLEN] = {};
// NEW: if (len > DLM_RESNAME_MAXLEN) return -EINVAL;
memcpy(key, name, len);
// ...
}
```
Without the check, `memcpy(key, name, len)` with `len > 64` writes past
the end of the 64-byte stack buffer `key`. This is a **stack buffer
overflow** — one of the most dangerous vulnerability classes in C.
### 3. VULNERABLE CODE PATHS
I traced all 6 callers of `dlm_search_rsb_tree`:
| Caller | Validates `len`? | Network-reachable? |
|--------|------------------|--------------------|
| `find_rsb_dir` | YES (via `find_rsb` at line 1089) | Yes |
| `find_rsb_nodir` | YES (via `find_rsb` at line 1089) | Yes |
| `_dlm_master_lookup` | YES (line 1268) | Yes |
| `receive_remove` | YES (line 4300) | Yes |
| **`dlm_dump_rsb_name`** | **NO** | **Yes** (via `receive_rcom_lookup`)
|
| **`find_rsb_root`** | **NO** | **Yes** (via `receive_rcom_names` →
`dlm_copy_master_names`) |
Two callers do NOT validate `len` before calling `dlm_search_rsb_tree`,
and both are reachable from network messages:
1. **`receive_rcom_lookup`** (rcom.c:379): When `rc_in->rc_id ==
0xFFFFFFFF`, it calls `dlm_dump_rsb_name(ls, rc_in->rc_buf, len)`
where `len` is derived from `rc_in->rc_header.h_length` (a network-
supplied 16-bit field). No bounds check.
2. **`receive_rcom_names`** (rcom.c:336): Calculates `inlen` from
`rc_in->rc_header.h_length - sizeof(struct dlm_rcom)` and passes it
directly to `dlm_copy_master_names` → `find_rsb_root` →
`dlm_search_rsb_tree`. No bounds check.
### 4. BUG SEVERITY
This is a **stack buffer overflow triggered by network input** in the
DLM (Distributed Lock Manager) subsystem:
- **Type**: Out-of-bounds write (stack buffer overflow)
- **Attack vector**: Network (DLM cluster communication protocol)
- **Trigger**: A malicious or buggy DLM cluster node sending a message
with `h_length` large enough to make the extracted `len` exceed 64
- **Impact**: Stack corruption, potential code execution, kernel
crash/panic
- **DLM context**: DLM is used in cluster filesystems like GFS2 and
OCFS2, which are used in production enterprise environments
While the attack surface requires being part of a DLM cluster (not
publicly internet-reachable in most deployments), this is still a
serious security bug. In shared hosting or cloud environments, cluster
node compromise could lead to kernel-level exploitation of other cluster
members.
### 5. FIX QUALITY
The fix is **defense-in-depth at the right layer**:
- Rather than fixing each individual caller, it adds the validation in
`dlm_search_rsb_tree` itself
- This protects against any future caller that might forget to validate
- The fix returns `-EINVAL`, which all callers handle (they all check
the return value)
- It's only 2 lines, extremely low risk of regression
- The pattern is identical to the existing checks in
`_dlm_master_lookup` (line 1268) and `receive_remove` (line 4300)
### 6. SCOPE AND RISK
- **Lines changed**: 2 (added `if` check and return)
- **Files changed**: 1 (`fs/dlm/lock.c`)
- **Risk of regression**: Extremely low — it adds a bounds check that
was already present in 4 of 6 callers
- **Subsystem**: DLM (mature, production-critical for cluster
filesystems)
### 7. APPLICABILITY TO STABLE
- The function `dlm_search_rsb_tree` was introduced in commit
`5be323b0c64db` ("dlm: move dlm_search_rsb_tree() out of lock") from
August 2024, which was merged in 6.12
- Older kernels may have equivalent vulnerable code but with different
structure
- For 6.12+ stable trees, the fix should apply cleanly
### 8. STABLE CRITERIA CHECK
- Obviously correct and tested: YES (trivial bounds check, accepted by
DLM maintainers Alexander Aring and David Teigland)
- Fixes a real bug: YES (stack buffer overflow from network input)
- Fixes an important issue: YES (security vulnerability — buffer
overflow exploitable from network)
- Small and contained: YES (2 lines in 1 file)
- No new features or APIs: YES (pure bug fix)
**YES**
fs/dlm/lock.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index c01a291db401b..a393ecaf3442a 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -626,7 +626,8 @@ int dlm_search_rsb_tree(struct rhashtable *rhash, const void *name, int len,
struct dlm_rsb **r_ret)
{
char key[DLM_RESNAME_MAXLEN] = {};
-
+ if (len > DLM_RESNAME_MAXLEN)
+ return -EINVAL;
memcpy(key, name, len);
*r_ret = rhashtable_lookup_fast(rhash, &key, dlm_rhash_rsb_params);
if (*r_ret)
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.18] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (10 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] dlm: validate length in dlm_search_rsb_tree Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.1] fs/buffer: add alert in try_to_free_buffers() for folios without buffers Sasha Levin
` (11 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Adrian Hunter, Frank Li, Alexandre Belloni, Sasha Levin,
jarkko.nikula
From: Adrian Hunter <adrian.hunter@intel.com>
[ Upstream commit 0818e4aa8fdeeed5973e0a8faeddc9da599fc897 ]
Extended Capability ID value 0 is special. It signifies the end of the
list. Stop reading Extended Capabilities if capability ID is 0.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260106164416.67074-3-adrian.hunter@intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me summarize the analysis.
---
## Detailed Analysis
### 1. COMMIT MESSAGE ANALYSIS
The subject is: "i3c: mipi-i3c-hci: Stop reading Extended Capabilities
if capability ID is 0"
The message clearly explains: **Extended Capability ID value 0 is
special. It signifies the end of the list.** This is consistent with how
hardware capability lists work in general (PCI uses the same convention,
as confirmed by the PCI extended capability parsing code at
`drivers/pci/pci.h:170` which checks `if (__header == 0) break;`).
The commit is authored by **Adrian Hunter** (Intel), a prolific and
highly respected kernel developer, and was reviewed by **Frank Li**
(NXP), another I3C subsystem contributor. It was accepted by the I3C
subsystem maintainer **Alexandre Belloni**.
### 2. CODE CHANGE ANALYSIS
The change is a single-line modification:
```275:276:drivers/i3c/master/mipi-i3c-hci/ext_caps.c
if (!cap_length)
break;
```
becomes:
```c
if (!cap_id || !cap_length)
break;
```
**What was wrong:** The `i3c_hci_parse_ext_caps()` function walks a
linked list of Extended Capabilities stored in MMIO register space. Per
the MIPI I3C HCI specification, cap_id == 0 is the end-of-list sentinel.
However, the code only checked for `cap_length == 0` to terminate the
walk. If the end-of-list entry has `cap_id == 0` but `cap_length != 0`,
the loop continues reading past the end of the valid capability list.
**What happens without the fix (when cap_id==0, cap_length!=0):**
1. The code does NOT break out of the loop (only `cap_length == 0`
triggers a break)
2. The `cap_id == 0` won't match any vendor-specific range (0xc0-0xcf)
3. The `cap_id == 0` won't match any entry in the `ext_capabilities[]`
table (lowest valid ID is 0x01)
4. The code prints `dev_notice("unknown ext_cap 0x00\n")` and continues
5. It advances `curr_cap` by `cap_length * 4` bytes, now pointing into
undefined MMIO space
6. Subsequent `readl()` calls read garbage data from MMIO registers past
the capability list
7. If garbage data happens to match a valid capability ID (like
0x01-0x10, 0x9d-0x9e), the corresponding **parser function will be
called with a bogus MMIO base address**, reading/writing the wrong
hardware registers
8. If garbage data produces a `cap_length` that fails the size check,
`i3c_hci_parse_ext_caps()` returns `-EINVAL`, causing
`i3c_hci_init()` to fail entirely and the **I3C controller to not
initialize at all**
### 3. CLASSIFICATION
This is a **real bug fix**. The bug is:
- **Spec violation**: cap_id==0 is end-of-list per the MIPI I3C HCI
specification
- **Potential driver initialization failure**: Garbage data past the
list can cause `-EINVAL` return, preventing the I3C controller from
working
- **Potential incorrect hardware register access**: If garbage data
matches a valid capability ID, parser functions will `readl()` and
potentially `writel()` at wrong MMIO offsets, which could cause
**hardware misbehavior or corruption**
### 4. SCOPE AND RISK ASSESSMENT
- **Lines changed:** 1 (adding `!cap_id ||` to an existing condition)
- **Files touched:** 1 (`ext_caps.c`)
- **Complexity:** Trivial - simply adds a termination check for a known
end-of-list sentinel
- **Risk of regression:** Essentially zero. If `cap_id == 0`, the spec
says it's end-of-list, so stopping is always the correct behavior. No
valid capability has ID 0.
### 5. USER IMPACT
The MIPI I3C HCI driver is used on real hardware - primarily Intel
platforms (as evidenced by Adrian Hunter's active Intel-specific I3C HCI
PCI patches, including adding support for Intel Nova Lake-S). Users with
I3C controllers that use capability ID 0 as the end-of-list marker (as
the spec prescribes) would hit this bug. The consequence ranges from
spurious log messages to complete failure of the I3C controller
initialization.
### 6. STABILITY & BACKPORT ASSESSMENT
- **Applies cleanly:** The vulnerable code is identical in all stable
trees (5.15, 6.1, 6.6, 6.12). The only change since introduction was a
`DBG()` to `dev_dbg()` conversion, which doesn't affect the
surrounding context enough to prevent clean application.
- **Obviously correct:** Adding `!cap_id` as a termination condition is
obviously correct per spec.
- **Self-contained:** No dependencies on other commits. The patch
message ID (`67074-3`) indicates it was patch 3 in a series, but the
fix is completely standalone.
- **Small and surgical:** One condition added to one line.
- **Tested:** Already reviewed, accepted, and merged into mainline.
### 7. COMPARISON WITH PCI
The PCI extended capability parser uses the exact same pattern: `if
(__header == 0) break;` (in `PCI_FIND_NEXT_EXT_CAP` macro at
`drivers/pci/pci.h:170`). The I3C HCI code was simply missing this
standard end-of-list check.
### Conclusion
This is a minimal, obviously correct, zero-risk fix for a real hardware-
facing bug that has existed since the driver was introduced in v5.11.
Without it, the I3C HCI driver may fail to initialize or access
incorrect hardware registers when walking past the end of the Extended
Capability list. The fix adds a single condition check (`!cap_id`) that
aligns with the MIPI I3C HCI specification and follows the same pattern
used by PCI capability parsing. It applies cleanly to all stable trees.
**YES**
drivers/i3c/master/mipi-i3c-hci/ext_caps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
index 7714f00ea9cc0..533a495e14c86 100644
--- a/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
+++ b/drivers/i3c/master/mipi-i3c-hci/ext_caps.c
@@ -272,7 +272,7 @@ int i3c_hci_parse_ext_caps(struct i3c_hci *hci)
cap_length = FIELD_GET(CAP_HEADER_LENGTH, cap_header);
dev_dbg(&hci->master.dev, "id=0x%02x length=%d",
cap_id, cap_length);
- if (!cap_length)
+ if (!cap_id || !cap_length)
break;
if (curr_cap + cap_length * 4 >= end) {
dev_err(&hci->master.dev,
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.1] fs/buffer: add alert in try_to_free_buffers() for folios without buffers
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (11 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0 Sasha Levin
@ 2026-02-10 23:30 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.15] i3c: master: svc: Initialize 'dev' to NULL in svc_i3c_master_ibi_isr() Sasha Levin
` (10 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:30 UTC (permalink / raw)
To: patches, stable
Cc: Deepakkumar Karn, Jan Kara, Christian Brauner, Sasha Levin, viro,
linux-fsdevel
From: Deepakkumar Karn <dkarn@redhat.com>
[ Upstream commit b68f91ef3b3fe82ad78c417de71b675699a8467c ]
try_to_free_buffers() can be called on folios with no buffers attached
when filemap_release_folio() is invoked on a folio belonging to a mapping
with AS_RELEASE_ALWAYS set but no release_folio operation defined.
In such cases, folio_needs_release() returns true because of the
AS_RELEASE_ALWAYS flag, but the folio has no private buffer data. This
causes try_to_free_buffers() to call drop_buffers() on a folio with no
buffers, leading to a null pointer dereference.
Adding a check in try_to_free_buffers() to return early if the folio has no
buffers attached, with WARN_ON_ONCE() to alert about the misconfiguration.
This provides defensive hardening.
Signed-off-by: Deepakkumar Karn <dkarn@redhat.com>
Link: https://patch.msgid.link/20251211131211.308021-1-dkarn@redhat.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
`folio_buffers()` exists in all relevant stable trees. The patch uses
only `WARN_ON_ONCE` and `folio_buffers`, both of which are available
everywhere.
## Comprehensive Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit describes a clear bug: `try_to_free_buffers()` can be called
on folios with no buffer_heads attached when `filemap_release_folio()`
is invoked on a folio belonging to a mapping with `AS_RELEASE_ALWAYS`
set but no `release_folio` operation defined. This causes a **NULL
pointer dereference** in `drop_buffers()`.
The commit is authored by Deepakkumar Karn (Red Hat), reviewed by Jan
Kara (SUSE, a very well-respected VFS/filesystem developer), and signed
off by Christian Brauner (the VFS subsystem maintainer).
### 2. CODE CHANGE ANALYSIS
The fix adds exactly 3 lines (plus a comment) to
`try_to_free_buffers()`:
```c
/* Misconfigured folio check */
if (WARN_ON_ONCE(!folio_buffers(folio)))
return true;
```
This is inserted after the writeback check and before the `mapping ==
NULL` check. The logic is:
- If the folio has no buffers (`folio_buffers()` returns NULL), emit a
warning (once) and return `true` (success — there's nothing to free)
- This prevents the NULL deref in `drop_buffers()` where `head =
folio_buffers(folio)` returns NULL and then `buffer_busy(bh)`
dereferences it
### 3. THE BUG MECHANISM
The call chain is:
1. Memory reclaim (`shrink_folio_list`) or truncation calls
`filemap_release_folio()`
2. `folio_needs_release()` returns `true` because `AS_RELEASE_ALWAYS` is
set
3. `mapping->a_ops->release_folio` is NULL (e.g., `afs_dir_aops` doesn't
define it in v6.14+)
4. Falls through to `try_to_free_buffers(folio)`
5. `drop_buffers()` dereferences `folio_buffers(folio)` which is NULL
6. **NULL pointer dereference crash**
### 4. AFFECTED VERSIONS
The specific trigger (AFS symlinks/directories with `afs_dir_aops`
lacking `release_folio`) only exists from **v6.14-rc1** onward,
introduced by commit `6dd80936618c` ("afs: Use netfslib for
directories"). Before v6.14, all filesystems that set
`AS_RELEASE_ALWAYS` also define `release_folio`.
This means:
- **v6.6.y**: NOT affected (afs_dir_aops has release_folio)
- **v6.12.y**: NOT affected (afs_dir_aops has release_folio)
- **v6.13.y**: NOT affected (afs_dir_aops has release_folio)
- **v6.14.y**: AFFECTED (afs_dir_aops stripped down)
- **v6.15.y+**: AFFECTED
### 5. RISK ASSESSMENT
**Risk: Very Low**
- The patch adds 3 lines - a NULL check with WARN_ON_ONCE and early
return
- It's completely self-contained, no dependencies
- `folio_buffers()` and `WARN_ON_ONCE` are available in all stable trees
- Returning `true` is correct: no buffers = nothing to free = success
- Reviewed by Jan Kara and Christian Brauner (top VFS experts)
- Could be considered pure defensive hardening in older kernels where
the trigger doesn't exist
**Benefit in affected trees (6.14+): High**
- Prevents a NULL pointer dereference crash reachable from normal memory
reclaim
- AFS symlinks with fscache enabled hitting memory pressure would crash
**Benefit in unaffected trees (6.6, 6.12): Low**
- Defensive hardening only - the specific trigger doesn't exist
- Protects against potential future similar misconfigurations if other
patches are backported
### 6. STABLE BACKPORT SUITABILITY
For trees where the bug is actually triggerable (6.14+), this is clearly
a YES - it fixes a NULL pointer dereference crash in a core kernel path
(memory reclaim). However, for older stable trees (6.6, 6.12), the bug
isn't reachable with the current code because all filesystems that set
`AS_RELEASE_ALWAYS` also properly define `release_folio`.
The fix itself is trivially small (3 lines), obviously correct, has zero
regression risk, and is well-reviewed. Even as defensive hardening, it's
the kind of patch that makes stable kernels more robust.
For the broader question of "should this be backported to stable trees":
The answer depends on which tree. For 6.14.y and later, it's an
unambiguous yes - it fixes a real crash. For 6.6.y and 6.12.y, it's
defensive hardening that prevents a crash scenario that currently isn't
reachable but could become reachable if other patches are backported.
Given the negligible risk and the fact that it hardens a core kernel
function against NULL dereference, even the older trees benefit.
**YES**
fs/buffer.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/buffer.c b/fs/buffer.c
index 838c0c5710229..28e4d53f17173 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2948,6 +2948,10 @@ bool try_to_free_buffers(struct folio *folio)
if (folio_test_writeback(folio))
return false;
+ /* Misconfigured folio check */
+ if (WARN_ON_ONCE(!folio_buffers(folio)))
+ return true;
+
if (mapping == NULL) { /* can this still happen? */
ret = drop_buffers(folio, &buffers_to_free);
goto out;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-5.15] i3c: master: svc: Initialize 'dev' to NULL in svc_i3c_master_ibi_isr()
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (12 preceding siblings ...)
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.1] fs/buffer: add alert in try_to_free_buffers() for folios without buffers Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.12] statmount: permission check should return EPERM Sasha Levin
` (9 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Frank Li, kernel test robot, Alexandre Belloni, Sasha Levin,
miquel.raynal, linux-i3c, imx
From: Frank Li <Frank.Li@nxp.com>
[ Upstream commit 3c9ffb4db787428a5851d5865823ab23842d5103 ]
Initialize the 'dev' pointer to NULL in svc_i3c_master_ibi_isr() and add
a NULL check in the error path.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/r/202512131016.YCKIsDXM-lkp@intel.com/
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20251215200852.3079073-1-Frank.Li@nxp.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Here's my complete analysis:
## Detailed Analysis
### 1. Bug Mechanism
The function `svc_i3c_master_ibi_isr()` declares `struct i3c_dev_desc
*dev;` without initialization (line 537). The variable `dev` is **only
assigned** inside the `SVC_I3C_MSTATUS_IBITYPE_IBI` case of the switch
statement (line 600). For the other three cases — `HOT_JOIN`,
`MASTER_REQUEST`, and `default` — `dev` is **never assigned** and
remains an uninitialized stack pointer containing garbage.
After the switch, the error path at line 629-635 checks:
```c
if (svc_i3c_master_error(master)) {
if (master->ibi.tbq_slot) {
data = i3c_dev_get_master_data(dev); // BUG: dev could be
uninitialized
```
`i3c_dev_get_master_data()` directly dereferences
`dev->common.master_priv` (in `include/linux/i3c/master.h:624-627`). If
`dev` is uninitialized garbage, this dereference causes **undefined
behavior** — most likely a kernel crash/oops, or worse, access to random
memory (potential security issue).
### 2. Practical Reachability
In the **current code**, `master->ibi.tbq_slot` is only set non-NULL by
`svc_i3c_master_handle_ibi()` (line 476), which is only called in the
`IBI` case where `dev` IS assigned. So in normal flow, if `tbq_slot` is
non-NULL in this invocation, `dev` should be valid.
**However, there are scenarios where the bug can trigger:**
- If `tbq_slot` was left non-NULL from a **previous invocation** due to
a code path that failed to clean it up (a secondary bug). Then on the
next ISR entry with a different `ibitype` (e.g., HOT_JOIN), `dev`
would be uninitialized while `tbq_slot` is non-NULL.
- The code is **fragile** — any future change that sets `tbq_slot` in a
different path would immediately make this crash reachable.
Notably, the "non critical tasks" section at line 646 **already has**
`if (dev)` guard, indicating the author of that code recognized `dev`
might not be valid. The error path at line 630 lacked this same
protection — an inconsistency.
### 3. Reporter and Detection
Reported by `kernel test robot <lkp@intel.com>` — Intel's automated
kernel testing infrastructure that runs static analysis tools (Smatch,
sparse, compiler warnings). The `Closes:` link to an lkp report confirms
this is a compiler/static-analyzer warning about use of an uninitialized
variable. These reports identify real code defects.
### 4. Fix Assessment
The fix is minimal and obviously correct:
1. **Initialize `dev` to NULL** (line change: `struct i3c_dev_desc
*dev;` → `struct i3c_dev_desc *dev = NULL;`)
2. **Add NULL check in error path** (line change: `if
(master->ibi.tbq_slot)` → `if (master->ibi.tbq_slot && dev)`)
This makes the error path consistent with the "non critical tasks" path
that already checks `if (dev)` at line 646.
### 5. Scope and Risk
- **Lines changed:** 2 (trivially small)
- **Files touched:** 1 (`drivers/i3c/master/svc-i3c-master.c`)
- **Risk of regression:** Zero — this only ADDS safety checks
- **No behavioral change** in the happy path (when `dev` is valid)
- The fix is **defense-in-depth** — prevents a crash if the guard
condition `tbq_slot != NULL` is ever true when `dev` wasn't set
### 6. Stable Tree Applicability
The original driver was added in v5.12 (`dd3c52846d59`). The error path
with the uninitialized `dev` has existed since the driver's creation.
The exact code shape changed in:
- `8d29fa6d921ca` (6.17 cycle) — moved from workqueue to ISR context
- `a7869b0a2540f` (6.18 cycle) — manual IBI response
For older stable trees, the fix would need to target the
`svc_i3c_master_ibi_work()` function instead of
`svc_i3c_master_ibi_isr()`, but the same pattern applies.
### 7. Classification
This is an **uninitialized variable fix** — one of the recognized bug
categories for stable backports. It prevents:
- Potential **kernel crash** from dereferencing garbage pointer
- Potential **security vulnerability** (accessing uncontrolled memory)
- **Compiler warnings** that pollute build output
### Conclusion
This commit fixes a real uninitialized variable issue detected by static
analysis. The fix is trivially small (2 lines), obviously correct, zero
risk of regression, and prevents a potential kernel crash/oops if the
error path is reached with `dev` uninitialized. It aligns the error
path's safety checks with those already present in the non-critical task
handling path. It meets all stable kernel criteria: fixes a real bug, is
small and contained, obviously correct, and introduces no new features.
**YES**
drivers/i3c/master/svc-i3c-master.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index a62f22ff8b576..857504d36e186 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -533,8 +533,8 @@ static int svc_i3c_master_handle_ibi_won(struct svc_i3c_master *master, u32 msta
static void svc_i3c_master_ibi_isr(struct svc_i3c_master *master)
{
struct svc_i3c_i2c_dev_data *data;
+ struct i3c_dev_desc *dev = NULL;
unsigned int ibitype, ibiaddr;
- struct i3c_dev_desc *dev;
u32 status, val;
int ret;
@@ -627,7 +627,7 @@ static void svc_i3c_master_ibi_isr(struct svc_i3c_master *master)
* for the slave to interrupt again.
*/
if (svc_i3c_master_error(master)) {
- if (master->ibi.tbq_slot) {
+ if (master->ibi.tbq_slot && dev) {
data = i3c_dev_get_master_data(dev);
i3c_generic_ibi_recycle_slot(data->ibi_pool,
master->ibi.tbq_slot);
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.12] statmount: permission check should return EPERM
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (13 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.15] i3c: master: svc: Initialize 'dev' to NULL in svc_i3c_master_ibi_isr() Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] audit: add missing syscalls to read class Sasha Levin
` (8 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Bhavik Sachdev, Miklos Szeredi, Christian Brauner, Sasha Levin,
viro, linux-fsdevel
From: Bhavik Sachdev <b.sachdev1904@gmail.com>
[ Upstream commit fccbe38a5d06dbe44bcd89196fe1d2c2272a1f4a ]
Currently, statmount() returns ENOENT when caller is not CAP_SYS_ADMIN
in the user namespace owner of target mount namespace. This should be
EPERM instead.
Suggested-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Bhavik Sachdev <b.sachdev1904@gmail.com>
Link: https://patch.msgid.link/20251129091455.757724-2-b.sachdev1904@gmail.com
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Good - the selftests don't check for a specific errno on the permission
path, so the change won't break tests.
Now let me summarize the analysis.
---
## Detailed Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message is clear: `statmount()` returns the wrong errno code
(`ENOENT`) when a caller lacks `CAP_SYS_ADMIN` in the user namespace
owning the target mount namespace. The commit changes it to `EPERM`,
which is the standard POSIX errno for "operation not permitted."
The commit was suggested by Miklos Szeredi (a senior VFS maintainer),
authored by Bhavik Sachdev, and merged by Christian Brauner (the VFS
maintainer). This gives strong confidence in correctness.
### 2. CODE CHANGE ANALYSIS
This is a **one-line change**: `-ENOENT` to `-EPERM` on line 5783 of
`fs/namespace.c`.
The affected code path is in the `statmount()` syscall
(SYSCALL_DEFINE4):
```5781:5783:fs/namespace.c
if (kreq.mnt_ns_id && (ns != current->nsproxy->mnt_ns) &&
!ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
return -ENOENT;
```
The condition checks: Is the caller requesting a specific mount
namespace ID? If so, is it different from the caller's own mount
namespace? And does the caller lack `CAP_SYS_ADMIN` in that namespace's
user namespace? If all three conditions are true, this is a **permission
denial**, and `-ENOENT` ("No such file or directory") is semantically
incorrect. `-EPERM` ("Operation not permitted") is the correct error
code.
### 3. BUG MECHANISM
This is a **wrong errno** bug introduced by commit `71aacb4c8c3d` ("fs:
Allow statmount() in foreign mount namespace") in v6.11-rc1. The
original author used `-ENOENT` for both "namespace doesn't exist" and
"you don't have permission", but these are semantically different
conditions that userspace needs to distinguish.
**Internal inconsistency**: The inner `do_statmount()` function (line
5572-5574) already correctly returns `-EPERM` for a different permission
check (`!is_path_reachable(...) && !ns_capable_noaudit(...)`). The outer
syscall returning `-ENOENT` for the same type of permission check is
inconsistent.
**Impact on userspace**: A userspace program calling `statmount()` on a
foreign mount namespace without sufficient privileges receives `ENOENT`,
which it would naturally interpret as "the mount namespace doesn't
exist." This misleads debugging and prevents proper error handling. A
container runtime or monitoring tool, for example, would think the
namespace is gone rather than that it lacks the right credentials —
leading to potentially wrong recovery actions.
### 4. SCOPE AND RISK
- **Scope**: Single-line change, single file (`fs/namespace.c`)
- **Risk**: Extremely low. This only changes an error code on a failure
path. No logic is altered. No new code paths are created.
- **Potential concern**: If any userspace program checked `errno ==
ENOENT` specifically after `statmount()` to handle permission
failures, it would need to be updated. However, no such code exists in
kernel selftests, and relying on ENOENT for permission denial would be
poor practice.
### 5. STABLE TREE APPLICABILITY
The buggy code was introduced in **v6.11-rc1** and exists in stable
branches **6.11.y** through **6.19.y**. I confirmed the exact same code
(with `-ENOENT`) exists at the same location in both
`stable/linux-6.11.y` and `stable/linux-6.12.y`. The patch applies
trivially with no dependencies.
**Note**: `listmount()` (line 5943-5945) has the same bug (`-ENOENT`
instead of `-EPERM` for the same permission check). The commit being
analyzed only fixes `statmount()`, not `listmount()`. The Link URL
contains `757724-2` suggesting this was patch 2 of a series; patch 1
likely fixes `listmount()`. However, each fix is independent - fixing
`statmount()` alone is valuable even without the companion `listmount()`
fix.
### 6. CLASSIFICATION
This is a **bug fix** — returning incorrect errno values from syscalls
is a well-understood class of bug that affects userspace programs' error
handling. It is:
- Small and surgical (one line)
- Obviously correct (EPERM is the right code for permission denial)
- Low risk (only changes a failure path's error code)
- Fixes a real userspace-visible issue (misleading errno)
### 7. CONCERNS
- **No companion fix for listmount()**: The same bug exists in
`listmount()` but this commit only addresses `statmount()`. Ideally
both should be backported together.
- **UAPI change**: This technically changes the behavior of a syscall
(different errno), which is visible to userspace. However, since the
previous value was incorrect and misleading, this is a correction, not
a regression.
### 8. VERDICT
This is a clear, minimal, correct bug fix to a syscall's error handling.
It returns the correct standard POSIX errno (`EPERM`) instead of a
misleading one (`ENOENT`) when a permission check fails. It's a one-line
change with zero risk of regression, authored with involvement from
senior VFS maintainers (Miklos Szeredi suggesting, Christian Brauner
merging). It applies cleanly to all affected stable trees (6.11+).
**YES**
fs/namespace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index c58674a20cad5..f6879f282daec 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -5780,7 +5780,7 @@ SYSCALL_DEFINE4(statmount, const struct mnt_id_req __user *, req,
if (kreq.mnt_ns_id && (ns != current->nsproxy->mnt_ns) &&
!ns_capable_noaudit(ns->user_ns, CAP_SYS_ADMIN))
- return -ENOENT;
+ return -EPERM;
ks = kmalloc(sizeof(*ks), GFP_KERNEL_ACCOUNT);
if (!ks)
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-5.10] audit: add missing syscalls to read class
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (14 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.12] statmount: permission check should return EPERM Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] hfsplus: pretend special inodes as regular files Sasha Levin
` (7 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Jeffrey Bencteux, Paul Moore, Sasha Levin, eparis, audit,
linux-arch
From: Jeffrey Bencteux <jeff@bencteux.fr>
[ Upstream commit bcb90a2834c7393c26df9609b889a3097b7700cd ]
The "at" variant of getxattr() and listxattr() are missing from the
audit read class. Calling getxattrat() or listxattrat() on a file to
read its extended attributes will bypass audit rules such as:
-w /tmp/test -p rwa -k test_rwa
The current patch adds missing syscalls to the audit read class.
Signed-off-by: Jeffrey Bencteux <jeff@bencteux.fr>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete understanding. Let me summarize the analysis.
## Analysis
### 1. Commit Message Analysis
The commit is clear: it adds `getxattrat` and `listxattrat` syscalls to
the audit "read" class. The commit message explains the concrete
consequence — calling these syscalls will **bypass** audit rules like
`-w /tmp/test -p rwa -k test_rwa`, which is a security monitoring gap.
### 2. Code Change Analysis
The change is only to `include/asm-generic/audit_read.h` — it adds 6
lines (two `#ifdef`-guarded entries). This file defines the
`read_class[]` array used by `audit_match_perm()` in `kernel/auditsc.c`
to determine whether a given syscall matches the "read" permission
class. The matching code:
```151:170:kernel/auditsc.c
static int audit_match_perm(struct audit_context *ctx, int mask)
{
unsigned n;
// ...
if ((mask & AUDIT_PERM_READ) &&
audit_match_class(AUDIT_CLASS_READ, n))
return 1;
// ...
}
```
Without `getxattrat` and `listxattrat` in the read class,
`audit_match_perm()` returns 0 for these syscalls when checking
`AUDIT_PERM_READ`, meaning audit watch rules with read (`r`) permission
never fire for these syscalls.
### 3. Root Cause — Omission in Original Syscall Introduction
The original commit `6140be90ec70c` ("fs/xattr: add *at family
syscalls") introduced all four `*xattrat` syscalls. It correctly added
`setxattrat` and `removexattrat` to `audit_change_attr.h` (the
attribute-change class), but **forgot** to add `getxattrat` and
`listxattrat` to `audit_read.h` (the read class). This is an
oversight/bug in the original commit.
### 4. Affected Stable Trees
- The `*xattrat` syscalls were introduced in **v6.13**.
- Stable trees **6.13.y through 6.18.y** all have these syscalls defined
but are **missing** the audit read class entries.
- Stable trees **6.12.y and older** do not have the syscalls, so the
`#ifdef` guards make this a safe no-op.
- I verified that 6.14.y has the syscalls AND is missing the audit
entries, confirming the bug is present.
### 5. Security Impact
This is a **security audit bypass**. Linux audit is a critical security
feature used for:
- Compliance monitoring (PCI-DSS, HIPAA, SOX)
- Intrusion detection
- Forensics
The `getxattrat()` syscall can read security-relevant extended
attributes (like `security.selinux` labels) without triggering audit
rules. An attacker or unauthorized user could use
`getxattrat()`/`listxattrat()` instead of `getxattr()`/`listxattr()` to
enumerate or read extended attributes while evading audit monitoring.
This is a concrete bypass of security controls.
### 6. Fix Quality
- **Trivially small**: 6 lines added, no lines removed, no logic changes
- **Obviously correct**: Follows the exact same `#ifdef __NR_xxx /
__NR_xxx, / #endif` pattern used by every other conditional entry in
the file
- **Safe**: The `#ifdef` guards mean it compiles cleanly even if the
syscall numbers don't exist
- **No dependencies**: This is a self-contained fix
- **Signed off by Paul Moore**: The audit subsystem maintainer
- **Zero risk of regression**: Adding entries to the read class array
cannot break anything; it only makes the audit matching more complete
### 7. Risk vs Benefit
- **Risk**: Essentially zero. The change is trivially small, obviously
correct, guarded by `#ifdef`, and touches only a static array
initializer.
- **Benefit**: Closes a security audit bypass gap for all users of audit
on kernels 6.13+.
**YES**
include/asm-generic/audit_read.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/asm-generic/audit_read.h b/include/asm-generic/audit_read.h
index 7bb7b5a83ae2e..fb9991f53fb6f 100644
--- a/include/asm-generic/audit_read.h
+++ b/include/asm-generic/audit_read.h
@@ -4,9 +4,15 @@ __NR_readlink,
#endif
__NR_quotactl,
__NR_listxattr,
+#ifdef __NR_listxattrat
+__NR_listxattrat,
+#endif
__NR_llistxattr,
__NR_flistxattr,
__NR_getxattr,
+#ifdef __NR_getxattrat
+__NR_getxattrat,
+#endif
__NR_lgetxattr,
__NR_fgetxattr,
#ifdef __NR_readlinkat
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-5.10] hfsplus: pretend special inodes as regular files
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (15 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] audit: add missing syscalls to read class Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] hfsplus: fix volume corruption issue for generic/498 Sasha Levin
` (6 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Tetsuo Handa, syzbot, Viacheslav Dubeyko, Sasha Levin, frank.li,
linux-fsdevel
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
[ Upstream commit ed8889ca21b6ab37bc1435c4009ce37a79acb9e6 ]
Since commit af153bb63a33 ("vfs: catch invalid modes in may_open()")
requires any inode be one of S_IFDIR/S_IFLNK/S_IFREG/S_IFCHR/S_IFBLK/
S_IFIFO/S_IFSOCK type, use S_IFREG for special inodes.
Reported-by: syzbot <syzbot+895c23f6917da440ed0d@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=895c23f6917da440ed0d
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/d0a07b1b-8b73-4002-8e29-e2bd56871262@I-love.SAKURA.ne.jp
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me compile my analysis.
---
## Detailed Analysis
### 1. Commit Message Analysis
The commit message is clear: HFS+ special/system inodes (catalog, extent
overflow, allocation bitmap, startup, and attributes files) never have
`i_mode` set, leaving it at 0 after `iget_locked()`. This violates the
new VFS requirement (from `af153bb63a33`) that all inodes must have a
valid `S_IF*` type when opened via `may_open()`.
The commit is reported by syzbot with a concrete reproducer and a link
to the syzkaller bug tracker. It's reviewed and signed off by the HFS+
maintainer Viacheslav Dubeyko.
### 2. Code Change Analysis
The fix is a single statement added to `hfsplus_system_read_inode()` in
`fs/hfsplus/super.c`:
```c
inode->i_mode = S_IFREG;
```
This is placed after the switch statement that reads the fork data for
each special inode type (HFSPLUS_EXT_CNID, HFSPLUS_CAT_CNID,
HFSPLUS_ALLOC_CNID, HFSPLUS_START_CNID, HFSPLUS_ATTR_CNID).
**The bug mechanism**: When `hfsplus_iget()` is called for a system
inode (inode number < `HFSPLUS_FIRSTUSER_CNID` and !=
`HFSPLUS_ROOT_CNID`), it calls `hfsplus_system_read_inode()`. This
function sets up the fork data and address space operations but **never
sets `i_mode`**. The `i_mode` remains 0 (the default from
`iget_locked()` → `alloc_inode()`). Zero is not a valid file type - it
doesn't match any of S_IFDIR, S_IFLNK, S_IFREG, S_IFCHR, S_IFBLK,
S_IFIFO, or S_IFSOCK.
When this inode is later opened via a userspace `openat()` syscall
(triggered by syzbot with a crafted filesystem), `may_open()` is called:
```4175:4232:fs/namei.c
static int may_open(struct mnt_idmap *idmap, const struct path *path,
int acc_mode, int flag)
{
// ...
switch (inode->i_mode & S_IFMT) {
case S_IFLNK:
// ...
case S_IFREG:
// ...
default:
VFS_BUG_ON_INODE(!IS_ANON_FILE(inode), inode);
}
```
With `i_mode = 0`, the switch falls to the default case, and since HFS+
special inodes aren't anonymous files, `VFS_BUG_ON_INODE` triggers →
**kernel BUG** (crash). This is confirmed by the syzbot crash report
showing `kernel BUG at fs/namei.c:3474` with a full stack trace through
`may_open` → `do_open` → `path_openat` → `do_filp_open` →
`do_sys_openat2` → `__x64_sys_openat`.
### 3. Relationship to Other Fixes
This is **NOT a duplicate** of `005d4b0d33f6` ("hfsplus: Verify inode
mode when loading from disk"). That fix addresses a DIFFERENT code path:
- `005d4b0d33f6` fixes `hfsplus_get_perms()` in `fs/hfsplus/inode.c` -
validates mode for **user/root inodes** loaded from disk (corrupted
on-disk mode)
- The commit we're analyzing fixes `hfsplus_system_read_inode()` in
`fs/hfsplus/super.c` - sets mode for **special/system inodes** that
never go through `hfsplus_get_perms()` at all
In `hfsplus_iget()`, the two paths are clearly separated:
- `inode->i_ino >= HFSPLUS_FIRSTUSER_CNID || inode->i_ino ==
HFSPLUS_ROOT_CNID` → `hfsplus_cat_read_inode()` →
`hfsplus_get_perms()` (fixed by `005d4b0d33f6`)
- Otherwise → `hfsplus_system_read_inode()` (fixed by THIS commit)
Confirmed via grep: there is literally NO `i_mode` reference in
`fs/hfsplus/super.c` currently.
### 4. CONFIG_DEBUG_VFS Dependency
The `VFS_BUG_ON_INODE` macro is only active when `CONFIG_DEBUG_VFS` is
enabled. Without it, it's compiled to `BUILD_BUG_ON_INVALID(cond)` which
is a no-op at runtime. `CONFIG_DEBUG_VFS` was introduced alongside
`af153bb63a33` in v6.15 (commit `8b17e540969a`).
So the **crash** only occurs on kernels 6.15+ with `CONFIG_DEBUG_VFS`
enabled. However, having `i_mode = 0` is technically incorrect on ALL
kernel versions - any code checking inode type would get wrong results
for these inodes.
### 5. Scope and Risk Assessment
- **Lines changed**: 6 (including comment) - effectively 1 functional
line
- **Files touched**: 1 (`fs/hfsplus/super.c`)
- **Risk**: Essentially zero. Setting `S_IFREG` on internal filesystem
inodes that previously had no type is strictly an improvement. These
inodes are filesystem metadata (btrees, allocation bitmap) - they are
file-like data structures, so `S_IFREG` is appropriate.
- **No behavioral regression**: The fix doesn't change how the
filesystem operates; it only ensures the inode has a valid VFS-level
type.
### 6. User Impact
- **Who's affected**: Anyone mounting HFS+ filesystems on kernels 6.15+
with `CONFIG_DEBUG_VFS`
- **Severity**: Kernel BUG/crash - system goes down
- **Trigger**: syzbot demonstrated this with crafted filesystem images,
but it could potentially also be triggered with corrupted real HFS+
volumes
- **syzbot confirmed**: 17,716 crashes recorded, reproducible with C
reproducer
### 7. Stability Indicators
- Reviewed-by: Viacheslav Dubeyko (HFS+ maintainer)
- Signed-off-by: Viacheslav Dubeyko (maintainer)
- syzbot reported with concrete reproducer
- Part of a series of similar fixes across multiple filesystems (bfs,
cramfs, isofs, jfs, minixfs, nilfs2, ntfs3, squashfs) - the ntfs3
equivalent `4e8011ffec79` is already merged
### 8. Dependencies
The prerequisite VFS commit `af153bb63a33` exists in 6.15+. For older
stable trees (6.14 and below), the `VFS_BUG_ON_INODE` doesn't exist, so
the crash won't happen. However, the fix itself applies cleanly
regardless of version since `hfsplus_system_read_inode()` has been
stable for many years. Setting a proper i_mode is correct behavior
independently of the VFS debug check.
### Conclusion
This is a clear **YES** for stable backporting:
- Fixes a syzbot-reported kernel BUG (crash) - 17,716 crash instances
recorded
- The fix is one functional line: `inode->i_mode = S_IFREG;`
- Zero risk of regression
- Obviously correct - internal inodes should have a valid file type
- Reviewed and accepted by the subsystem maintainer
- Complements the already-merged `005d4b0d33f6` by covering a different
code path
- Part of a broader class of fixes across multiple filesystems for the
same VFS requirement
**YES**
fs/hfsplus/super.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index aaffa9e060a0a..7f327b777ece8 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -53,6 +53,12 @@ static int hfsplus_system_read_inode(struct inode *inode)
return -EIO;
}
+ /*
+ * Assign a dummy file type, for may_open() requires that
+ * an inode has a valid file type.
+ */
+ inode->i_mode = S_IFREG;
+
return 0;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-5.10] hfsplus: fix volume corruption issue for generic/498
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (16 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] hfsplus: pretend special inodes as regular files Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.18] netfs: when subreq is marked for retry, do not check if it faced an error Sasha Levin
` (5 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Viacheslav Dubeyko, John Paul Adrian Glaubitz, Yangtao Li,
linux-fsdevel, Sasha Levin
From: Viacheslav Dubeyko <slava@dubeyko.com>
[ Upstream commit 9a8c4ad44721da4c48e1ff240ac76286c82837fe ]
The xfstests' test-case generic/498 leaves HFS+ volume
in corrupted state:
sudo ./check generic/498
FSTYP -- hfsplus
PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc1+ #18 SMP PREEMPT_DYNAMIC Thu Dec 4 12:24:45 PST 2025
MKFS_OPTIONS -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
generic/498 _check_generic_filesystem: filesystem on /dev/loop51 is inconsistent
(see XFSTESTS-2/xfstests-dev/results//generic/498.full for details)
Ran: generic/498
Failures: generic/498
Failed 1 of 1 tests
sudo fsck.hfsplus -d /dev/loop51
** /dev/loop51
Using cacheBlockSize=32K cacheTotalBlock=1024 cacheSize=32768K.
Executing fsck_hfs (version 540.1-Linux).
** Checking non-journaled HFS Plus Volume.
The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
Invalid leaf record count
(It should be 16 instead of 2)
** Checking multi-linked files.
CheckHardLinks: found 1 pre-Leopard file inodes.
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
Verify Status: VIStat = 0x0000, ABTStat = 0x0000 EBTStat = 0x0000
CBTStat = 0x8000 CatStat = 0x00000000
** Repairing volume.
** Rechecking volume.
** Checking non-journaled HFS Plus Volume.
The volume name is untitled
** Checking extents overflow file.
** Checking catalog file.
** Checking multi-linked files.
CheckHardLinks: found 1 pre-Leopard file inodes.
** Checking catalog hierarchy.
** Checking extended attributes file.
** Checking volume bitmap.
** Checking volume information.
** The volume untitled was repaired successfully.
The generic/498 test executes such steps on final phase:
mkdir $SCRATCH_MNT/A
mkdir $SCRATCH_MNT/B
mkdir $SCRATCH_MNT/A/C
touch $SCRATCH_MNT/B/foo
$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/B/foo
ln $SCRATCH_MNT/B/foo $SCRATCH_MNT/A/C/foo
$XFS_IO_PROG -c "fsync" $SCRATCH_MNT/A
"Simulate a power failure and mount the filesystem
to check that what we explicitly fsync'ed exists."
_flakey_drop_and_remount
The FSCK tool complains about "Invalid leaf record count".
HFS+ b-tree header contains leaf_count field is updated
by hfs_brec_insert() and hfs_brec_remove(). The hfs_brec_insert()
is involved into hard link creation process. However,
modified in-core leaf_count field is stored into HFS+
b-tree header by hfs_btree_write() method. But,
unfortunately, hfs_btree_write() hasn't been called
by hfsplus_cat_write_inode() and hfsplus_file_fsync()
stores not fully consistent state of the Catalog File's
b-tree.
This patch adds calling hfs_btree_write() method in
the hfsplus_cat_write_inode() with the goal of
storing consistent state of Catalog File's b-tree.
Finally, it makes FSCK tool happy.
sudo ./check generic/498
FSTYP -- hfsplus
PLATFORM -- Linux/x86_64 hfsplus-testing-0001 6.18.0-rc1+ #22 SMP PREEMPT_DYNAMIC Sat Dec 6 17:01:31 PST 2025
MKFS_OPTIONS -- /dev/loop51
MOUNT_OPTIONS -- /dev/loop51 /mnt/scratch
generic/498 33s ... 31s
Ran: generic/498
Passed all 1 tests
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
cc: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
cc: Yangtao Li <frank.li@vivo.com>
cc: linux-fsdevel@vger.kernel.org
Link: https://lore.kernel.org/r/20251207035821.3863657-1-slava@dubeyko.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Here is my detailed analysis:
---
## Commit Analysis: hfsplus: fix volume corruption issue for generic/498
### 1. The Bug: Filesystem Corruption After Power Failure
This commit fixes a **real volume corruption bug** in the HFS+
filesystem. The corruption occurs in a specific but realistic scenario:
creating a hard link, performing an fsync, then experiencing a power
failure. After recovery, `fsck.hfsplus` reports "Invalid leaf record
count" in the catalog b-tree header.
### 2. Root Cause Analysis
The bug involves a disconnect between in-memory btree metadata updates
and their persistence to disk:
**Step 1: `hfs_brec_insert()` updates `leaf_count` in memory only.**
```105:108:fs/hfsplus/brec.c
if (node->type == HFS_NODE_LEAF) {
tree->leaf_count++;
mark_inode_dirty(tree->inode);
}
```
When a hard link is created (e.g., `ln $SCRATCH_MNT/B/foo
$SCRATCH_MNT/A/C/foo`), `hfs_brec_insert()` is called on the catalog
b-tree. It increments `tree->leaf_count` in the in-memory `struct
hfs_btree` and calls `mark_inode_dirty(tree->inode)` to dirty the tree
inode — but it does NOT write the b-tree header record to disk.
**Step 2: `hfs_btree_write()` is what persists the header to disk.**
```283:311:fs/hfsplus/btree.c
int hfs_btree_write(struct hfs_btree *tree)
{
// ... finds node 0 (header node), maps page ...
head->leaf_count = cpu_to_be32(tree->leaf_count);
// ... other header fields ...
set_page_dirty(page);
hfs_bnode_put(node);
return 0;
}
```
This function serializes the in-memory btree metadata (including
`leaf_count`) to the header node's page and marks it dirty. This is the
**only** way the btree header gets updated on disk.
**Step 3: `hfsplus_cat_write_inode()` was NOT calling
`hfs_btree_write()`.**
Before this fix, `hfsplus_cat_write_inode()` (lines 612-691 in
`inode.c`) writes the catalog entry for a regular inode (folder/file
data, permissions, timestamps, etc.) but never flushes the btree header.
It sets `HFSPLUS_I_CAT_DIRTY`, which tells `hfsplus_file_fsync()` to
call `filemap_write_and_wait()` on the catalog tree's page cache — but
the header page was never dirtied in the first place.
**Step 4: The fsync path misses the btree header.**
In `hfsplus_file_fsync()`:
```339:345:fs/hfsplus/inode.c
sync_inode_metadata(inode, 1);
// ↑ This triggers hfsplus_write_inode() →
hfsplus_cat_write_inode()
// which writes the catalog ENTRY but NOT the btree HEADER
if (test_and_clear_bit(HFSPLUS_I_CAT_DIRTY, &hip->flags))
error =
filemap_write_and_wait(sbi->cat_tree->inode->i_mapping);
// ↑ This flushes dirty pages, but the header page isn't dirty!
```
**Step 5: Where it DID work — system inodes.**
For "system" inodes (the btree inodes themselves),
`hfsplus_system_write_inode()` in `super.c` DOES call
`hfs_btree_write()`:
```114:159:fs/hfsplus/super.c
// hfsplus_system_write_inode:
if (tree) {
int err = hfs_btree_write(tree);
// ...
}
```
But this path is only triggered during `hfsplus_sync_fs()` (full
filesystem sync), not during an individual file's fsync. So the btree
header only gets written on full sync, not on per-file fsync — creating
the corruption window.
### 3. The Fix
The fix adds `hfs_btree_write(tree)` to `hfsplus_cat_write_inode()`:
```diff
+ struct hfs_btree *tree = HFSPLUS_SB(inode->i_sb)->cat_tree;
...
- if (hfs_find_init(HFSPLUS_SB(main_inode->i_sb)->cat_tree, &fd))
+ if (hfs_find_init(tree, &fd))
...
hfs_find_exit(&fd);
+
+ if (!res) {
+ res = hfs_btree_write(tree);
+ if (res) {
+ pr_err("b-tree write err: %d, ino %lu\n",
+ res, inode->i_ino);
+ }
+ }
+
return res;
```
Now when `hfsplus_cat_write_inode()` runs (via `sync_inode_metadata`
during fsync), it also dirties the btree header page. When
`hfsplus_file_fsync()` then calls `filemap_write_and_wait()` on the
catalog tree's mapping, the header page (with the correct `leaf_count`)
gets flushed to disk. After power failure, the on-disk btree header
matches reality.
### 4. Scope and Risk Assessment
- **Lines changed**: ~10 lines added, 1 line modified (extracting `tree`
variable)
- **Files changed**: 1 file (`fs/hfsplus/inode.c`)
- **Function affected**: `hfsplus_cat_write_inode()` only
- **Pattern**: Identical to `hfsplus_system_write_inode()` which already
calls `hfs_btree_write()` for the same tree
- **Performance impact**: Minimal — `hfs_btree_write()` just writes to a
cached page and marks it dirty; the actual I/O happens asynchronously
or during the subsequent `filemap_write_and_wait()`
- **Risk of regression**: Very low — this makes the catalog inode write
path consistent with the system inode write path
### 5. Dependency Analysis
This commit is **completely standalone**:
- `hfs_btree_write()` has existed since the early HFS+ implementation
- The `HFSPLUS_SB()->cat_tree` structure hasn't changed
- The `hfsplus_cat_write_inode()` function's structure is stable across
kernel versions
- It does NOT depend on the companion fix `3f04ee216bc14` ("fix volume
corruption issue for generic/101") which modified
`hfsplus_file_fsync()` and `hfsplus_sync_fs()` — those are completely
different functions
- The patch should apply cleanly to any recent stable tree
### 6. User Impact
- **Severity**: HIGH — this is **filesystem/volume corruption** (data
integrity)
- **Trigger**: Any operation that modifies the catalog btree leaf count
(hard link creation/deletion) followed by fsync + power failure
- **Users affected**: Anyone using HFS+ filesystem (macOS interop,
embedded systems, dual-boot scenarios)
- **Reproducibility**: 100% reproducible via xfstests generic/498
### 7. Classification
This is a clear **data corruption fix**:
- Fixes metadata inconsistency after crash recovery
- The bug has been present since the HFS+ driver was written (the btree
header write was always missing from `hfsplus_cat_write_inode()`)
- Small, surgical fix using an established function
- No new features, APIs, or behavioral changes
- Zero risk to non-HFS+ users
The fix is small, surgical, and meets all stable kernel criteria.
**YES**
fs/hfsplus/inode.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index c762bf909d1aa..6153e5cc6eb65 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -615,6 +615,7 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
int hfsplus_cat_write_inode(struct inode *inode)
{
struct inode *main_inode = inode;
+ struct hfs_btree *tree = HFSPLUS_SB(inode->i_sb)->cat_tree;
struct hfs_find_data fd;
hfsplus_cat_entry entry;
int res = 0;
@@ -627,7 +628,7 @@ int hfsplus_cat_write_inode(struct inode *inode)
if (!main_inode->i_nlink)
return 0;
- if (hfs_find_init(HFSPLUS_SB(main_inode->i_sb)->cat_tree, &fd))
+ if (hfs_find_init(tree, &fd))
/* panic? */
return -EIO;
@@ -692,6 +693,15 @@ int hfsplus_cat_write_inode(struct inode *inode)
set_bit(HFSPLUS_I_CAT_DIRTY, &HFSPLUS_I(inode)->flags);
out:
hfs_find_exit(&fd);
+
+ if (!res) {
+ res = hfs_btree_write(tree);
+ if (res) {
+ pr_err("b-tree write err: %d, ino %lu\n",
+ res, inode->i_ino);
+ }
+ }
+
return res;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.18] netfs: when subreq is marked for retry, do not check if it faced an error
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (17 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] hfsplus: fix volume corruption issue for generic/498 Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19] hfs: Replace BUG_ON with error handling for CNID count checks Sasha Levin
` (4 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Shyam Prasad N, David Howells, Steve French, Sasha Levin, pc,
netfs, linux-fsdevel
From: Shyam Prasad N <sprasad@microsoft.com>
[ Upstream commit 82e8885bd7633a36ee9050e6d7f348a4155eed5f ]
The *_subreq_terminated functions today only process the NEED_RETRY
flag when the subreq was successful or failed with EAGAIN error.
However, there could be other retriable errors for network filesystems.
Avoid this by processing the NEED_RETRY irrespective of the error
code faced by the subreq. If it was specifically marked for retry,
the error code must not matter.
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me provide my detailed analysis.
---
## Comprehensive Analysis
### 1. Commit Message Analysis
The commit title "netfs: when subreq is marked for retry, do not check
if it faced an error" clearly describes a bug fix. The commit explains
that `*_subreq_terminated` functions only process the
`NETFS_SREQ_NEED_RETRY` flag when the subrequest was successful or had
an `-EAGAIN` error, but network filesystems may set `NEED_RETRY` for
other error codes.
The commit has:
- **Acked-by: David Howells** — the creator and maintainer of the netfs
subsystem
- **Signed-off-by: Shyam Prasad N** — a CIFS/SMB kernel developer at
Microsoft
- **Signed-off-by: Steve French** — the CIFS/SMB subsystem maintainer
### 2. Code Change Analysis — The Bug
**Read Path Bug** in `netfs_read_subreq_terminated()`
(`read_collect.c`):
When CIFS handles a `MID_REQUEST_SUBMITTED` or `MID_RETRY_NEEDED`
response (as seen in `smb2pdu.c` lines 4633-4640):
```4639:4640:fs/smb/client/smb2pdu.c
__set_bit(NETFS_SREQ_NEED_RETRY, &rdata->subreq.flags);
rdata->result = -EAGAIN;
```
Then at line 4707: `rdata->subreq.error = rdata->result;` (= -EAGAIN),
and line 4710 calls `netfs_read_subreq_terminated()`.
In the current code:
```532:547:fs/netfs/read_collect.c
if (!subreq->error && subreq->transferred < subreq->len) {
// ... this block is SKIPPED because error is -EAGAIN
(non-zero)
}
```
Then:
```549:560:fs/netfs/read_collect.c
if (unlikely(subreq->error < 0)) {
trace_netfs_failure(rreq, subreq, subreq->error,
netfs_fail_read);
if (subreq->source == NETFS_READ_FROM_CACHE) {
netfs_stat(&netfs_n_rh_read_failed);
__set_bit(NETFS_SREQ_NEED_RETRY,
&subreq->flags);
} else {
netfs_stat(&netfs_n_rh_download_failed);
__set_bit(NETFS_SREQ_FAILED, &subreq->flags);
// BUG: overrides NEED_RETRY!
}
```
For server downloads, `NETFS_SREQ_FAILED` is set **despite**
`NETFS_SREQ_NEED_RETRY` already being set by CIFS. Since the collection
code at line 269 checks `FAILED` **before** `NEED_RETRY` at line 277,
and the retry code at line 94 abandons if `FAILED` is set, the
subrequest is **permanently failed** instead of being retried.
**Write Path Bug** in `netfs_write_subrequest_terminated()`
(`write_collect.c`):
```493:498:fs/netfs/write_collect.c
if (IS_ERR_VALUE(transferred_or_error)) {
subreq->error = transferred_or_error;
if (subreq->error == -EAGAIN)
set_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags);
else
set_bit(NETFS_SREQ_FAILED, &subreq->flags);
```
AFS sets `NETFS_SREQ_NEED_RETRY` for auth errors (`-EACCES`, `-EPERM`,
`-ENOKEY`, etc.) at `fs/afs/write.c:165` before calling
`netfs_write_subrequest_terminated()` with those error codes. Since the
error is not `-EAGAIN`, the netfs layer overrides the filesystem's retry
request with `FAILED`.
### 3. The Fix
1. **`read_collect.c`**: Adds a check for `NETFS_SREQ_NEED_RETRY` before
the error handling block. If NEED_RETRY is set, it pauses the request
and jumps past error checks via `goto skip_error_checks`, preventing
FAILED from being set.
2. **`write_collect.c`**: Replaces the EAGAIN-only check with a check
for whether NEED_RETRY is already set. If set, FAILED is not applied.
This delegates retry decisions to the filesystem.
3. **`read_retry.c` / `write_issue.c`**: Adds `subreq->error = 0` in
`netfs_reissue_read()` and `netfs_reissue_write()` to clear stale
error codes before reissuing.
4. **`read_retry.c` abandon path**: Removes the `!subreq->error` check
since subrequests with `NEED_RETRY` can now legitimately carry error
codes.
### 4. User Impact
This bug directly affects:
- **CIFS/SMB users** experiencing network disruptions (laptop lid
close/open, WiFi drops, VPN changes): reads fail with errors instead
of transparently retrying
- **AFS users** needing key rotation during writes: writes fail instead
of retrying with a different key
- These are common real-world scenarios for enterprise and laptop users
### 5. Risk Assessment
**Positive:**
- Small patch (~30 lines across 4 files)
- ACKed by the netfs subsystem maintainer
- Logically sound: filesystems know better than the generic layer when
to retry
- Addresses concrete, user-facing failures
**Concerns:**
- The write path removes the EAGAIN→NEED_RETRY fallback. AFS's
`afs_alloc_operation()` failure (line 133) passes -EAGAIN without pre-
setting NEED_RETRY — this would now fail instead of retry. However,
David Howells (AFS author) ACKed this change, accepting the behavioral
shift.
- 9p similarly doesn't pre-set NEED_RETRY — -EAGAIN from 9p would no
longer trigger retry. This is likely intentional: the filesystem
should decide, not the generic layer.
- The affected code (with the new collector architecture) exists from
v6.14 onward. v6.14 has a slightly different function signature for
`netfs_write_subrequest_terminated` (with `was_async` parameter),
requiring minor backport adjustment.
### 6. Dependency Check
- No missing dependencies: `NETFS_RREQ_PAUSE`,
`netfs_rreq_trace_set_pause`, and all other referenced symbols exist
in the current stable trees.
- The patch is self-contained and doesn't appear to be part of a series.
- For v6.18 stable, the patch should apply cleanly as the code matches
exactly.
### 7. Conclusion
This commit fixes a real, user-visible bug where network filesystem I/O
operations marked for retry by CIFS and AFS are incorrectly treated as
permanent failures by the netfs generic layer. The bug causes spurious
I/O errors during common scenarios like network reconnections and
authentication retries. The fix is small, reviewed by the subsystem
maintainer, and addresses concrete failure paths. While it includes a
deliberate behavioral shift in the write path (removing EAGAIN special-
casing), this was reviewed and ACKed by the relevant maintainers. The
risk is low relative to the benefit of fixing CIFS reconnection failures
for a large user base.
**YES**
fs/netfs/read_collect.c | 10 ++++++++++
fs/netfs/read_retry.c | 4 ++--
fs/netfs/write_collect.c | 8 ++++----
fs/netfs/write_issue.c | 1 +
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index 7a0ffa675fb17..137f0e28a44c5 100644
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -546,6 +546,15 @@ void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq)
}
}
+ /* If need retry is set, error should not matter unless we hit too many
+ * retries. Pause the generation of new subreqs
+ */
+ if (test_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags)) {
+ trace_netfs_rreq(rreq, netfs_rreq_trace_set_pause);
+ set_bit(NETFS_RREQ_PAUSE, &rreq->flags);
+ goto skip_error_checks;
+ }
+
if (unlikely(subreq->error < 0)) {
trace_netfs_failure(rreq, subreq, subreq->error, netfs_fail_read);
if (subreq->source == NETFS_READ_FROM_CACHE) {
@@ -559,6 +568,7 @@ void netfs_read_subreq_terminated(struct netfs_io_subrequest *subreq)
set_bit(NETFS_RREQ_PAUSE, &rreq->flags);
}
+skip_error_checks:
trace_netfs_sreq(subreq, netfs_sreq_trace_terminated);
netfs_subreq_clear_in_progress(subreq);
netfs_put_subrequest(subreq, netfs_sreq_trace_put_terminated);
diff --git a/fs/netfs/read_retry.c b/fs/netfs/read_retry.c
index b99e84a8170af..7793ba5e3e8fc 100644
--- a/fs/netfs/read_retry.c
+++ b/fs/netfs/read_retry.c
@@ -12,6 +12,7 @@
static void netfs_reissue_read(struct netfs_io_request *rreq,
struct netfs_io_subrequest *subreq)
{
+ subreq->error = 0;
__clear_bit(NETFS_SREQ_MADE_PROGRESS, &subreq->flags);
__set_bit(NETFS_SREQ_IN_PROGRESS, &subreq->flags);
netfs_stat(&netfs_n_rh_retry_read_subreq);
@@ -242,8 +243,7 @@ static void netfs_retry_read_subrequests(struct netfs_io_request *rreq)
subreq = list_next_entry(subreq, rreq_link);
abandon:
list_for_each_entry_from(subreq, &stream->subrequests, rreq_link) {
- if (!subreq->error &&
- !test_bit(NETFS_SREQ_FAILED, &subreq->flags) &&
+ if (!test_bit(NETFS_SREQ_FAILED, &subreq->flags) &&
!test_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags))
continue;
subreq->error = -ENOMEM;
diff --git a/fs/netfs/write_collect.c b/fs/netfs/write_collect.c
index cbf3d9194c7bf..61eab34ea67ef 100644
--- a/fs/netfs/write_collect.c
+++ b/fs/netfs/write_collect.c
@@ -492,11 +492,11 @@ void netfs_write_subrequest_terminated(void *_op, ssize_t transferred_or_error)
if (IS_ERR_VALUE(transferred_or_error)) {
subreq->error = transferred_or_error;
- if (subreq->error == -EAGAIN)
- set_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags);
- else
+ /* if need retry is set, error should not matter */
+ if (!test_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags)) {
set_bit(NETFS_SREQ_FAILED, &subreq->flags);
- trace_netfs_failure(wreq, subreq, transferred_or_error, netfs_fail_write);
+ trace_netfs_failure(wreq, subreq, transferred_or_error, netfs_fail_write);
+ }
switch (subreq->source) {
case NETFS_WRITE_TO_CACHE:
diff --git a/fs/netfs/write_issue.c b/fs/netfs/write_issue.c
index dd8743bc8d7fe..34894da5a23ec 100644
--- a/fs/netfs/write_issue.c
+++ b/fs/netfs/write_issue.c
@@ -250,6 +250,7 @@ void netfs_reissue_write(struct netfs_io_stream *stream,
iov_iter_truncate(&subreq->io_iter, size);
subreq->retry_count++;
+ subreq->error = 0;
__clear_bit(NETFS_SREQ_MADE_PROGRESS, &subreq->flags);
__set_bit(NETFS_SREQ_IN_PROGRESS, &subreq->flags);
netfs_stat(&netfs_n_wh_retry_write_subreq);
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19] hfs: Replace BUG_ON with error handling for CNID count checks
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (18 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.18] netfs: when subreq is marked for retry, do not check if it faced an error Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.1] smb: client: add proper locking around ses->iface_last_update Sasha Levin
` (3 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Jori Koolstra, syzbot+17cc9bb6d8d69b4139f0, Viacheslav Dubeyko,
Sasha Levin, frank.li, linux-fsdevel
From: Jori Koolstra <jkoolstra@xs4all.nl>
[ Upstream commit b226804532a875c10276168dc55ce752944096bd ]
In a06ec283e125 next_id, folder_count, and file_count in the super block
info were expanded to 64 bits, and BUG_ONs were added to detect
overflow. This triggered an error reported by syzbot: if the MDB is
corrupted, the BUG_ON is triggered. This patch replaces this mechanism
with proper error handling and resolves the syzbot reported bug.
Singed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
Reported-by: syzbot+17cc9bb6d8d69b4139f0@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=17cc9bb6d8d69b4139f0
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/20251220191006.2465256-1-jkoolstra@xs4all.nl
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have the complete picture. Let me write the final analysis.
---
## Comprehensive Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message clearly states:
- It fixes BUG_ON() calls introduced by commit `a06ec283e125` ("hfs: add
logic of correcting a next unused CNID")
- The BUG_ON triggers on corrupted MDB (Master Directory Block) data
- Reported by syzbot with a reproducible crash
- Reviewed by the HFS subsystem maintainer (Viacheslav Dubeyko)
### 2. CODE CHANGE ANALYSIS
The patch touches 5 files within `fs/hfs/`:
**a) `fs/hfs/inode.c` - `hfs_new_inode()`:** Replaces three
`BUG_ON(value > U32_MAX)` checks with proper error handling:
- If `next_id > U32_MAX`: reverses the `atomic64_inc`, prints error,
jumps to `out_discard` which calls `iput(inode)` then returns
`ERR_PTR(-ERANGE)`
- If `folder_count > U32_MAX`: same pattern with `atomic64_dec` rollback
- If `file_count > U32_MAX`: same pattern
- The function return type changes from `NULL` on error to `ERR_PTR()`
on error
**b) `fs/hfs/inode.c` - `hfs_delete_inode()`:** Removes two `BUG_ON()`
calls that checked counts before decrementing. These are safe to remove
because the increment path now prevents exceeding U32_MAX.
**c) `fs/hfs/dir.c` - `hfs_create()` and `hfs_mkdir()`:** Updated
callers to check `IS_ERR(inode)` / `PTR_ERR(inode)` instead of `!inode`,
matching the new return convention.
**d) `fs/hfs/dir.c` - `hfs_remove()`:** Adds a pre-check via
`is_hfs_cnid_counts_valid()` before removing files/directories to
prevent operations on a corrupted filesystem.
**e) `fs/hfs/mdb.c` - `hfs_mdb_commit()`:** Removes three `BUG_ON()`
calls in the MDB commit path. These were the most dangerous - they could
trigger during periodic writeback work, causing a panic even without
user interaction after mount.
**f) `fs/hfs/mdb.c` - `hfs_mdb_get()`:** Adds mount-time validation via
`is_hfs_cnid_counts_valid()`. If the on-disk values are corrupted (>
U32_MAX), the filesystem is forced to read-only mode, preventing the
BUG_ON paths from ever being reached.
**g) `fs/hfs/mdb.c` - `is_hfs_cnid_counts_valid()`:** New helper
function that validates all three CNID-related counters.
**h) `fs/hfs/super.c` - `hfs_sync_fs()` and `flush_mdb()`:** Adds
`is_hfs_cnid_counts_valid()` calls before `hfs_mdb_commit()` as an extra
safety check.
### 3. THE BUG MECHANISM
Commit `a06ec283e125` (merged in v6.18) expanded `next_id`,
`file_count`, and `folder_count` from `u32` to `atomic64_t` for
atomicity, and added `BUG_ON()` to detect overflow past `U32_MAX`.
However, these values are read from the on-disk MDB at mount time via
`be32_to_cpu()` into `atomic64_t`. If the MDB is corrupted such that
these 32-bit on-disk values, when interpreted, lead to increments past
`U32_MAX` (the on-disk `drNxtCNID` could be `0xFFFFFFFF` or similar),
then creating a new file triggers `BUG_ON(next_id > U32_MAX)` at line
222 of `inode.c`.
The syzbot crash report confirms this exact scenario:
- **Crash site:** `kernel BUG at fs/hfs/inode.c:222` in
`hfs_new_inode()`
- **Call path:** `openat() -> path_openat() -> lookup_open() ->
hfs_create() -> hfs_new_inode()`
- **Trigger:** Opening a file for creation on a corrupted HFS filesystem
image
- **Impact:** Instant kernel panic/oops (invalid opcode trap from
BUG_ON)
### 4. DEPENDENCY ANALYSIS
**Critical dependency:** This fix ONLY applies to kernels containing
`a06ec283e125`, which first appeared in v6.18-rc1. The BUG_ON code does
not exist in v6.17 or earlier kernels. In those older kernels,
`next_id`, `file_count`, and `folder_count` are plain `u32` with simple
`++`/`--` operations and no overflow checks.
- **v6.18.y stable:** Has the BUG_ON (confirmed in v6.18.5). NEEDS this
fix.
- **v6.12.y and older:** Does NOT have the BUG_ON. Does NOT need this
fix.
### 5. SEVERITY AND IMPACT
- **Severity:** HIGH. This is a kernel BUG/panic triggered by corrupted
filesystem data. Any user mounting a damaged or maliciously crafted
HFS image can crash the kernel.
- **Attack surface:** HFS images can come from USB sticks, CD-ROMs, disk
images, or network mounts. The crash is reachable from the `openat()`
syscall (creating a file on a mounted corrupted HFS).
- **Security implications:** A crafted HFS image can crash the kernel,
constituting a denial-of-service vulnerability. This is especially
concerning for systems that auto-mount removable media.
### 6. RISK ASSESSMENT
- **Size:** ~130 lines changed across 5 files, all within `fs/hfs/`
- **Scope:** Entirely contained to the HFS filesystem subsystem
- **Pattern:** Well-understood "replace BUG_ON with error handling"
pattern
- **Quality:** Reviewed by the HFS maintainer, tested by syzbot (patch
testing passed), and went through 4 patch revisions (v1 through v4)
- **Risk:** LOW. The changes are:
- Mount-time validation forces corrupted filesystems read-only (safe)
- BUG_ON replaced with `return ERR_PTR(-ERANGE)` (graceful failure)
- Callers updated to handle the new error convention
- Atomic rollbacks on error (correct)
### 7. MINOR CONCERNS
- There's a minor whitespace issue (spaces vs tabs in `out_discard:`
label indentation and `hfs_remove()`) - cosmetic only, no functional
impact
- The `is_hfs_cnid_counts_valid()` calls in `hfs_sync_fs()` and
`flush_mdb()` only print warnings but don't prevent `hfs_mdb_commit()`
from running - however, the BUG_ONs in `hfs_mdb_commit()` are already
removed, so this is a soft warning rather than crash prevention
- The commit has a typo "Singed-off-by" (should be "Signed-off-by") but
this doesn't affect the code
### 8. CONCLUSION
This is a textbook stable backport candidate:
- Fixes a syzbot-reported, reproducible kernel BUG/panic
- The bug is triggered from userspace via a common syscall path
(`openat()`)
- Corrupted filesystem images are a well-known attack vector
- The fix is contained, reviewed, and follows the standard "BUG_ON ->
error handling" pattern
- It applies specifically to v6.18.y stable which contains the buggy
BUG_ON code
The fix is small, surgical, and meets all stable kernel criteria.
**YES**
fs/hfs/dir.c | 15 +++++++++++----
fs/hfs/hfs_fs.h | 1 +
fs/hfs/inode.c | 30 ++++++++++++++++++++++++------
fs/hfs/mdb.c | 31 +++++++++++++++++++++++++++----
fs/hfs/super.c | 3 +++
5 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
index 86a6b317b474a..0c615c078650c 100644
--- a/fs/hfs/dir.c
+++ b/fs/hfs/dir.c
@@ -196,8 +196,8 @@ static int hfs_create(struct mnt_idmap *idmap, struct inode *dir,
int res;
inode = hfs_new_inode(dir, &dentry->d_name, mode);
- if (!inode)
- return -ENOMEM;
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
res = hfs_cat_create(inode->i_ino, dir, &dentry->d_name, inode);
if (res) {
@@ -226,8 +226,8 @@ static struct dentry *hfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
int res;
inode = hfs_new_inode(dir, &dentry->d_name, S_IFDIR | mode);
- if (!inode)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(inode))
+ return ERR_CAST(inode);
res = hfs_cat_create(inode->i_ino, dir, &dentry->d_name, inode);
if (res) {
@@ -254,11 +254,18 @@ static struct dentry *hfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
*/
static int hfs_remove(struct inode *dir, struct dentry *dentry)
{
+ struct super_block *sb = dir->i_sb;
struct inode *inode = d_inode(dentry);
int res;
if (S_ISDIR(inode->i_mode) && inode->i_size != 2)
return -ENOTEMPTY;
+
+ if (unlikely(!is_hfs_cnid_counts_valid(sb))) {
+ pr_err("cannot remove file/folder\n");
+ return -ERANGE;
+ }
+
res = hfs_cat_delete(inode->i_ino, dir, &dentry->d_name);
if (res)
return res;
diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index e94dbc04a1e43..ac0e83f77a0f1 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -199,6 +199,7 @@ extern void hfs_delete_inode(struct inode *inode);
extern const struct xattr_handler * const hfs_xattr_handlers[];
/* mdb.c */
+extern bool is_hfs_cnid_counts_valid(struct super_block *sb);
extern int hfs_mdb_get(struct super_block *sb);
extern void hfs_mdb_commit(struct super_block *sb);
extern void hfs_mdb_close(struct super_block *sb);
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 524db1389737d..878535db64d67 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -187,16 +187,23 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
s64 next_id;
s64 file_count;
s64 folder_count;
+ int err = -ENOMEM;
if (!inode)
- return NULL;
+ goto out_err;
+
+ err = -ERANGE;
mutex_init(&HFS_I(inode)->extents_lock);
INIT_LIST_HEAD(&HFS_I(inode)->open_dir_list);
spin_lock_init(&HFS_I(inode)->open_dir_lock);
hfs_cat_build_key(sb, (btree_key *)&HFS_I(inode)->cat_key, dir->i_ino, name);
next_id = atomic64_inc_return(&HFS_SB(sb)->next_id);
- BUG_ON(next_id > U32_MAX);
+ if (next_id > U32_MAX) {
+ atomic64_dec(&HFS_SB(sb)->next_id);
+ pr_err("cannot create new inode: next CNID exceeds limit\n");
+ goto out_discard;
+ }
inode->i_ino = (u32)next_id;
inode->i_mode = mode;
inode->i_uid = current_fsuid();
@@ -210,7 +217,11 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
if (S_ISDIR(mode)) {
inode->i_size = 2;
folder_count = atomic64_inc_return(&HFS_SB(sb)->folder_count);
- BUG_ON(folder_count > U32_MAX);
+ if (folder_count> U32_MAX) {
+ atomic64_dec(&HFS_SB(sb)->folder_count);
+ pr_err("cannot create new inode: folder count exceeds limit\n");
+ goto out_discard;
+ }
if (dir->i_ino == HFS_ROOT_CNID)
HFS_SB(sb)->root_dirs++;
inode->i_op = &hfs_dir_inode_operations;
@@ -220,7 +231,11 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
} else if (S_ISREG(mode)) {
HFS_I(inode)->clump_blocks = HFS_SB(sb)->clumpablks;
file_count = atomic64_inc_return(&HFS_SB(sb)->file_count);
- BUG_ON(file_count > U32_MAX);
+ if (file_count > U32_MAX) {
+ atomic64_dec(&HFS_SB(sb)->file_count);
+ pr_err("cannot create new inode: file count exceeds limit\n");
+ goto out_discard;
+ }
if (dir->i_ino == HFS_ROOT_CNID)
HFS_SB(sb)->root_files++;
inode->i_op = &hfs_file_inode_operations;
@@ -244,6 +259,11 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
hfs_mark_mdb_dirty(sb);
return inode;
+
+ out_discard:
+ iput(inode);
+ out_err:
+ return ERR_PTR(err);
}
void hfs_delete_inode(struct inode *inode)
@@ -252,7 +272,6 @@ void hfs_delete_inode(struct inode *inode)
hfs_dbg("ino %lu\n", inode->i_ino);
if (S_ISDIR(inode->i_mode)) {
- BUG_ON(atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX);
atomic64_dec(&HFS_SB(sb)->folder_count);
if (HFS_I(inode)->cat_key.ParID == cpu_to_be32(HFS_ROOT_CNID))
HFS_SB(sb)->root_dirs--;
@@ -261,7 +280,6 @@ void hfs_delete_inode(struct inode *inode)
return;
}
- BUG_ON(atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX);
atomic64_dec(&HFS_SB(sb)->file_count);
if (HFS_I(inode)->cat_key.ParID == cpu_to_be32(HFS_ROOT_CNID))
HFS_SB(sb)->root_files--;
diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
index 53f3fae602179..e0150945cf13b 100644
--- a/fs/hfs/mdb.c
+++ b/fs/hfs/mdb.c
@@ -64,6 +64,27 @@ static int hfs_get_last_session(struct super_block *sb,
return 0;
}
+bool is_hfs_cnid_counts_valid(struct super_block *sb)
+{
+ struct hfs_sb_info *sbi = HFS_SB(sb);
+ bool corrupted = false;
+
+ if (unlikely(atomic64_read(&sbi->next_id) > U32_MAX)) {
+ pr_warn("next CNID exceeds limit\n");
+ corrupted = true;
+ }
+ if (unlikely(atomic64_read(&sbi->file_count) > U32_MAX)) {
+ pr_warn("file count exceeds limit\n");
+ corrupted = true;
+ }
+ if (unlikely(atomic64_read(&sbi->folder_count) > U32_MAX)) {
+ pr_warn("folder count exceeds limit\n");
+ corrupted = true;
+ }
+
+ return !corrupted;
+}
+
/*
* hfs_mdb_get()
*
@@ -156,6 +177,11 @@ int hfs_mdb_get(struct super_block *sb)
atomic64_set(&HFS_SB(sb)->file_count, be32_to_cpu(mdb->drFilCnt));
atomic64_set(&HFS_SB(sb)->folder_count, be32_to_cpu(mdb->drDirCnt));
+ if (!is_hfs_cnid_counts_valid(sb)) {
+ pr_warn("filesystem possibly corrupted, running fsck.hfs is recommended. Mounting read-only.\n");
+ sb->s_flags |= SB_RDONLY;
+ }
+
/* TRY to get the alternate (backup) MDB. */
sect = part_start + part_size - 2;
bh = sb_bread512(sb, sect, mdb2);
@@ -209,7 +235,7 @@ int hfs_mdb_get(struct super_block *sb)
attrib = mdb->drAtrb;
if (!(attrib & cpu_to_be16(HFS_SB_ATTRIB_UNMNT))) {
- pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended. mounting read-only.\n");
+ pr_warn("filesystem was not cleanly unmounted, running fsck.hfs is recommended. Mounting read-only.\n");
sb->s_flags |= SB_RDONLY;
}
if ((attrib & cpu_to_be16(HFS_SB_ATTRIB_SLOCK))) {
@@ -273,15 +299,12 @@ void hfs_mdb_commit(struct super_block *sb)
/* These parameters may have been modified, so write them back */
mdb->drLsMod = hfs_mtime();
mdb->drFreeBks = cpu_to_be16(HFS_SB(sb)->free_ablocks);
- BUG_ON(atomic64_read(&HFS_SB(sb)->next_id) > U32_MAX);
mdb->drNxtCNID =
cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->next_id));
mdb->drNmFls = cpu_to_be16(HFS_SB(sb)->root_files);
mdb->drNmRtDirs = cpu_to_be16(HFS_SB(sb)->root_dirs);
- BUG_ON(atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX);
mdb->drFilCnt =
cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->file_count));
- BUG_ON(atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX);
mdb->drDirCnt =
cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->folder_count));
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 47f50fa555a45..70e118c27e200 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -34,6 +34,7 @@ MODULE_LICENSE("GPL");
static int hfs_sync_fs(struct super_block *sb, int wait)
{
+ is_hfs_cnid_counts_valid(sb);
hfs_mdb_commit(sb);
return 0;
}
@@ -65,6 +66,8 @@ static void flush_mdb(struct work_struct *work)
sbi->work_queued = 0;
spin_unlock(&sbi->work_lock);
+ is_hfs_cnid_counts_valid(sb);
+
hfs_mdb_commit(sb);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.1] smb: client: add proper locking around ses->iface_last_update
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (19 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19] hfs: Replace BUG_ON with error handling for CNID count checks Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.6] btrfs: handle user interrupt properly in btrfs_trim_fs() Sasha Levin
` (2 subsequent siblings)
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Henrique Carvalho, Steve French, Sasha Levin, sfrench, linux-cifs,
samba-technical
From: Henrique Carvalho <henrique.carvalho@suse.com>
[ Upstream commit e97dcac3dc0bd37e4b56aaa6874b572a3a461102 ]
There is a missing ses->iface_lock in cifs_setup_session,
around ses->iface_last_update.
Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The locking infrastructure and documentation are identical in the stable
branch. Now I have all the information needed for a thorough analysis.
---
## Complete Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit subject is "smb: client: add proper locking around
ses->iface_last_update". The body explains that there is a "missing
ses->iface_lock in cifs_setup_session, around ses->iface_last_update."
This is a direct, clear statement of a locking bug fix. The commit is
authored by Henrique Carvalho (SUSE employee) and accepted by Steve
French (the SMB/CIFS maintainer).
### 2. CODE CHANGE ANALYSIS
The patch is minimal — exactly 2 lines added:
```c
+ spin_lock(&ses->iface_lock);
ses->iface_last_update = 0;
+ spin_unlock(&ses->iface_lock);
```
This wraps the write to `ses->iface_last_update` (which forces an
interface list refresh) with the `ses->iface_lock` spinlock.
### 3. BUG MECHANISM — THE DATA RACE
**The documentation is unambiguous.** In `fs/smb/client/cifsglob.h`:
```1111:1123:fs/smb/client/cifsglob.h
- iface_lock should be taken when accessing any of these fields
*/
spinlock_t iface_lock;
/* ========= begin: protected by iface_lock ======== */
struct list_head iface_list;
size_t iface_count;
unsigned long iface_last_update; /* jiffies */
/* ========= end: protected by iface_lock ======== */
```
And in the locking documentation at lines 2000-2002:
```2000:2002:fs/smb/client/cifsglob.h
- cifs_ses->iface_lock cifs_ses->iface_list
sesInfoAlloc
- ->iface_count
- ->iface_last_update
```
`iface_last_update` is explicitly documented as protected by
`iface_lock`. The buggy code at `connect.c:4273` writes to this field
without holding the lock.
**Concurrent access paths that create the race:**
1. **Writer (buggy, in `cifs_setup_session`):** Sets
`ses->iface_last_update = 0` during session reconnect — called from
`smb2_reconnect()`, `cifs_reconnect_tcon()`, `cifs_get_smb_ses()`,
and `cifs_ses_add_channel()`.
2. **Reader (in `SMB3_request_interfaces`, smb2ops.c:828):** Reads
`iface_last_update` as an optimization check before performing an
expensive ioctl. This is called from the
`smb2_query_server_interfaces` delayed work that runs periodically
every `SMB_INTERFACE_POLL_INTERVAL` seconds.
3. **Reader/Writer (in `parse_server_interfaces`, smb2ops.c:641):**
Reads `iface_last_update` with `iface_lock` held, writes
`iface_last_update` at lines 669 and 798.
**The concrete race scenario:** Thread A is doing a session reconnect
and calls `cifs_setup_session()` which writes `iface_last_update = 0`
without the lock. Concurrently, Thread B (the periodic delayed work
`smb2_query_server_interfaces`) calls
`SMB3_request_interfaces`/`parse_server_interfaces` which reads
`iface_last_update` under `iface_lock`. This is a classic data race
where a writer and reader access shared data without consistent
synchronization.
On weakly-ordered architectures (ARM64), this can lead to the reader
seeing a torn or stale value of `iface_last_update`, potentially causing
the interface list not to be refreshed when it should be (or vice
versa). Even on x86-64 where `unsigned long` writes are naturally
atomic, this violates the documented locking discipline and could
confuse KCSAN (Kernel Concurrency Sanitizer).
### 4. SCOPE AND RISK ASSESSMENT
- **Size:** 2 lines added — the smallest possible fix
- **Files touched:** 1 (`fs/smb/client/connect.c`)
- **Risk of regression:** Extremely low. The added
`spin_lock/spin_unlock` pair is properly nested inside the already-
held `ses_lock`. Looking at the locking hierarchy documented in
`cifsglob.h`, `ses_lock` and `iface_lock` are independent spinlocks
(no nested ordering requirement documented). The critical section is
one `unsigned long` assignment — negligible contention.
- **Subsystem:** SMB client — a filesystem used by many enterprise users
for networked file access
### 5. USER IMPACT
This affects all users of the SMB/CIFS client who use multichannel
sessions (Azure files, enterprise NAS). The race occurs during session
reconnect — a critical recovery path that fires when a server connection
drops. The race could cause:
- **Missed interface refresh:** If the write to `iface_last_update = 0`
tears or is lost due to the race, the client might not refresh the
interface list during reconnect, potentially connecting secondary
channels to stale/wrong IP addresses
- **Incorrect reconnection behavior:** After a server failover (common
with Azure files), secondary channels might connect to the wrong
server
### 6. AFFECTED VERSIONS
The buggy commit `d9a6d78096056a3cb5c5f07a730ab92f2f9ac4e6` was
introduced in v6.7-rc1 and was already backported to stable branches:
- 6.1.y (as `c9569bfd2868`)
- 6.6.y (as `aabf4851d160`)
- 6.12.y (as `d9a6d78096056`)
The code context is identical in all these branches — the fix applies
cleanly.
### 7. DEPENDENCY CHECK
This fix is completely self-contained. It requires only:
- `ses->iface_lock` spinlock to exist (present since the multichannel
feature was added)
- The `iface_last_update` field to be in the `iface_lock`-protected
region (documented since its introduction)
No other commits are needed.
### 8. CLASSIFICATION
This is a **race condition fix** (data race on a shared variable
accessed without proper synchronization). It falls squarely into the
"synchronization changes" category of stable-worthy fixes. The fix is:
- Obviously correct (adds documented required locking)
- Tested (accepted by maintainer)
- Small (2 lines)
- Fixes a real bug (data race during reconnect)
- Contained (no side effects)
- Does not introduce new features or APIs
**YES**
fs/smb/client/connect.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index ce620503e9f70..60c76375f0f50 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -4270,7 +4270,9 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
ses->ses_status = SES_IN_SETUP;
/* force iface_list refresh */
+ spin_lock(&ses->iface_lock);
ses->iface_last_update = 0;
+ spin_unlock(&ses->iface_lock);
}
spin_unlock(&ses->ses_lock);
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-6.6] btrfs: handle user interrupt properly in btrfs_trim_fs()
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (20 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.1] smb: client: add proper locking around ses->iface_last_update Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] minix: Add required sanity checking to minix_check_superblock() Sasha Levin
2026-02-11 7:56 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Adrian Hunter
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: jinbaohong, Qu Wenruo, Robbie Ko, Filipe Manana, David Sterba,
Sasha Levin, clm, linux-btrfs
From: jinbaohong <jinbaohong@synology.com>
[ Upstream commit bfb670b9183b0e4ba660aff2e396ec1cc01d0761 ]
When a fatal signal is pending or the process is freezing,
btrfs_trim_block_group() and btrfs_trim_free_extents() return -ERESTARTSYS.
Currently this is treated as a regular error: the loops continue to the
next iteration and count it as a block group or device failure.
Instead, break out of the loops immediately and return -ERESTARTSYS to
userspace without counting it as a failure. Also skip the device loop
entirely if the block group loop was interrupted.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Robbie Ko <robbieko@synology.com>
Signed-off-by: jinbaohong <jinbaohong@synology.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete understanding. Let me provide my thorough
analysis.
---
## Detailed Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message is clear and precise: when `btrfs_trim_block_group()`
or `btrfs_trim_free_extents()` return `-ERESTARTSYS` (due to fatal
signal or freezing), the current code treats it as a regular error. It
continues the loops, counts it as a failure, and logs misleading
warnings. The fix is to break out immediately and return `-ERESTARTSYS`
to userspace.
The commit has excellent review pedigree: reviewed by Qu Wenruo, Filipe
Manana, and David Sterba (the btrfs maintainer himself). Three separate
reviews is strong confidence.
### 2. CODE CHANGE ANALYSIS
The fix touches a single function `btrfs_trim_fs()` in `fs/btrfs/extent-
tree.c`, adding 11 lines across three locations:
**Location 1** - Block group loop: After `btrfs_trim_block_group()`
returns, check for `-ERESTARTSYS`/`-EINTR` and break immediately:
```c
if (ret == -ERESTARTSYS || ret == -EINTR) {
btrfs_put_block_group(cache);
break;
}
```
Note the critical `btrfs_put_block_group(cache)` call before `break` —
this prevents a reference count leak. When using `continue`, the loop
iterator `cache = btrfs_next_block_group(cache)` handles putting the old
reference. But on `break`, we must do it explicitly. This matches the
existing pattern earlier in the same loop:
```6530:6533:fs/btrfs/extent-tree.c
if (cache->start >= range_end) {
btrfs_put_block_group(cache);
break;
}
```
**Location 2** - Between the two loops: Skip the device trimming loop
entirely if the block group loop was interrupted:
```c
if (ret == -ERESTARTSYS || ret == -EINTR)
return ret;
```
**Location 3** - Device loop and final return: Break out of the device
loop on interrupt and return appropriately:
```c
if (ret == -ERESTARTSYS || ret == -EINTR)
break;
...
if (ret == -ERESTARTSYS || ret == -EINTR)
return ret;
```
### 3. THE BUG MECHANISM
This bug is a **follow-up to commit `69313850dce33` ("btrfs: add
cancellation points to trim loops")** which was merged in v6.12 and is
`Cc: stable@vger.kernel.org # 5.15+`. That commit added
`btrfs_trim_interrupted()` checks to the inner trim functions
(`trim_no_bitmap`, `trim_bitmaps`, `btrfs_issue_discard`,
`btrfs_trim_free_extents`) so they return `-ERESTARTSYS` when a fatal
signal is pending or the process is freezing.
**The problem**: The outer function `btrfs_trim_fs()` was NOT updated to
handle this `-ERESTARTSYS` return. So the inner loops correctly detect
the interrupt and return early, but the outer loop just treats it as a
regular error and continues:
1. `btrfs_trim_block_group(cache_0)` → detects signal → returns
`-ERESTARTSYS`
2. Outer loop: `bg_failed++`, `bg_ret = -ERESTARTSYS`, `continue`
3. `btrfs_trim_block_group(cache_1)` → detects signal again → returns
`-ERESTARTSYS`
4. Repeat for ALL remaining block groups
5. Then iterate ALL devices, each returning `-ERESTARTSYS` immediately
On a large filesystem with thousands of block groups and multiple
devices, this means:
- **Delayed response to Ctrl+C/SIGKILL**: The process doesn't terminate
promptly
- **Blocked system suspend**: `freezing(current)` remains true, but the
outer loop keeps going, preventing the process from actually freezing.
This was the exact scenario reported in [bug
219180](https://bugzilla.kernel.org/show_bug.cgi?id=219180) and [SUSE
bug 1229737](https://bugzilla.suse.com/show_bug.cgi?id=1229737)
- **Misleading dmesg warnings**: `btrfs_warn(fs_info, "failed to trim
%llu block group(s)...")` fires, counting all the interrupted block
groups as "failures"
- **Wrong return value**: Instead of returning `-ERESTARTSYS` cleanly to
userspace, the function may return a mixed error code
### 4. SCOPE AND RISK ASSESSMENT
- **Size**: 11 lines added, 0 removed. Extremely small and surgical.
- **Files touched**: 1 (`fs/btrfs/extent-tree.c`)
- **Scope**: Only affects the interrupt/signal error path. The normal
trim path (no signal pending) is completely unaffected — all new code
is gated behind `ret == -ERESTARTSYS || ret == -EINTR` checks.
- **Risk**: Very low. The added checks are early-exit conditions that
only trigger when a signal is pending or process is freezing. There's
no way these can cause a regression in normal operation.
- **Reference counting**: Correctly handled
(`btrfs_put_block_group(cache)` before break).
### 5. USER IMPACT
- **Who is affected**: Any user running `fstrim` on a btrfs filesystem
who interrupts it (Ctrl+C) or has a system that suspends while trim is
running. This is a very common scenario, especially on laptops with
btrfs and periodic fstrim timers.
- **Call path**: `fstrim` → `FITRIM` ioctl → `btrfs_ioctl_fitrim()` →
`btrfs_trim_fs()`
- **Severity**: The original bugs from the linked reports were about
systems unable to suspend. The cancellation point commit
(`69313850dce33`) fixed the inner loops but left the outer loop
broken, meaning the fix was incomplete. This commit completes it.
### 6. DEPENDENCY CHECK
This commit depends on two preceding commits:
1. **`912d1c6680bdb` ("btrfs: continue trimming remaining devices on
failure")** - Changes `break` to `continue` in the device loop.
**Already targeted for stable** (`Fixes:` tag and `Cc:
stable@vger.kernel.org # 5.4+`).
2. **`1cc4ada4182fa` ("btrfs: preserve first error in
btrfs_trim_fs()")** - Changes `bg_ret = ret` to `if (!bg_ret) bg_ret
= ret`. **Not targeted for stable**. This is a small context
dependency; the core fix logic is independent of it.
Both prerequisites are small (1-line and 15-line changes respectively).
The first is already stable-bound. The second would be needed for clean
application but could alternatively be resolved by a minor context
adjustment during backport.
The fix also requires `69313850dce33` ("btrfs: add cancellation points
to trim loops") which is `Cc: stable # 5.15+` and should already be in
stable trees 5.15+.
### 7. STABILITY INDICATORS
- Reviewed by 3 btrfs experts
- The parent commit adding interruption infrastructure has been in
stable since 5.15+
- The fix is straightforward conditional checks — no complex logic
### 8. CLASSIFICATION
This is a **bug fix** that:
- Fixes incomplete signal/interrupt handling
- Fixes potential system suspend blocking
- Fixes misleading kernel warnings
- Fixes incorrect error propagation to userspace
- Completes an existing stable fix (`69313850dce33`)
It is NOT:
- A new feature
- A performance optimization
- A code cleanup
- An API change
### Summary
This commit fixes a real, user-visible bug where btrfs trim operations
cannot be properly interrupted by signals (Ctrl+C) or system suspend. It
completes the fix started by `69313850dce33` (already in stable 5.15+)
which added cancellation points to inner trim loops but left the outer
loop in `btrfs_trim_fs()` unaware of the `-ERESTARTSYS` return code. The
fix is extremely small (11 lines), well-reviewed by 3 btrfs developers
including the maintainer, has zero risk to normal code paths, correctly
handles reference counting, and addresses the same class of bug (suspend
blocking) that motivated the original stable-tagged commit. It requires
two small prerequisite commits (one already stable-targeted) for clean
application.
**YES**
fs/btrfs/extent-tree.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 1bf081243efb2..8bdb609f58a7e 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -6555,6 +6555,10 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
range->minlen);
trimmed += group_trimmed;
+ if (ret == -ERESTARTSYS || ret == -EINTR) {
+ btrfs_put_block_group(cache);
+ break;
+ }
if (ret) {
bg_failed++;
bg_ret = ret;
@@ -6568,6 +6572,9 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
"failed to trim %llu block group(s), last error %d",
bg_failed, bg_ret);
+ if (ret == -ERESTARTSYS || ret == -EINTR)
+ return ret;
+
mutex_lock(&fs_devices->device_list_mutex);
list_for_each_entry(device, &fs_devices->devices, dev_list) {
if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
@@ -6576,6 +6583,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
ret = btrfs_trim_free_extents(device, &group_trimmed);
trimmed += group_trimmed;
+ if (ret == -ERESTARTSYS || ret == -EINTR)
+ break;
if (ret) {
dev_failed++;
dev_ret = ret;
@@ -6589,6 +6598,8 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
"failed to trim %llu device(s), last error %d",
dev_failed, dev_ret);
range->len = trimmed;
+ if (ret == -ERESTARTSYS || ret == -EINTR)
+ return ret;
if (bg_ret)
return bg_ret;
return dev_ret;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.19-5.10] minix: Add required sanity checking to minix_check_superblock()
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (21 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.6] btrfs: handle user interrupt properly in btrfs_trim_fs() Sasha Levin
@ 2026-02-10 23:31 ` Sasha Levin
2026-02-11 7:56 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Adrian Hunter
23 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-02-10 23:31 UTC (permalink / raw)
To: patches, stable
Cc: Jori Koolstra, Jan Kara, syzbot+5ad0824204c7bf9b67f2,
Christian Brauner, Sasha Levin, penguin-kernel, mjguzik,
chentaotao
From: Jori Koolstra <jkoolstra@xs4all.nl>
[ Upstream commit 8c97a6ddc95690a938ded44b4e3202f03f15078c ]
The fs/minix implementation of the minix filesystem does not currently
support any other value for s_log_zone_size than 0. This is also the
only value supported in util-linux; see mkfs.minix.c line 511. In
addition, this patch adds some sanity checking for the other minix
superblock fields, and moves the minix_blocks_needed() checks for the
zmap and imap also to minix_check_super_block().
This also closes a related syzbot bug report.
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
Link: https://patch.msgid.link/20251208153947.108343-1-jkoolstra@xs4all.nl
Reviewed-by: Jan Kara <jack@suse.cz>
Reported-by: syzbot+5ad0824204c7bf9b67f2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5ad0824204c7bf9b67f2
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a complete picture. Let me write the comprehensive analysis.
---
## Comprehensive Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message clearly states it adds "required sanity checking" for
the minix superblock, specifically:
- Validates `s_log_zone_size == 0` (only supported value)
- Adds validation for `s_ninodes`, `s_firstdatazone`, and zone/inode map
block counts
- Moves existing `minix_blocks_needed()` checks from
`minix_fill_super()` to `minix_check_superblock()` (earlier
validation)
- Explicitly closes a **syzbot bug report** with a C reproducer
Key trust indicators:
- **Reported-by: syzbot** - fuzzer-found, reproducible bug with a
concrete C reproducer
- **Reviewed-by: Jan Kara** - experienced filesystem developer at SUSE
- **Signed-off-by: Christian Brauner** - VFS maintainer
- The syzbot bug has been open for **1521+ days** (since Dec 2021),
demonstrating a long-standing issue
### 2. CODE CHANGE ANALYSIS
**The root cause bug**: In `minix_statfs()` at line 415:
```415:415:fs/minix/inode.c
buf->f_blocks = (sbi->s_nzones - sbi->s_firstdatazone) <<
sbi->s_log_zone_size;
```
The `s_log_zone_size` field is read directly from the on-disk superblock
(`__u16` type, values 0-65535) and stored in `sbi->s_log_zone_size`
(unsigned long) **without any validation**. When a crafted/corrupt minix
image provides a large value (e.g., 768, as shown in the syzbot crash),
this causes a UBSAN shift-out-of-bounds because the shift exponent
exceeds 64 bits.
The same vulnerability also exists in `minix_count_free_blocks()` in
`bitmap.c`:
```102:103:fs/minix/bitmap.c
return (count_free(sbi->s_zmap, sb->s_blocksize, bits)
<< sbi->s_log_zone_size);
```
Additionally, `sbi->s_nzones - sbi->s_firstdatazone` can **underflow**
(unsigned subtraction wrapping around) if `s_firstdatazone >= s_nzones`,
affecting both `minix_statfs()` and `minix_count_free_blocks()`.
**What the fix does (line by line)**:
1. **Validates `s_log_zone_size == 0`**: The Linux minix implementation
doesn't support zone sizes different from block sizes. This is
consistent with `mkfs.minix` in util-linux. This single check
prevents the UBSAN shift-out-of-bounds.
2. **Validates `s_ninodes >= 1`**: Prevents issues with zero-inode
filesystems.
3. **Validates `s_firstdatazone > 4`**: The minimum layout of a minix FS
requires: boot block (0), superblock (1), at least 1 imap block (2),
at least 1 zmap block (3), at least 1 inode table block (4), so first
data zone must be >= 5.
4. **Validates `s_firstdatazone < s_nzones`**: Prevents unsigned
underflow in `s_nzones - s_firstdatazone` used in multiple places.
5. **Moves `minix_blocks_needed()` checks earlier**: Moves existing
imap/zmap block validation from after bitmap buffer allocation (in
`minix_fill_super()`) to before allocation (in
`minix_check_superblock()`). This is an improvement because it
rejects bad images before doing unnecessary I/O (sb_bread calls for
bitmap blocks).
6. **Removes the old `s_imap_blocks == 0 || s_zmap_blocks == 0` check**:
This is subsumed by the new `minix_blocks_needed()` checks — if
either block count is 0 while `s_ninodes >= 1`, the blocks_needed
check will catch it.
### 3. CLASSIFICATION
This is clearly a **bug fix** — specifically a UBSAN: shift-out-of-
bounds fix triggered by crafted filesystem images. It also fixes
potential unsigned integer underflow. There are zero new features.
### 4. SCOPE AND RISK ASSESSMENT
- **Lines changed**: 29 additions, 21 deletions — **small and
contained**
- **Files touched**: 1 file (`fs/minix/inode.c`)
- **Complexity**: Low — all changes are straightforward value
comparisons
- **Risk of regression**: Very low — the changes only add validation at
mount time. A valid minix filesystem would pass all checks. A
filesystem rejected by these checks was always corrupt/invalid.
- **Subsystem**: minix filesystem — mature, rarely modified, not in a
hot development path
### 5. USER IMPACT
- **Severity**: UBSAN/undefined behavior triggered from userspace (mount
syscall with crafted image)
- **Attack surface**: Any user who can mount a minix filesystem (often
root, but in some configurations could be unprivileged)
- **Syzbot confirmed affected stable trees**: linux-5.15 and linux-6.1
both have the same bug (syzbot "Similar bugs" section shows them as
unpatched: "0/3")
- **Reproducible**: Yes, syzbot provides a C reproducer
- **Duration**: The bug has been open since December 2021 — over 4 years
### 6. STABILITY INDICATORS
- Reviewed-by: Jan Kara (experienced FS developer)
- Signed-off-by: Christian Brauner (VFS maintainer)
- Tested by syzbot (patch testing returned OK)
- The commit has been in mainline since the 6.19 merge window
### 7. DEPENDENCY CHECK
- `minix_check_superblock()` was introduced in commit 270ef41094e9f (Aug
2020, with `Cc: stable`), so it **exists in all current stable
trees**.
- The function signature was changed to `struct super_block *sb` in
32ac86efff91a (also Aug 2020, also in stable), so the same signature
is used everywhere.
- `minix_blocks_needed()` has existed since 2011 (commit 016e8d44bc06d).
- The mount API conversion (7cd7bfe593287, Mar 2024) may mean older
stable trees (5.15, 6.1, 6.6) need minor context adjustments, but the
core changes to `minix_check_superblock()` apply cleanly regardless.
- **No other commits are needed** — this is fully self-contained.
### 8. CONCLUSION
This commit fixes a **long-standing syzbot-reported UBSAN bug** (1521+
days old) in the minix filesystem's superblock validation. The bug is
triggered by crafted filesystem images and causes undefined behavior
(shift-out-of-bounds) that is reproducible with a C reproducer. The fix
is small (29 additions, 21 deletions in 1 file), self-contained,
obviously correct, reviewed by an experienced FS developer, and signed
off by the VFS maintainer. The affected code exists in all stable trees,
and syzbot confirms the bug is present and unfixed in at least
linux-5.15 and linux-6.1. The risk of regression is minimal since the
changes only reject invalid filesystem images at mount time. This is a
textbook stable backport candidate.
**YES**
fs/minix/inode.c | 50 ++++++++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index 51ea9bdc813f7..c8c6b2135abe7 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -170,10 +170,38 @@ static int minix_reconfigure(struct fs_context *fc)
static bool minix_check_superblock(struct super_block *sb)
{
struct minix_sb_info *sbi = minix_sb(sb);
+ unsigned long block;
- if (sbi->s_imap_blocks == 0 || sbi->s_zmap_blocks == 0)
+ if (sbi->s_log_zone_size != 0) {
+ printk("minix-fs error: zone size must equal block size. "
+ "s_log_zone_size > 0 is not supported.\n");
+ return false;
+ }
+
+ if (sbi->s_ninodes < 1 || sbi->s_firstdatazone <= 4 ||
+ sbi->s_firstdatazone >= sbi->s_nzones)
return false;
+ /* Apparently minix can create filesystems that allocate more blocks for
+ * the bitmaps than needed. We simply ignore that, but verify it didn't
+ * create one with not enough blocks and bail out if so.
+ */
+ block = minix_blocks_needed(sbi->s_ninodes, sb->s_blocksize);
+ if (sbi->s_imap_blocks < block) {
+ printk("MINIX-fs: file system does not have enough "
+ "imap blocks allocated. Refusing to mount.\n");
+ return false;
+ }
+
+ block = minix_blocks_needed(
+ (sbi->s_nzones - sbi->s_firstdatazone + 1),
+ sb->s_blocksize);
+ if (sbi->s_zmap_blocks < block) {
+ printk("MINIX-fs: file system does not have enough "
+ "zmap blocks allocated. Refusing to mount.\n");
+ return false;
+ }
+
/*
* s_max_size must not exceed the block mapping limitation. This check
* is only needed for V1 filesystems, since V2/V3 support an extra level
@@ -293,26 +321,6 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
minix_set_bit(0,sbi->s_imap[0]->b_data);
minix_set_bit(0,sbi->s_zmap[0]->b_data);
- /* Apparently minix can create filesystems that allocate more blocks for
- * the bitmaps than needed. We simply ignore that, but verify it didn't
- * create one with not enough blocks and bail out if so.
- */
- block = minix_blocks_needed(sbi->s_ninodes, s->s_blocksize);
- if (sbi->s_imap_blocks < block) {
- printk("MINIX-fs: file system does not have enough "
- "imap blocks allocated. Refusing to mount.\n");
- goto out_no_bitmap;
- }
-
- block = minix_blocks_needed(
- (sbi->s_nzones - sbi->s_firstdatazone + 1),
- s->s_blocksize);
- if (sbi->s_zmap_blocks < block) {
- printk("MINIX-fs: file system does not have enough "
- "zmap blocks allocated. Refusing to mount.\n");
- goto out_no_bitmap;
- }
-
/* set up enough so that it can read an inode */
s->s_op = &minix_sops;
s->s_time_min = 0;
--
2.51.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
` (22 preceding siblings ...)
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] minix: Add required sanity checking to minix_check_superblock() Sasha Levin
@ 2026-02-11 7:56 ` Adrian Hunter
23 siblings, 0 replies; 27+ messages in thread
From: Adrian Hunter @ 2026-02-11 7:56 UTC (permalink / raw)
To: Sasha Levin, patches, stable; +Cc: Frank Li, Alexandre Belloni
On 11/02/2026 01:30, Sasha Levin wrote:
> From: Adrian Hunter <adrian.hunter@intel.com>
>
> [ Upstream commit 78f63ae4a82db173f93adca462e63d11ba06b126 ]
>
> The MIPI I3C HCI specification does not define reset values for
> RING_OPERATION1 fields, and some controllers (e.g., Intel) do not clear
> them during a software reset. Ensure the ring pointers are explicitly
> set to zero during bus initialization to avoid inconsistent state.
This patch was part of a larger patch set to enable support for
Runtime PM. There is also no support for System Suspend at this
point.
Without Runtime PM, initialization is done once only, and at that
point RING_OPERATION1 has been initialized.
Also, in my opinion, someone serious about I3C usage would want
power management support, and would therefore back port the
entire patch set (and the subsequent System PM patch set) to their
kernel.
Consequently, I doubt back porting this patch separately helps
anyone.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> Link: https://patch.msgid.link/20260113072702.16268-2-adrian.hunter@intel.com
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>
> LLM Generated explanations, may be completely bogus:
>
> `RING_OPERATION1` has been defined and used since the very first version
> of the driver. The fix is self-contained and has no dependencies.
>
> ## Detailed Analysis
>
> ### 1. COMMIT MESSAGE ANALYSIS
>
> The commit message is clear and precise: the MIPI I3C HCI specification
> does not define reset values for `RING_OPERATION1` fields, and some
> controllers (specifically Intel) do not clear these fields during
> software reset. The fix ensures the ring pointers are set to zero during
> initialization.
>
> - Author: **Adrian Hunter** (Intel) - one of the most experienced kernel
> contributors, working directly on the hardware
> - Reviewed-by: **Frank Li** (NXP) - another vendor confirmed the fix is
> correct
> - Accepted by: **Alexandre Belloni** - subsystem maintainer
>
> ### 2. CODE CHANGE ANALYSIS
>
> The change is a single hardware register write
> (`rh_reg_write(RING_OPERATION1, 0)`) inserted at the `ring_ready:`
> label, before the ring is enabled via `RING_CONTROL`.
>
> **The register `RING_OPERATION1`** (offset 0x28) contains three critical
> ring buffer pointer fields:
> - `RING_OP1_CR_ENQ_PTR` (bits 7:0) — Command/Response enqueue pointer
> - `RING_OP1_CR_SW_DEQ_PTR` (bits 15:8) — Command/Response software
> dequeue pointer
> - `RING_OP1_IBI_DEQ_PTR` (bits 23:16) — IBI dequeue pointer
>
> **The bug mechanism**: The `hci_rh_data` structure is allocated with
> `kzalloc`, so all software-side pointers (`done_ptr`, `ibi_chunk_ptr`)
> start at 0. But if the hardware RING_OPERATION1 register retains stale
> nonzero values (because the spec doesn't mandate reset values, and Intel
> controllers don't clear them), there's a **hardware/software pointer
> mismatch** from the moment the ring is enabled.
>
> **Impact of the mismatch** — critical operations that rely on consistent
> pointers:
>
> 1. **`hci_dma_queue_xfer()`** (line 382-383): Reads
> `RING_OP1_CR_ENQ_PTR` to determine where to place the next command. A
> stale nonzero value means commands go to wrong ring offsets, while
> responses are read from offset 0 (`done_ptr = 0`).
>
> 2. **`hci_dma_xfer_done()`** (line 552-555): Updates
> `RING_OP1_CR_SW_DEQ_PTR`. Stale values in other fields are masked
> out, but the initial read from `hci_dma_queue_xfer` would already
> have gone wrong.
>
> 3. **`hci_dma_process_ibi()`** (line 614-615): Reads
> `RING_OP1_IBI_DEQ_PTR`. A stale nonzero value means IBI processing
> starts from the wrong position in the status ring.
>
> **Consequences**: This can cause complete DMA ring malfunction on
> affected controllers:
> - Commands placed at incorrect ring entry positions
> - Response processing reading uninitialized entries
> - IBI processing starting at wrong positions
> - Potential DMA data corruption (ring_data = `rh->xfer +
> rh->xfer_struct_sz * stale_ptr`)
>
> ### 3. CLASSIFICATION
>
> This is a **hardware initialization bug fix**. It falls into the
> "hardware quirk/workaround" category — the MIPI spec has an ambiguity
> (no defined reset values), and Intel hardware takes advantage of that
> ambiguity in a way the driver didn't account for.
>
> ### 4. SCOPE AND RISK ASSESSMENT
>
> - **Lines changed**: 1 line of code + 6 lines of comment = 8 lines
> total, 1 file
> - **Risk**: Extremely low. Writing 0 to ring pointers before enabling
> the ring is always correct during initialization (software pointers
> are zero, hardware pointers should match). This cannot break any
> working configuration.
> - **Placement**: The write is at `ring_ready:` label, which is reached
> for all rings (both IBI and non-IBI), and occurs just before the ring
> is enabled with `RING_CTRL_ENABLE | RING_CTRL_RUN_STOP`. This is the
> correct place — after setup, before enable.
>
> ### 5. USER IMPACT
>
> This affects anyone using the MIPI I3C HCI driver on Intel controllers.
> I3C is increasingly used in modern platforms for sensor hubs, power
> management ICs, and other peripherals. Without this fix, the controller
> can enter inconsistent state leading to I3C bus communication failures.
>
> ### 6. STABILITY INDICATORS
>
> - Reviewed-by from NXP vendor (Frank Li)
> - Author is from Intel and knows the hardware intimately
> - Accepted by subsystem maintainer
>
> ### 7. DEPENDENCY CHECK
>
> - The `ring_ready:` label and `RING_OPERATION1` register definition
> exist in all versions of this driver since its introduction in v5.11
> - The `rh_reg_write` macro has been unchanged since the original driver
> - **No dependencies** on other commits — this is fully self-contained
> - The only context difference across stable trees is whether
> `RING_CONTROL` is written with `RING_CTRL_ENABLE` alone (pre-v6.6) or
> `RING_CTRL_ENABLE | RING_CTRL_RUN_STOP` (v6.7+), but the new
> `RING_OPERATION1` write goes before it and is independent
> - Minor backport adjustment may be needed for trivial context
> differences in the surrounding `ring_ready:` label area, but the patch
> should apply cleanly or with minimal fuzz
>
> ### Summary
>
> This is a small, surgical, obviously correct hardware initialization
> fix. It prevents inconsistent DMA ring pointer state on Intel I3C HCI
> controllers where the hardware doesn't reset `RING_OPERATION1` during
> software reset. The mismatch between hardware pointer values
> (stale/random) and software pointer values (zero from kzalloc) can cause
> command/response ring processing failures, IBI processing from wrong
> positions, and potential DMA corruption. The fix is one register write,
> has zero risk of regression, requires no dependencies, and applies to
> all stable trees back to v5.15 where this driver exists.
>
> **YES**
>
> drivers/i3c/master/mipi-i3c-hci/dma.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index c401a9425cdc5..951abfea5a6fd 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -342,6 +342,14 @@ static int hci_dma_init(struct i3c_hci *hci)
> rh_reg_write(INTR_SIGNAL_ENABLE, regval);
>
> ring_ready:
> + /*
> + * The MIPI I3C HCI specification does not document reset values for
> + * RING_OPERATION1 fields and some controllers (e.g. Intel controllers)
> + * do not reset the values, so ensure the ring pointers are set to zero
> + * here.
> + */
> + rh_reg_write(RING_OPERATION1, 0);
> +
> rh_reg_write(RING_CONTROL, RING_CTRL_ENABLE |
> RING_CTRL_RUN_STOP);
> }
^ permalink raw reply [flat|nested] 27+ messages in thread