* [PATCH AUTOSEL 7.0-6.1] smb: client: Zero-pad short GSS session keys per MS-SMB2
[not found] <20260520111944.3424570-1-sashal@kernel.org>
@ 2026-05-20 11:18 ` Sasha Levin
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0] smb: client: avoid integer overflow in SMB2 READ length check Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-05-20 11:18 UTC (permalink / raw)
To: patches, stable
Cc: Piyush Sachdeva, Piyush Sachdeva, Steve French, Sasha Levin,
sfrench, linux-cifs, samba-technical, linux-kernel
From: Piyush Sachdeva <s.piyush1024@gmail.com>
[ Upstream commit 8cb6fc3231500233ddaf63cb7fd5435008d9ed5f ]
Per MS-SMB2 section 3.2.5.3, Session.SessionKey is the first 16 bytes
of the GSS cryptographic key, right-padded with zero bytes if the key
is shorter than 16 bytes.
SMB2_auth_kerberos() copies the GSS session key from the cifs.upcall
response using kmemdup(msg->data, msg->sesskey_len, ...) and stores
the GSS-reported length verbatim in ses->auth_key.len. generate_key()
reads SMB2_NTLMV2_SESSKEY_SIZE bytes from this buffer when feeding the
HMAC-SHA256 KDF for signing key derivation. If a GSS mechanism returns
a session key shorter than 16 bytes (e.g. a deprecated single-DES
Kerberos enctype with an 8-byte session key), the KDF call performs an
out-of-bounds slab read and derives keys that do not match the server,
which pads per the spec.
Modern KDCs disable short-key enctypes by default, so this is latent
rather than reachable in production, but it is still a kernel heap
over-read.
Allocate auth_key.response with kzalloc() at a length of
max(msg->sesskey_len, SMB2_NTLMV2_SESSKEY_SIZE), copy the GSS key in,
and rely on kzalloc()'s zero initialization for the spec-mandated
padding. Set ses->auth_key.len to the padded length. Larger GSS keys
(e.g. the 32-byte aes256-cts-hmac-sha1-96 session key) continue to be
stored at their natural length, preserving the FullSessionKey path.
Emit a cifs_dbg(VFS, ...) message when a short key is encountered to
surface deprecated-enctype usage.
NTLMv2 and NTLMSSP code paths produce a 16-byte session key by
construction and are unaffected.
Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com>
Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
### Phase 1: Commit Message Forensics
Record 1.1: subsystem `smb: client`; action verb `Zero-pad`; intent is
to make short GSS session keys comply with MS-SMB2 and avoid short-
buffer use in SMB2 key derivation.
Record 1.2: tags present are:
- `Signed-off-by: Piyush Sachdeva <psachdeva@microsoft.com>`
- `Signed-off-by: Piyush Sachdeva <s.piyush1024@gmail.com>`
- `Signed-off-by: Steve French <stfrench@microsoft.com>`
No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`,
`Link:`, or `Cc: stable@vger.kernel.org` tag was present in the supplied
commit message.
Record 1.3: the commit describes a real memory-safety bug:
`SMB2_auth_kerberos()` allocates exactly `msg->sesskey_len` bytes, but
SMB2 key derivation later consumes `SMB2_NTLMV2_SESSKEY_SIZE` bytes,
which is `16`. If `msg->sesskey_len < 16`, the kernel reads past the
allocated buffer and also derives a non-spec-compliant key. MS-SMB2 says
`Session.SessionKey` is the first 16 bytes of the cryptographic key,
right-padded with zeros if shorter.
Record 1.4: this is not just cleanup. It is a hidden memory-safety and
protocol-correctness fix: short allocation plus fixed 16-byte HMAC key
input creates an out-of-bounds read.
### Phase 2: Diff Analysis
Record 2.1: one file changed: `fs/smb/client/smb2pdu.c`, one hunk in
`SMB2_auth_kerberos()`, roughly 18 insertions and 5 deletions from the
supplied diff. Scope is a single-file surgical fix.
Record 2.2: before, `ses->auth_key.response = kmemdup(msg->data,
msg->sesskey_len, GFP_KERNEL)` and `ses->auth_key.len =
msg->sesskey_len`. After, the code sets `ses->auth_key.len =
max(msg->sesskey_len, 16)`, allocates a zeroed buffer of that size,
copies only the original GSS key, and leaves zero padding if the key was
short.
Record 2.3: bug category is memory safety / out-of-bounds read plus
protocol correctness. The verified consumer is `generate_key()` in
`fs/smb/client/smb2transport.c`, which calls
`hmac_sha256_init_usingrawkey(&hmac_ctx, ses->auth_key.response,
SMB2_NTLMV2_SESSKEY_SIZE)`. The HMAC helper copies `raw_key_len` bytes
from the pointer, so a shorter allocation is a real over-read.
Record 2.4: fix quality is good: it preserves larger keys, fixes only
Kerberos/GSS key storage, does not change the security blob pointer, and
uses zeroed allocation to implement the spec padding. Regression risk is
low; the only behavior change for short keys is from invalid over-
read/wrong key material to zero-padded spec behavior. It adds one VFS
debug message for short keys.
### Phase 3: Git History Investigation
Record 3.1: `git blame` on current `fs/smb/client/smb2pdu.c` shows the
current `kmemdup(msg->data, msg->sesskey_len)` lines came from
`d9d1e319b39e` (`smb: client: fix broken multichannel with
krb5+signing`), first contained in local tags starting at `v7.0`. The
fixed-size 16-byte HMAC use in current `generate_key()` came from
`4b4c6fdb25de`, first contained in local tags starting at `v6.18`.
Record 3.2: no `Fixes:` tag is present, so there was no tagged
introducing commit to follow.
Record 3.3: recent local history for `smb2pdu.c` includes related
Kerberos/multichannel work (`d9d1e319b39e`). Recent `smb2transport.c`
history includes crypto-library conversion commits. The candidate itself
is standalone for current `v7.0.y` context.
Record 3.4: local `git log master --author='Piyush Sachdeva' -10 --
fs/smb/client fs/cifs` found no matching prior local commits. `Steve
French` is listed in `MAINTAINERS` as maintainer for `COMMON INTERNET
FILE SYSTEM CLIENT (CIFS and SMB3)`, and he signed off this patch.
Record 3.5: no functional dependency was found for the current `v7.0.y`
file: `git apply --check` of the supplied patch against this checkout
succeeded. Older stable trees need path/context adjustments because
older releases use `fs/cifs/` and some have the allocation inside an `if
(!is_binding)` / `if (!ses->binding)` block.
### Phase 4: Mailing List and External Research
Record 4.1: no candidate commit hash was available locally. `git log
master`, `pending-7.0`, and `for-greg/7.0-200` with the exact subject
found no commit. `b4 dig -c '<subject>'`, `b4 dig -a`, and `b4 dig -w`
failed because `b4 dig` needs a commitish.
Record 4.2: reviewer/recipient data could not be obtained from `b4 dig`
for this candidate. The only maintainer signal verified is Steve
French’s signoff and MAINTAINERS entry.
Record 4.3: no `Link:` or `Reported-by:` tags exist. Lore web queries
were blocked by Anubis, so no bug-report thread was verified.
Record 4.4: no series context was verified. No local subject match was
found in searched branches.
Record 4.5: stable-list search via lore was blocked by Anubis; no
stable-specific discussion was verified.
### Phase 5: Code Semantic Analysis
Record 5.1: modified function: `SMB2_auth_kerberos()`.
Record 5.2: caller path verified locally: `connect.c` calls
`server->ops->sess_setup()`, SMB2/SMB3 ops point that to
`SMB2_sess_setup()`, `SMB2_select_sec()` selects `SMB2_auth_kerberos()`
for Kerberos, and `SMB2_sess_setup()` runs the selected auth function.
Record 5.3: key callees are `cifs_get_spnego_key()`,
`SMB2_sess_sendreceive()`, and `SMB2_sess_establish_session()`.
`SMB2_sess_establish_session()` calls
`server->ops->generate_signingkey()`, which reaches
`generate_smb3signingkey()` and then `generate_key()` for SMB3 dialects.
Record 5.4: reachability is from a user-requested CIFS/SMB mount using
Kerberos, with `CONFIG_CIFS_UPCALL` enabled. `Kconfig` says
`CIFS_UPCALL` enables Kerberos/SPNEGO advanced session setup and is used
for Kerberos tickets needed to mount secure servers.
Record 5.5: similar short allocation plus 16-byte consumer patterns
exist across stable tags. I verified the same `kmemdup(msg->data,
msg->sesskey_len)` and 16-byte key use in `v5.4`, `v5.10`, `v5.15`,
`v6.1`, `v6.6`, `v6.12`, `v6.18`, `v6.19`, and `v7.0`, with path changes
from `fs/cifs/` to `fs/smb/client/`.
### Phase 6: Cross-Referencing and Stable Tree Analysis
Record 6.1: buggy code exists in multiple stable-era tags. `v5.4`
through `v6.1` use `fs/cifs/`; `v6.6+` uses `fs/smb/client/`. The
relevant allocation and 16-byte key consumption pattern is present in
those checked tags.
Record 6.2: expected backport difficulty is clean for current `v7.0.y`
because `git apply --check` succeeded. Older stable trees need minor
backporting for path and context differences; pre-`d9d1e319b39e` trees
should preserve their existing binding conditional unless that
multichannel Kerberos fix is also backported.
Record 6.3: local searches for the exact subject, `short GSS session
key`, and related `MS-SMB2` terms did not find an existing local fix.
### Phase 7: Subsystem and Maintainer Context
Record 7.1: subsystem is CIFS/SMB3 client filesystem/network filesystem
code. Criticality is important: it affects users mounting SMB shares
with Kerberos/SPNEGO.
Record 7.2: subsystem is active; recent `master` history under
`fs/smb/client` includes multiple SMB client fixes and refactors. The
maintained subsystem is marked `Supported` in `MAINTAINERS`.
### Phase 8: Impact and Risk Assessment
Record 8.1: affected population is config- and workload-specific:
CIFS/SMB2+ Kerberos users with `CONFIG_CIFS_UPCALL`, especially where
the GSS session key returned to the kernel is shorter than 16 bytes.
Record 8.2: trigger condition is a Kerberos/SPNEGO session setup where
`msg->sesskey_len < SMB2_NTLMV2_SESSKEY_SIZE`. The mount path is user
reachable, but whether unprivileged users can trigger it depends on
mount permissions and local configuration, which I did not verify.
Record 8.3: failure mode is a kernel heap out-of-bounds read during HMAC
key setup and incorrect SMB key derivation. Severity is high for memory
safety, though likely low-frequency because it requires a short GSS
session key.
Record 8.4: benefit is high for affected users because it removes a
kernel heap over-read and implements the protocol-required padding. Risk
is low because the patch is small, local, and preserves behavior for
keys of length 16 or greater.
### Phase 9: Final Synthesis
Record 9.1: evidence for backporting:
- Verified short allocation in `SMB2_auth_kerberos()`.
- Verified fixed 16-byte read in `generate_key()`.
- Verified HMAC helper copies the specified key length.
- Verified MS-SMB2 requires right-zero-padding short session keys.
- Verified bug pattern exists across multiple stable-era tags.
- Fix is small, local, and cleanly applies to current `v7.0.y`.
Evidence against / concerns:
- No `Reported-by`, `Tested-by`, `Reviewed-by`, `Fixes`, or `Link` tag
was supplied.
- No lore review discussion could be verified.
- Older stable trees need minor path/context backport adjustments.
- Trigger appears specific to short GSS keys; I did not independently
verify real-world frequency.
Unresolved:
- No upstream commit hash was available locally, so `b4 dig` could not
retrieve the original thread.
- Lore web searches were blocked.
- I did not compile-test the patch.
Record 9.2 stable rules:
1. Obviously correct and tested: obviously correct by code inspection;
external testing not verified.
2. Fixes a real bug: yes, verified out-of-bounds read when `sesskey_len
< 16`.
3. Important issue: yes, kernel heap over-read and broken key
derivation.
4. Small and contained: yes, one function in one file.
5. No new feature/API: yes, no new API or userspace interface.
6. Can apply to stable: yes for current `v7.0.y`; minor backport work
needed for older trees.
Record 9.3: no automatic exception category applies. This is not a
device ID, quirk, DT, build, or documentation-only change.
Record 9.4: decision is to backport. The memory-safety fix is small and
technically well justified; the limited trigger scope reduces urgency
but not suitability.
## Verification
- [Phase 1] Parsed the supplied subject and tags; no `Fixes`, `Reported-
by`, `Tested-by`, `Reviewed-by`, `Link`, or stable Cc was present.
- [Phase 2] Read `SMB2_auth_kerberos()` in `fs/smb/client/smb2pdu.c`;
verified current exact-size `kmemdup()` allocation and
`ses->auth_key.len = msg->sesskey_len`.
- [Phase 2] Read `generate_key()` in `fs/smb/client/smb2transport.c`;
verified it passes `SMB2_NTLMV2_SESSKEY_SIZE` bytes from
`ses->auth_key.response`.
- [Phase 2] Read `lib/crypto/sha256.c`; verified HMAC raw-key setup
copies `raw_key_len` bytes from the supplied pointer.
- [Phase 2] Verified `SMB2_NTLMV2_SESSKEY_SIZE` is `16` in
`fs/smb/common/smb2pdu.h`.
- [Phase 3] `git blame` identified local current-line history for the
allocation and HMAC key setup.
- [Phase 3] `git show` inspected `d9d1e319b39e` and `4b4c6fdb25de`.
- [Phase 3] `git tag --contains` showed `d9d1e319b39e` starts at local
`v7.0` tags and `4b4c6fdb25de` at local `v6.18` tags.
- [Phase 4] `b4 dig` attempts by subject failed because no local
commitish exists; lore WebFetch searches were blocked by Anubis.
- [Phase 5] `rg` and `ReadFile` traced `connect.c -> SMB2_sess_setup()
-> SMB2_auth_kerberos() -> SMB2_sess_establish_session() ->
generate_key()`.
- [Phase 6] Checked `v5.4`, `v5.10`, `v5.15`, `v6.1`, `v6.6`, `v6.12`,
`v6.18`, `v6.19`, and `v7.0` for the allocation and 16-byte consumer
pattern.
- [Phase 6] `git apply --check` confirmed the supplied patch applies
cleanly to current `v7.0.y`.
- [Phase 7] Verified CIFS/SMB3 client maintainership and supported
status in `MAINTAINERS`.
- [Phase 8] Verified `CONFIG_CIFS_UPCALL` Kconfig text describes
Kerberos/SPNEGO upcall support for secure SMB mounts.
- UNVERIFIED: original mailing-list review, patch revisions, explicit
stable discussion, compile-test results, and real-world frequency of
short GSS session keys.
**YES**
fs/smb/client/smb2pdu.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 5188218c25be4..0792e0c38b44f 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -1714,17 +1714,30 @@ SMB2_auth_kerberos(struct SMB2_sess_data *sess_data)
is_binding = (ses->ses_status == SES_GOOD);
spin_unlock(&ses->ses_lock);
+ /*
+ * Per MS-SMB2 3.2.5.3, Session.SessionKey is the first 16 bytes of the
+ * GSS cryptographic key, right-padded with zero bytes if shorter.
+ * Allocate at least SMB2_NTLMV2_SESSKEY_SIZE bytes (zeroed) so the KDF
+ * input buffer is always valid for HMAC-SHA256 even with deprecated
+ * Kerberos enctypes that return a short session key.
+ */
+ if (unlikely(msg->sesskey_len < SMB2_NTLMV2_SESSKEY_SIZE))
+ cifs_dbg(VFS,
+ "short GSS session key (%u bytes); zero-padding per MS-SMB2 3.2.5.3\n",
+ msg->sesskey_len);
+
kfree_sensitive(ses->auth_key.response);
- ses->auth_key.response = kmemdup(msg->data,
- msg->sesskey_len,
- GFP_KERNEL);
+ ses->auth_key.len = max_t(unsigned int, msg->sesskey_len,
+ SMB2_NTLMV2_SESSKEY_SIZE);
+ ses->auth_key.response = kzalloc(ses->auth_key.len, GFP_KERNEL);
if (!ses->auth_key.response) {
cifs_dbg(VFS, "%s: can't allocate (%u bytes) memory\n",
- __func__, msg->sesskey_len);
+ __func__, ses->auth_key.len);
+ ses->auth_key.len = 0;
rc = -ENOMEM;
goto out_put_spnego_key;
}
- ses->auth_key.len = msg->sesskey_len;
+ memcpy(ses->auth_key.response, msg->data, msg->sesskey_len);
sess_data->iov[1].iov_base = msg->data + msg->sesskey_len;
sess_data->iov[1].iov_len = msg->secblob_len;
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* [PATCH AUTOSEL 7.0] smb: client: avoid integer overflow in SMB2 READ length check
[not found] <20260520111944.3424570-1-sashal@kernel.org>
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0-6.1] smb: client: Zero-pad short GSS session keys per MS-SMB2 Sasha Levin
@ 2026-05-20 11:18 ` Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-05-20 11:18 UTC (permalink / raw)
To: patches, stable
Cc: Jeremy Erazo, Steve French, Sasha Levin, sfrench, linux-cifs,
samba-technical, linux-kernel
From: Jeremy Erazo <mendozayt13@gmail.com>
[ Upstream commit 81a874233c305d29e37fdb70b691ff4254294c0b ]
SMB2 READ response validation in cifs_readv_receive() and
handle_read_data() checks data_offset + data_len against the received
buffer length. Both values are attacker-controlled fields from the
server response and are stored as unsigned int, so the addition can
wrap before the bounds check:
fs/smb/client/transport.c:1259
if (!use_rdma_mr && (data_offset + data_len > buflen))
fs/smb/client/smb2ops.c:4839
else if (buf_len >= data_offset + data_len)
A malicious SMB server can use this to bypass validation. In the
non-encrypted receive path the client attempts an oversized socket
read and stalls for the SMB response timeout (180 seconds) before
reconnecting. In the SMB3 encrypted path, runtime testing shows the
malformed length can reach copy_to_iter() in handle_read_data() with
attacker-controlled size, where usercopy hardening stops the oversized
copy before bytes reach userspace.
Guard both call sites with check_add_overflow(), which is already
used elsewhere in this subsystem (smb2pdu.c). On overflow, treat the
response as malformed and reject with -EIO.
Signed-off-by: Jeremy Erazo <mendozayt13@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
### Phase 1: Commit Message Forensics
Record: Subsystem is `smb: client`; action verb is `avoid`; claimed
intent is preventing integer overflow in SMB2 READ response length
validation.
Record: Tags present are `Signed-off-by: Jeremy Erazo
<mendozayt13@gmail.com>` and `Signed-off-by: Steve French
<stfrench@microsoft.com>`. No `Fixes:`, `Reported-by:`, `Tested-by:`,
`Reviewed-by:`, `Acked-by:`, `Link:`, or `Cc: stable@vger.kernel.org`
tags were present.
Record: The body describes attacker-controlled SMB2 READ response fields
`DataOffset` and `DataLength` being added as `unsigned int`, allowing
wraparound before bounds checks. The stated symptoms are a 180-second
client stall/reconnect in the non-encrypted path and an oversized
`copy_to_iter()` attempt in the encrypted path, stopped by usercopy
hardening.
Record: This is a direct bug fix, not hidden cleanup: it changes
overflow-prone bounds checks into checked arithmetic and rejects
malformed responses with `-EIO`.
### Phase 2: Diff Analysis
Record: Files changed: `fs/smb/client/smb2ops.c` and
`fs/smb/client/transport.c`; upstream stat is 12 insertions and 7
deletions. Modified functions are `handle_read_data()` and
`cifs_readv_receive()`. Scope is small and surgical.
Record: In `handle_read_data()`, before the patch `buf_len >=
data_offset + data_len` could pass after unsigned wrap. After the patch,
`check_add_overflow(data_offset, data_len, &end_off)` must be false and
`buf_len >= end_off` must be true before copying from `buf +
data_offset`.
Record: In `cifs_readv_receive()`, before the patch `data_offset +
data_len > buflen` could wrap and fail to reject malformed lengths.
After the patch, overflow or `end_off > buflen` marks the response
malformed and discards the frame.
Record: Bug category is integer overflow leading to bounds-check bypass.
The fix quality is good: it uses the kernel overflow helper already used
in this SMB client area, changes no ABI, and affects only malformed
server responses. Regression risk is low.
### Phase 3: Git History Investigation
Record: Upstream commit found on `origin/master` as
`81a874233c305d29e37fdb70b691ff4254294c0b`, merged by `b0662be9131d8` in
“Pull smb client fixes from Steve French”, explicitly listing “Fix
integer overflow in read”.
Record: `git blame` shows the current `handle_read_data()` vulnerable
expression attributed to `7c00c3a625f8`, but `git grep` confirmed the
same expression exists as far back as `v4.14`, so the bug predates that
current-line attribution. The `cifs_readv_receive()` vulnerable
expression is attributed to `fb157ed226d2`, described by `git describe
--contains` as `v6.0-rc1~75^2~4`.
Record: No `Fixes:` tag exists, so Step 3.2 is not applicable.
Record: Recent file history contains other SMB client fixes, including
OOB and data-corruption fixes, but no prerequisite for this commit was
identified. The commit is standalone.
Record: Jeremy Erazo had no prior SMB client commits in the checked
history. Steve French has many SMB/CIFS commits and is the SMB client
maintainer who committed this patch.
### Phase 4: Mailing List And External Research
Record: `b4 dig -c 81a874233c305d29e37fdb70b691ff4254294c0b` found the
original submission at `https://patch.msgid.link/20260514120334.2925013-
1-mendozayt13@gmail.com`.
Record: `b4 dig -a` found only v1 of the patch; no later revisions were
found.
Record: `b4 dig -w` showed Jeremy Erazo, Steve French, `linux-
cifs@vger.kernel.org`, `samba-technical@lists.samba.org`, and `linux-
kernel@vger.kernel.org` were included.
Record: Saved mbox contained the patch only; no review replies, NAKs,
stable nominations, or objections were present in that matched thread.
WebFetch to lore search pages was blocked by Anubis, so stable-list
search via web could not be independently verified.
### Phase 5: Code Semantic Analysis
Record: Modified functions are `handle_read_data()` and
`cifs_readv_receive()`.
Record: Callers: `smb2_async_readv()` passes `cifs_readv_receive` and
`smb3_handle_read_data` to `cifs_call_async()`. `cifs_call_async()`
stores them in the MID entry. The receive loop in `connect.c` invokes
`mids[0]->receive()` for non-encrypted async reads, while encrypted
large reads call `receive_encrypted_read()` and then
`handle_read_data()`.
Record: User reachability is verified through normal file reads:
`cifs_issue_read()` calls the dialect `async_readv()` operation, and
SMB2/SMB3 operation tables use `smb2_async_readv()`.
Record: Key callees are `server->ops->read_data_offset()`,
`server->ops->read_data_length()`, `cifs_read_iter_from_socket()`,
`cifs_readv_discard()`, and `copy_to_iter()`. `smb2_read_data_offset()`
reads `DataOffset`; `smb2_read_data_length()` reads `DataLength` or
`DataRemaining`.
Record: Similar pattern search found the same vulnerable `data_offset +
data_len` expressions in active stable tags; `check_add_overflow()` is
already used elsewhere in SMB client files.
### Phase 6: Stable Tree Analysis
Record: The vulnerable `handle_read_data()` expression exists in `v7.0`,
`v6.12`, `v6.6`, `v6.1`, `v5.15`, `v5.10`, `v4.19`, and `v4.14`. The
vulnerable `cifs_readv_receive()` expression exists in `v7.0`, `v6.12`,
`v6.6`, and `v6.1`; it was not found in `v5.15`/`v5.10` by the checked
grep.
Record: The patch applies cleanly to the current `7.0` tree with `git
apply --check`.
Record: Backport difficulty should be low for `v7.0`, `v6.12`, and
`v6.6`; `v6.1` and older need path adjustment from `fs/smb/client` to
`fs/cifs`. Older trees may need per-tree adjustment because
`v5.15`/`v5.10` only showed the `smb2ops.c` instance, and `v4.14` did
not have `check_add_overflow()` in `include/linux`.
### Phase 7: Subsystem Context
Record: Subsystem is SMB/CIFS client filesystem/network filesystem code.
Criticality is important: it affects SMB/CIFS mounts and reads from
remote servers.
Record: The subsystem is active; recent history includes SMB client
fixes for OOB reads, data corruption, replay initialization, races, and
read-path issues.
### Phase 8: Impact And Risk
Record: Affected users are SMB2/SMB3 client users mounting shares from a
malicious or broken server.
Record: Trigger condition is a crafted SMB2 READ response with
`DataOffset + DataLength` wrapping `unsigned int`. This is reachable
from normal file read paths over an SMB mount. Whether an unprivileged
local user can trigger it depends on mount/access policy; a malicious
server can trigger it once the client reads from that share.
Record: Failure mode is high severity: denial of service via read
stall/reconnect in the non-encrypted path, and malformed attacker-
controlled copy length reaching `copy_to_iter()` in the encrypted path.
I verified the call path to `copy_to_iter()` and the 3 * default
60-second unresponsive timeout logic; I did not independently reproduce
the runtime usercopy-hardening behavior.
Record: Benefit is high because this prevents a remotely supplied
malformed SMB response from bypassing validation. Risk is low because
the patch only rejects overflowed or out-of-bounds malformed READ
responses and does not alter valid response handling.
### Phase 9: Final Synthesis
Evidence for backporting: real bounds-check bypass from attacker-
controlled network fields; reachable in normal SMB read paths; high-
impact DoS/memory-safety-adjacent behavior; tiny two-function fix;
committed by the SMB maintainer; merged by Linus as an SMB client fix;
vulnerable code exists in multiple stable trees.
Evidence against backporting: no `Fixes:`/`Cc: stable`/review tags;
older stable trees need path/context adjustments, and very old trees may
lack `check_add_overflow()`.
Unresolved: I could not verify stable-list discussion because lore
WebFetch search was blocked, and I did not reproduce the runtime
hardening behavior. These do not change the decision because the integer
overflow and reachable bounds-check bypass are verified from code.
Stable rules checklist: obviously correct and tested enough by
review/merge evidence, yes; fixes a real bug, yes; important issue, yes;
small and contained, yes; no new feature/API, yes; applies cleanly to
7.0 and should be straightforward elsewhere with noted adjustments,
yes/minor backport work.
Verification:
- [Phase 1] Parsed upstream commit
`81a874233c305d29e37fdb70b691ff4254294c0b`; found only Jeremy/Steve
`Signed-off-by` tags.
- [Phase 2] `git show --stat` confirmed 2 files, 12 insertions, 7
deletions.
- [Phase 3] `git blame` and `git describe --contains` checked history
for both changed sites.
- [Phase 4] `b4 dig`, `b4 dig -a`, `b4 dig -w`, and saved mbox verified
lore URL, v1-only series, recipients, and lack of replies.
- [Phase 5] `rg` and source reads verified read call path from
`cifs_issue_read()` to `smb2_async_readv()` to receive handlers.
- [Phase 6] `git grep` checked vulnerable expressions in `v7.0`,
`v6.12`, `v6.6`, `v6.1`, `v5.15`, `v5.10`, `v4.19`, and `v4.14`; `git
apply --check` confirmed clean 7.0 application.
- [Phase 8] Source reads verified socket-read loop, reconnect timeout
basis, and `copy_to_iter()` path.
- UNVERIFIED: independent runtime reproduction of the oversized
copy/usercopy-hardening stop.
- UNVERIFIED: stable mailing-list search beyond `b4` mbox, because lore
WebFetch search pages were blocked.
This should be backported to stable trees.
**YES**
fs/smb/client/smb2ops.c | 4 +++-
fs/smb/client/transport.c | 15 +++++++++------
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index ccc06c83956b5..d443cc8097df6 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4721,6 +4721,7 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
{
unsigned int data_offset;
unsigned int data_len;
+ unsigned int end_off;
unsigned int cur_off;
unsigned int cur_page_idx;
unsigned int pad_len;
@@ -4836,7 +4837,8 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
}
rdata->got_bytes = buffer_len;
- } else if (buf_len >= data_offset + data_len) {
+ } else if (!check_add_overflow(data_offset, data_len, &end_off) &&
+ buf_len >= end_off) {
/* read response payload is in buf */
WARN_ONCE(buffer, "read data can be either in buf or in buffer");
copied = copy_to_iter(buf + data_offset, data_len, &rdata->subreq.io_iter);
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index 05f8099047e1a..fdf4e50c27ceb 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -1158,7 +1158,7 @@ int
cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
{
int length, len;
- unsigned int data_offset, data_len;
+ unsigned int data_offset, data_len, end_off;
struct cifs_io_subrequest *rdata = mid->callback_data;
char *buf = server->smallbuf;
unsigned int buflen = server->pdu_size;
@@ -1256,11 +1256,14 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid)
use_rdma_mr = rdata->mr;
#endif
data_len = server->ops->read_data_length(buf, use_rdma_mr);
- if (!use_rdma_mr && (data_offset + data_len > buflen)) {
- /* data_len is corrupt -- discard frame */
- rdata->result = smb_EIO2(smb_eio_trace_read_rsp_malformed,
- data_offset + data_len, buflen);
- return cifs_readv_discard(server, mid);
+ if (!use_rdma_mr) {
+ if (check_add_overflow(data_offset, data_len, &end_off) ||
+ end_off > buflen) {
+ /* data_len is corrupt -- discard frame */
+ rdata->result = smb_EIO2(smb_eio_trace_read_rsp_malformed,
+ end_off, buflen);
+ return cifs_readv_discard(server, mid);
+ }
}
#ifdef CONFIG_CIFS_SMB_DIRECT
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread