* [PATCH] smb: client: reject a tree connect response whose byte count is too small
@ 2026-07-28 18:06 ` Bryam Vargas
0 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas via B4 Relay @ 2026-07-28 18:06 UTC (permalink / raw)
To: Steve French, Paulo Alcantara
Cc: Shyam Prasad N, samba-technical, Ronnie Sahlberg, Jeff Layton,
linux-cifs, linux-kernel, Tom Talpey, Bharath SM, David Howells
From: Bryam Vargas <hexlabsecurity@proton.me>
CIFSTCon() bounds its strnlen() over the byte area with the server's
ByteCount minus two, which for ByteCount 0 or 1 goes negative as an int
and converts to a huge size_t. The later subtraction then wraps __u16
bytes_left, and that is what bounds cifs_strndup_from_utf16().
pByteArea() scales with the response's WordCount, also server-chosen, so
the walk can start past everything the request itself wrote, of which
cifs_buf_get() clears only the first 67 bytes. KMSAN reports the read of
uninitialised heap, and the bytes reach userspace through
tcon->nativeFileSystem in /proc/fs/cifs/DebugData.
Reject a byte area too small for what the parser consumes. A response
carrying a service string has at least two bytes there, so nothing valid
is rejected, and both subtractions are then safe.
Fixes: cc20c031bb06 ("cifs: convert CIFSTCon to use new unicode helper functions")
Cc: stable@kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Found with KMSAN. KASAN cannot see this one: the read never leaves the allocation.
The server picks both ends of the walk. pByteArea() is buf + 33 + 2*WordCount + 2 and
CIFSTCon() does not check WordCount, so WordCount 128 puts bcc_ptr at 291 -- past the 67 bytes
cifs_buf_get() clears, past the 256 header_assemble() clears, and past everything the request
itself wrote. checkSMB() is satisfied, since clc_len = 33 + 2*128 + 2 + 0 = 291 and the
response is exactly 291 bytes. ByteCount 0 then removes the bound.
Reproducer. A fake SMB1 server that answers negprot, completes SESSION_SETUP_ANDX, and replies
to TREE_CONNECT_ANDX with WordCount 128 and ByteCount 0.
A/B on v7.2-rc1 with KMSAN, the module rebuilt between arms and the previous session torn down
each time:
wct 3, BCC 8, unpatched: 0 reports
wct 128, BCC 0, patched: 0 reports
wct 128, BCC 0, unpatched: 110 reports, 28 of them in cifs_utf16_bytes
BUG: KMSAN: uninit-value in cifs_utf16_bytes+0x37e/0x400 [cifs]
cifs_utf16_bytes+0x37e/0x400 [cifs]
cifs_strndup_from_utf16+0x5c/0x210 [cifs]
CIFSTCon+0x1102/0x1510 [cifs]
cifs_setup_ipc+0x3b9/0xcf0 [cifs]
cifs_get_smb_ses+0x16e7/0x2cf0 [cifs]
cifs_mount_get_session+0x2b2/0x730 [cifs]
dfs_mount_share+0x43c/0x3510 [cifs]
cifs_mount+0xcc/0x1140 [cifs]
__se_sys_fsconfig+0x6e5/0xb10
Uninit was created at:
kmem_cache_alloc_noprof+0x556/0xf40
mempool_alloc_noprof+0x104/0x240
cifs_buf_get+0x45/0xc0 [cifs]
CIFSTCon+0x77/0x1510 [cifs]
cifs_setup_ipc+0x3b9/0xcf0 [cifs]
The uninitialised buffer is allocated by the same CIFSTCon() call that reads it, and the path is
cifs_setup_ipc(), so this fires on the IPC$ tree connect every SMB1 mount performs. A separate
run caught it through smb2_reconnect_server() as well, i.e. with nobody at the keyboard.
A KMSAN positive control ran first on the same kernel and the same out-of-tree build -- a module
branching on an OPTIMIZER_HIDE_VAR'd uninitialised local -- and reported. Without that the two
zeros above wouldn't mean anything, since clang folds a plain undef to a defined value.
DebugData is created with proc_create_single("DebugData", 0, ...), and mode 0 is promoted to
S_IRUGO; ls -l confirms -r--r--r--. open_files, created on the next line, asks for 0400.
690c522fa5a6 ("cifs: use get/put_unaligned functions to access ByteCount") is what made
bytes_left a __u16, so the second subtraction wraps rather than going negative. The strnlen()
underflow predates it.
I did not add a WordCount check. checkSMB() rejects a frame shorter than its calculated size,
so once bytes_left is honest the byte area is always inside what was received.
The field this fills is only informational, and failing the whole tree connect over it is a
choice -- but a response with fewer than two bytes of byte area carries no service string, so
there is nothing left to parse. Happy to make it skip the field and keep the mount if you'd
rather.
---
fs/smb/client/cifssmb.c | 6 ++++++
fs/smb/client/trace.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 40162d5554ea..bb835fdae296 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -615,6 +615,11 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
tcon->tid = smb_buffer_response->Tid;
bcc_ptr = pByteArea(smb_buffer_response);
bytes_left = get_bcc(smb_buffer_response);
+ if (bytes_left < 2) {
+ rc = smb_EIO2(smb_eio_trace_tcon_bcc_too_small,
+ bytes_left, 2);
+ goto out;
+ }
length = strnlen(bcc_ptr, bytes_left - 2);
if (smb_buffer->Flags2 & SMBFLG2_UNICODE)
is_unicode = true;
@@ -670,6 +675,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
reset_cifs_unix_caps(xid, tcon, NULL, NULL);
}
}
+out:
cifs_buf_release(smb_buffer);
return rc;
}
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index 5b21ad3c15fb..94de11d1c490 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -133,6 +133,7 @@
EM(smb_eio_trace_sym_slash, "sym_slash") \
EM(smb_eio_trace_sym_target_len, "sym_target_len") \
EM(smb_eio_trace_symlink_file_size, "symlink_file_size") \
+ EM(smb_eio_trace_tcon_bcc_too_small, "tcon_bcc_too_small") \
EM(smb_eio_trace_tdis_in_reconnect, "tdis_in_reconnect") \
EM(smb_eio_trace_tx_chained_async, "tx_chained_async") \
EM(smb_eio_trace_tx_compress_failed, "tx_compress_failed") \
---
base-commit: 4235cb24ec1e8e96843f3671ba4da2a6ccca2c7b
change-id: 20260728-b4-disp-6b8e68d4-fa25e109c92e
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] smb: client: reject a tree connect response whose byte count is too small
@ 2026-07-28 18:06 ` Bryam Vargas
0 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas @ 2026-07-28 18:06 UTC (permalink / raw)
To: Steve French, Paulo Alcantara
Cc: Shyam Prasad N, samba-technical, Ronnie Sahlberg, Jeff Layton,
linux-cifs, linux-kernel, Tom Talpey, Bharath SM, David Howells
CIFSTCon() bounds its strnlen() over the byte area with the server's
ByteCount minus two, which for ByteCount 0 or 1 goes negative as an int
and converts to a huge size_t. The later subtraction then wraps __u16
bytes_left, and that is what bounds cifs_strndup_from_utf16().
pByteArea() scales with the response's WordCount, also server-chosen, so
the walk can start past everything the request itself wrote, of which
cifs_buf_get() clears only the first 67 bytes. KMSAN reports the read of
uninitialised heap, and the bytes reach userspace through
tcon->nativeFileSystem in /proc/fs/cifs/DebugData.
Reject a byte area too small for what the parser consumes. A response
carrying a service string has at least two bytes there, so nothing valid
is rejected, and both subtractions are then safe.
Fixes: cc20c031bb06 ("cifs: convert CIFSTCon to use new unicode helper functions")
Cc: stable@kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Found with KMSAN. KASAN cannot see this one: the read never leaves the allocation.
The server picks both ends of the walk. pByteArea() is buf + 33 + 2*WordCount + 2 and
CIFSTCon() does not check WordCount, so WordCount 128 puts bcc_ptr at 291 -- past the 67 bytes
cifs_buf_get() clears, past the 256 header_assemble() clears, and past everything the request
itself wrote. checkSMB() is satisfied, since clc_len = 33 + 2*128 + 2 + 0 = 291 and the
response is exactly 291 bytes. ByteCount 0 then removes the bound.
Reproducer. A fake SMB1 server that answers negprot, completes SESSION_SETUP_ANDX, and replies
to TREE_CONNECT_ANDX with WordCount 128 and ByteCount 0.
A/B on v7.2-rc1 with KMSAN, the module rebuilt between arms and the previous session torn down
each time:
wct 3, BCC 8, unpatched: 0 reports
wct 128, BCC 0, patched: 0 reports
wct 128, BCC 0, unpatched: 110 reports, 28 of them in cifs_utf16_bytes
BUG: KMSAN: uninit-value in cifs_utf16_bytes+0x37e/0x400 [cifs]
cifs_utf16_bytes+0x37e/0x400 [cifs]
cifs_strndup_from_utf16+0x5c/0x210 [cifs]
CIFSTCon+0x1102/0x1510 [cifs]
cifs_setup_ipc+0x3b9/0xcf0 [cifs]
cifs_get_smb_ses+0x16e7/0x2cf0 [cifs]
cifs_mount_get_session+0x2b2/0x730 [cifs]
dfs_mount_share+0x43c/0x3510 [cifs]
cifs_mount+0xcc/0x1140 [cifs]
__se_sys_fsconfig+0x6e5/0xb10
Uninit was created at:
kmem_cache_alloc_noprof+0x556/0xf40
mempool_alloc_noprof+0x104/0x240
cifs_buf_get+0x45/0xc0 [cifs]
CIFSTCon+0x77/0x1510 [cifs]
cifs_setup_ipc+0x3b9/0xcf0 [cifs]
The uninitialised buffer is allocated by the same CIFSTCon() call that reads it, and the path is
cifs_setup_ipc(), so this fires on the IPC$ tree connect every SMB1 mount performs. A separate
run caught it through smb2_reconnect_server() as well, i.e. with nobody at the keyboard.
A KMSAN positive control ran first on the same kernel and the same out-of-tree build -- a module
branching on an OPTIMIZER_HIDE_VAR'd uninitialised local -- and reported. Without that the two
zeros above wouldn't mean anything, since clang folds a plain undef to a defined value.
DebugData is created with proc_create_single("DebugData", 0, ...), and mode 0 is promoted to
S_IRUGO; ls -l confirms -r--r--r--. open_files, created on the next line, asks for 0400.
690c522fa5a6 ("cifs: use get/put_unaligned functions to access ByteCount") is what made
bytes_left a __u16, so the second subtraction wraps rather than going negative. The strnlen()
underflow predates it.
I did not add a WordCount check. checkSMB() rejects a frame shorter than its calculated size,
so once bytes_left is honest the byte area is always inside what was received.
The field this fills is only informational, and failing the whole tree connect over it is a
choice -- but a response with fewer than two bytes of byte area carries no service string, so
there is nothing left to parse. Happy to make it skip the field and keep the mount if you'd
rather.
---
fs/smb/client/cifssmb.c | 6 ++++++
fs/smb/client/trace.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 40162d5554ea..bb835fdae296 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -615,6 +615,11 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
tcon->tid = smb_buffer_response->Tid;
bcc_ptr = pByteArea(smb_buffer_response);
bytes_left = get_bcc(smb_buffer_response);
+ if (bytes_left < 2) {
+ rc = smb_EIO2(smb_eio_trace_tcon_bcc_too_small,
+ bytes_left, 2);
+ goto out;
+ }
length = strnlen(bcc_ptr, bytes_left - 2);
if (smb_buffer->Flags2 & SMBFLG2_UNICODE)
is_unicode = true;
@@ -670,6 +675,7 @@ CIFSTCon(const unsigned int xid, struct cifs_ses *ses,
reset_cifs_unix_caps(xid, tcon, NULL, NULL);
}
}
+out:
cifs_buf_release(smb_buffer);
return rc;
}
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index 5b21ad3c15fb..94de11d1c490 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -133,6 +133,7 @@
EM(smb_eio_trace_sym_slash, "sym_slash") \
EM(smb_eio_trace_sym_target_len, "sym_target_len") \
EM(smb_eio_trace_symlink_file_size, "symlink_file_size") \
+ EM(smb_eio_trace_tcon_bcc_too_small, "tcon_bcc_too_small") \
EM(smb_eio_trace_tdis_in_reconnect, "tdis_in_reconnect") \
EM(smb_eio_trace_tx_chained_async, "tx_chained_async") \
EM(smb_eio_trace_tx_compress_failed, "tx_compress_failed") \
---
base-commit: 4235cb24ec1e8e96843f3671ba4da2a6ccca2c7b
change-id: 20260728-b4-disp-6b8e68d4-fa25e109c92e
Best regards,
--
Bryam Vargas <hexlabsecurity@proton.me>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] smb: client: reject a tree connect response whose byte count is too small
2026-07-28 18:06 ` Bryam Vargas
(?)
@ 2026-07-29 1:42 ` kernel test robot
-1 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2026-07-29 1:42 UTC (permalink / raw)
To: Bryam Vargas via B4 Relay, Steve French, Paulo Alcantara
Cc: llvm, oe-kbuild-all, Shyam Prasad N, samba-technical,
Ronnie Sahlberg, Jeff Layton, linux-cifs, linux-kernel,
Tom Talpey, Bharath SM, David Howells
Hi Bryam,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 4235cb24ec1e8e96843f3671ba4da2a6ccca2c7b]
url: https://github.com/intel-lab-lkp/linux/commits/Bryam-Vargas-via-B4-Relay/smb-client-reject-a-tree-connect-response-whose-byte-count-is-too-small/20260729-024634
base: 4235cb24ec1e8e96843f3671ba4da2a6ccca2c7b
patch link: https://lore.kernel.org/r/20260728-b4-disp-6b8e68d4-v1-1-e69277237297%40proton.me
patch subject: [PATCH] smb: client: reject a tree connect response whose byte count is too small
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260729/202607290953.sMPYxQPT-lkp@intel.com/config)
compiler: clang version 22.1.3 (https://github.com/llvm/llvm-project e9846648fd6183ee6d8cbdb4502213fcf902a211)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260729/202607290953.sMPYxQPT-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202607290953.sMPYxQPT-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> fs/smb/client/cifssmb.c:1791:19: warning: implicit conversion from 'int' to 'enum smb_eio_trace' changes value from 128 to -128 [-Wconstant-conversion]
1791 | return smb_EIO(smb_eio_trace_write_too_far);
| ~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/smb/client/cifssmb.c:1982:17: warning: implicit conversion from 'int' to 'enum smb_eio_trace' changes value from 128 to -128 [-Wconstant-conversion]
1982 | rc = smb_EIO(smb_eio_trace_write_too_far);
| ~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/smb/client/cifssmb.c:2074:19: warning: implicit conversion from 'int' to 'enum smb_eio_trace' changes value from 128 to -128 [-Wconstant-conversion]
2074 | return smb_EIO(smb_eio_trace_write_too_far);
| ~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
vim +1791 fs/smb/client/cifssmb.c
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1761
ec637e3ffb6b97 fs/cifs/cifssmb.c Steve French 2005-12-12 1762
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1763 int
6d5786a34d98bf fs/cifs/cifssmb.c Pavel Shilovsky 2012-06-20 1764 CIFSSMBWrite(const unsigned int xid, struct cifs_io_parms *io_parms,
dbbab32574c384 fs/cifs/cifssmb.c Al Viro 2016-09-05 1765 unsigned int *nbytes, const char *buf)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1766 {
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1767 int rc = -EACCES;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1768 WRITE_REQ *pSMB = NULL;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1769 WRITE_RSP *pSMBr = NULL;
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1770 int bytes_returned, wct;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1771 __u32 bytes_sent;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1772 __u16 byte_count;
fa2989f4473413 fs/cifs/cifssmb.c Pavel Shilovsky 2011-05-26 1773 __u32 pid = io_parms->pid;
fa2989f4473413 fs/cifs/cifssmb.c Pavel Shilovsky 2011-05-26 1774 __u16 netfid = io_parms->netfid;
fa2989f4473413 fs/cifs/cifssmb.c Pavel Shilovsky 2011-05-26 1775 __u64 offset = io_parms->offset;
96daf2b09178d8 fs/cifs/cifssmb.c Steve French 2011-05-27 1776 struct cifs_tcon *tcon = io_parms->tcon;
83bfbd0bb9025f fs/smb/client/cifssmb.c David Howells 2025-09-05 1777 unsigned int count = io_parms->length, in_len;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1778
a24e2d7d8f5123 fs/cifs/cifssmb.c Steve French 2010-04-03 1779 *nbytes = 0;
a24e2d7d8f5123 fs/cifs/cifssmb.c Steve French 2010-04-03 1780
f96637be081141 fs/cifs/cifssmb.c Joe Perches 2013-05-04 1781 /* cifs_dbg(FYI, "write at %lld %d bytes\n", offset, count);*/
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1782 if (tcon->ses == NULL)
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1783 return -ECONNABORTED;
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1784
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1785 if (tcon->ses->capabilities & CAP_LARGE_FILES)
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1786 wct = 14;
4c3130efda1ef4 fs/cifs/cifssmb.c Steve French 2008-12-09 1787 else {
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1788 wct = 12;
4c3130efda1ef4 fs/cifs/cifssmb.c Steve French 2008-12-09 1789 if ((offset >> 32) > 0) {
4c3130efda1ef4 fs/cifs/cifssmb.c Steve French 2008-12-09 1790 /* can not handle big offset for old srv */
f80ac7eda1cf52 fs/smb/client/cifssmb.c David Howells 2025-10-24 @1791 return smb_EIO(smb_eio_trace_write_too_far);
4c3130efda1ef4 fs/cifs/cifssmb.c Steve French 2008-12-09 1792 }
4c3130efda1ef4 fs/cifs/cifssmb.c Steve French 2008-12-09 1793 }
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1794
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1795 rc = smb_init(SMB_COM_WRITE_ANDX, wct, tcon, (void **) &pSMB,
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1796 (void **) &pSMBr);
83bfbd0bb9025f fs/smb/client/cifssmb.c David Howells 2025-09-05 1797 if (rc < 0)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1798 return rc;
83bfbd0bb9025f fs/smb/client/cifssmb.c David Howells 2025-09-05 1799 in_len = rc;
fa2989f4473413 fs/cifs/cifssmb.c Pavel Shilovsky 2011-05-26 1800
fa2989f4473413 fs/cifs/cifssmb.c Pavel Shilovsky 2011-05-26 1801 pSMB->hdr.Pid = cpu_to_le16((__u16)pid);
fa2989f4473413 fs/cifs/cifssmb.c Pavel Shilovsky 2011-05-26 1802 pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid >> 16));
fa2989f4473413 fs/cifs/cifssmb.c Pavel Shilovsky 2011-05-26 1803
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1804 /* tcon and ses pointer are checked in smb_init */
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1805 if (tcon->ses->server == NULL)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1806 return -ECONNABORTED;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1807
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1808 pSMB->AndXCommand = 0xFF; /* none */
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1809 pSMB->Fid = netfid;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1810 pSMB->OffsetLow = cpu_to_le32(offset & 0xFFFFFFFF);
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1811 if (wct == 14)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1812 pSMB->OffsetHigh = cpu_to_le32(offset >> 32);
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1813
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1814 pSMB->Reserved = 0xFFFFFFFF;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1815 pSMB->WriteMode = 0;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1816 pSMB->Remaining = 0;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1817
50c2f753887270 fs/cifs/cifssmb.c Steve French 2007-07-13 1818 /* Can increase buffer size if buffer is big enough in some cases ie we
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1819 can send more if LARGE_WRITE_X capability returned by the server and if
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1820 our buffer is big enough or if we convert to iovecs on socket writes
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1821 and eliminate the copy to the CIFS buffer */
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1822 if (tcon->ses->capabilities & CAP_LARGE_WRITE_X) {
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1823 bytes_sent = min_t(const unsigned int, CIFSMaxBufSize, count);
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1824 } else {
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1825 bytes_sent = (tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1826 & ~0xFF;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1827 }
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1828
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1829 if (bytes_sent > count)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1830 bytes_sent = count;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1831 pSMB->DataOffset =
83bfbd0bb9025f fs/smb/client/cifssmb.c David Howells 2025-09-05 1832 cpu_to_le16(offsetof(struct smb_com_write_req, Data));
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1833 if (buf)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1834 memcpy(pSMB->Data, buf, bytes_sent);
dbbab32574c384 fs/cifs/cifssmb.c Al Viro 2016-09-05 1835 else if (count != 0) {
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1836 /* No buffer */
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1837 cifs_buf_release(pSMB);
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1838 return -EINVAL;
e30dcf3a1905b4 fs/cifs/cifssmb.c Steve French 2005-09-20 1839 } /* else setting file size with write of zero bytes */
e30dcf3a1905b4 fs/cifs/cifssmb.c Steve French 2005-09-20 1840 if (wct == 14)
e30dcf3a1905b4 fs/cifs/cifssmb.c Steve French 2005-09-20 1841 byte_count = bytes_sent + 1; /* pad */
ad7a2926b9e53c fs/cifs/cifssmb.c Steve French 2008-02-07 1842 else /* wct == 12 */
e30dcf3a1905b4 fs/cifs/cifssmb.c Steve French 2005-09-20 1843 byte_count = bytes_sent + 5; /* bigger pad, smaller smb hdr */
ad7a2926b9e53c fs/cifs/cifssmb.c Steve French 2008-02-07 1844
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1845 pSMB->DataLengthLow = cpu_to_le16(bytes_sent & 0xFFFF);
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1846 pSMB->DataLengthHigh = cpu_to_le16(bytes_sent >> 16);
83bfbd0bb9025f fs/smb/client/cifssmb.c David Howells 2025-09-05 1847 in_len += byte_count;
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1848
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1849 if (wct == 14)
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1850 pSMB->ByteCount = cpu_to_le16(byte_count);
50c2f753887270 fs/cifs/cifssmb.c Steve French 2007-07-13 1851 else { /* old style write has byte count 4 bytes earlier
50c2f753887270 fs/cifs/cifssmb.c Steve French 2007-07-13 1852 so 4 bytes pad */
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1853 struct smb_com_writex_req *pSMBW =
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1854 (struct smb_com_writex_req *)pSMB;
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1855 pSMBW->ByteCount = cpu_to_le16(byte_count);
1c9551878c4629 fs/cifs/cifssmb.c Steve French 2005-08-30 1856 }
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1857
83bfbd0bb9025f fs/smb/client/cifssmb.c David Howells 2025-09-05 1858 rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB, in_len,
dbbab32574c384 fs/cifs/cifssmb.c Al Viro 2016-09-05 1859 (struct smb_hdr *) pSMBr, &bytes_returned, 0);
44c581866e2ae4 fs/cifs/cifssmb.c Pavel Shilovsky 2012-05-28 1860 cifs_stats_inc(&tcon->stats.cifs_stats.num_writes);
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1861 if (rc) {
f96637be081141 fs/cifs/cifssmb.c Joe Perches 2013-05-04 1862 cifs_dbg(FYI, "Send error in write = %d\n", rc);
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1863 } else {
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1864 *nbytes = le16_to_cpu(pSMBr->CountHigh);
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1865 *nbytes = (*nbytes) << 16;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1866 *nbytes += le16_to_cpu(pSMBr->Count);
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1867
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1868 /*
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1869 * Mask off high 16 bits when bytes written as returned by the
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1870 * server is greater than bytes requested by the client. Some
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1871 * OS/2 servers are known to set incorrect CountHigh values.
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1872 */
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1873 if (*nbytes > count)
6513a81e9325d7 fs/cifs/cifssmb.c Suresh Jayaraman 2010-03-31 1874 *nbytes &= 0xFFFF;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1875 }
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1876
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1877 cifs_buf_release(pSMB);
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1878
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1879 /* Note: On -EAGAIN error only caller can retry on handle based calls
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1880 since file handle passed in no longer valid */
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1881
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1882 return rc;
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1883 }
^1da177e4c3f41 fs/cifs/cifssmb.c Linus Torvalds 2005-04-16 1884
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] smb: client: reject a tree connect response whose byte count is too small
2026-07-28 18:06 ` Bryam Vargas
(?)
(?)
@ 2026-08-18 0:00 ` Namjae Jeon
2026-08-21 3:16 ` Bryam Vargas
-1 siblings, 1 reply; 5+ messages in thread
From: Namjae Jeon @ 2026-08-18 0:00 UTC (permalink / raw)
To: hexlabsecurity
Cc: Steve French, Paulo Alcantara, Shyam Prasad N, samba-technical,
Ronnie Sahlberg, Jeff Layton, linux-cifs, linux-kernel,
Tom Talpey, Bharath SM, David Howells
On Wed, Jul 29, 2026 at 3:12 AM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> From: Bryam Vargas <hexlabsecurity@proton.me>
Hi Bryam,
>
> CIFSTCon() bounds its strnlen() over the byte area with the server's
> ByteCount minus two, which for ByteCount 0 or 1 goes negative as an int
> and converts to a huge size_t. The later subtraction then wraps __u16
> bytes_left, and that is what bounds cifs_strndup_from_utf16().
>
> pByteArea() scales with the response's WordCount, also server-chosen, so
> the walk can start past everything the request itself wrote, of which
> cifs_buf_get() clears only the first 67 bytes. KMSAN reports the read of
> uninitialised heap, and the bytes reach userspace through
> tcon->nativeFileSystem in /proc/fs/cifs/DebugData.
>
> Reject a byte area too small for what the parser consumes. A response
> carrying a service string has at least two bytes there, so nothing valid
> is rejected, and both subtractions are then safe.
>
> Fixes: cc20c031bb06 ("cifs: convert CIFSTCon to use new unicode helper functions")
> Cc: stable@kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Can you check the build warning that was reported by the kernel test robot ?
All warnings (new ones prefixed by >>):
>> fs/smb/client/cifssmb.c:1791:19: warning: implicit conversion from 'int' to 'enum smb_eio_trace' changes value from 128 to -128 [-Wconstant-conversion]
1791 | return smb_EIO(smb_eio_trace_write_too_far);
| ~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/smb/client/cifssmb.c:1982:17: warning: implicit conversion from
'int' to 'enum smb_eio_trace' changes value from 128 to -128
[-Wconstant-conversion]
1982 | rc = smb_EIO(smb_eio_trace_write_too_far);
| ~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
fs/smb/client/cifssmb.c:2074:19: warning: implicit conversion from
'int' to 'enum smb_eio_trace' changes value from 128 to -128
[-Wconstant-conversion]
2074 | return smb_EIO(smb_eio_trace_write_too_far);
| ~~~~~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] smb: client: reject a tree connect response whose byte count is too small
2026-08-18 0:00 ` Namjae Jeon
@ 2026-08-21 3:16 ` Bryam Vargas
0 siblings, 0 replies; 5+ messages in thread
From: Bryam Vargas @ 2026-08-21 3:16 UTC (permalink / raw)
To: Namjae Jeon
Cc: Steve French, Paulo Alcantara, Shyam Prasad N, Ronnie Sahlberg,
Jeff Layton, Tom Talpey, Bharath SM, David Howells,
samba-technical, linux-cifs, linux-kernel
Namjae,
> Can you check the build warning that was reported by the kernel test robot ?
It's mine. The three sites it points at are in the write path, but the
cause is in trace.h.
enum smb_eio_trace is __mode(byte), and the list has held exactly 128
entries since f80ac7eda1cf added it, which puts the last one,
smb_eio_trace_write_too_far, at index 127. My patch inserts
smb_eio_trace_tcon_bcc_too_small ahead of tdis_in_reconnect, everything
after it shifts up one, and write_too_far lands on 128.
It is worse than a warning where CONFIG_WERROR is set, and x86_64 defconfig
sets it: clang gives the enum a signed underlying type, converts the value
to -128 and the build stops. I reproduced that at v7.2 with clang 19.1.7 --
three errors and cifssmb.o fails, so v1 as it stands does not build there.
gcc gives the same enum an unsigned underlying type and says nothing, which
is why the tree had not run into it. Where WERROR is off, the recorded
value stops matching the __print_symbolic() table and those three events
print a raw number instead of their name.
So the enum is full for everyone, not only for me: the next
smb_eio_trace_* anyone adds lands on 128 too, and under gcc it does it
quietly. Of the 68 __mode(byte) enums in the tree this was the only one
with no room left, though rxrpc_abort_reason has 127 entries.
v2 is two patches: 1/2 drops __mode(byte) from enum smb_eio_trace, 2/2 is
the tree connect fix unchanged. I dropped the attribute rather than
widening it because on x86_64 the record does not grow either way -- the
field precedes an unsigned long at offset 8, so sizeof(struct
trace_event_raw_smb3_eio) is 32 whichever type the enum gets -- and a plain
enum avoids a spelling the tree does not use (all 68 __mode() uses are
__mode(byte)). If you or David would rather keep it packed and widen it,
say so and I'll respin.
Two corrections to v1 while I am here, both in the changelog rather than
the code. The initialised prefix is 256 bytes, not the 67 I wrote:
header_assemble() memsets 256 right after cifs_buf_get(), so the byte area
only starts past it from WordCount 111. And the impact is understated --
once bytes_left wraps, the bound handed to cifs_strndup_from_utf16() can
reach 65535 against a ~16 KB cifs_req_poolp object, so this is a slab
out-of-bounds read and not only an uninitialised one.
v2 is on the list:
https://lore.kernel.org/all/20260820-b4-disp-58f78a28-v2-0-1fb7a6cb1533@proton.me/
Thanks for the review.
Bryam
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-21 3:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28 18:06 [PATCH] smb: client: reject a tree connect response whose byte count is too small Bryam Vargas via B4 Relay
2026-07-28 18:06 ` Bryam Vargas
2026-07-29 1:42 ` kernel test robot
2026-08-18 0:00 ` Namjae Jeon
2026-08-21 3:16 ` Bryam Vargas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.