* [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; 3+ 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] 3+ 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; 3+ 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] 3+ messages in thread
[parent not found: <20260511221931.2370053-1-sashal@kernel.org>]
* [PATCH AUTOSEL 7.0-6.1] smb: client: Zero-pad short GSS session keys per MS-SMB2 [not found] <20260511221931.2370053-1-sashal@kernel.org> @ 2026-05-11 22:19 ` Sasha Levin 0 siblings, 0 replies; 3+ messages in thread From: Sasha Levin @ 2026-05-11 22:19 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] 3+ messages in thread
end of thread, other threads:[~2026-05-20 11:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[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 ` [PATCH AUTOSEL 7.0] smb: client: avoid integer overflow in SMB2 READ length check Sasha Levin
[not found] <20260511221931.2370053-1-sashal@kernel.org>
2026-05-11 22:19 ` [PATCH AUTOSEL 7.0-6.1] smb: client: Zero-pad short GSS session keys per MS-SMB2 Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox