[PATCH AUTOSEL 6.19-6.6] smb: client: prevent races in ->query_interfaces()

Sasha Levin
2026-02-10T23:31:29Z

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