mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2026-09-22 09:34:56 +02:00
smb: client: restrict implied bcc[0] exemption to responses without data area
commit53b7c271f0upstream. smb2_check_message() has a long-standing quirk that accepts a response whose calculated length is one byte larger than the bytes actually received ("server can return one byte more due to implied bcc[0]"). This was introduced to accommodate servers that omit the trailing bcc[0] overlap byte when no data area is present. However, the exemption is applied unconditionally, regardless of whether the command actually carries a data area (has_smb2_data_area[]). When a response with a data area is subject to the +1 exemption, the reported data can extend one byte beyond the bytes actually received, yet smb2_check_message() still accepts it. The subsequent decoder then reads past the end of the receive buffer. This is reachable during NEGOTIATE and SESSION_SETUP, before the session is established. The resulting out-of-bounds reads are visible under KASAN when mounting against a non-conforming server; both the SPNEGO/negTokenInit and the NTLMSSP challenge decoders are affected: BUG: KASAN: slab-out-of-bounds in asn1_ber_decoder+0x16a7/0x1b00 Read of size 1 at addr ffff8880084d67c0 by task mount.cifs/81 CPU: 1 UID: 0 PID: 81 Comm: mount.cifs Not tainted 7.1.0-rc6 #1 Call Trace: <TASK> dump_stack_lvl+0x4e/0x70 print_report+0x157/0x4c9 kasan_report+0xce/0x100 asn1_ber_decoder+0x16a7/0x1b00 decode_negTokenInit+0x19/0x30 SMB2_negotiate+0x31d9/0x4c90 cifs_negotiate_protocol+0x1f2/0x3f0 cifs_get_smb_ses+0x93f/0x17e0 cifs_mount_get_session+0x7f/0x3a0 cifs_mount+0xb4/0xcf0 cifs_smb3_do_mount+0x23a/0x1500 smb3_get_tree+0x3b0/0x630 vfs_get_tree+0x82/0x2d0 fc_mount+0x10/0x1b0 path_mount+0x50d/0x1de0 __x64_sys_mount+0x20b/0x270 do_syscall_64+0xee/0x590 entry_SYSCALL_64_after_hwframe+0x77/0x7f </TASK> Allocated by task 85: kmem_cache_alloc_noprof+0x106/0x380 mempool_alloc_noprof+0x116/0x1e0 cifs_small_buf_get+0x31/0x80 allocate_buffers+0x10d/0x2b0 cifs_demultiplex_thread+0x1d5/0x1d50 kthread+0x2c6/0x390 ret_from_fork+0x36e/0x5a0 ret_from_fork_asm+0x1a/0x30 The buggy address is located 0 bytes to the right of allocated 448-byte region [ffff8880084d6600, ffff8880084d67c0) which belongs to the cache cifs_small_rq of size 448 BUG: KASAN: slab-out-of-bounds in kmemdup_noprof+0x36/0x50 Read of size 329 at addr ffff88800726c678 by task mount.cifs/89 CPU: 0 UID: 0 PID: 89 Comm: mount.cifs Tainted: G B 7.1.0-rc6 #1 Call Trace: <TASK> dump_stack_lvl+0x4e/0x70 print_report+0x157/0x4c9 kasan_report+0xce/0x100 kasan_check_range+0x10f/0x1e0 __asan_memcpy+0x23/0x60 kmemdup_noprof+0x36/0x50 decode_ntlmssp_challenge+0x457/0x680 SMB2_sess_auth_rawntlmssp_negotiate+0x6f0/0xcb0 SMB2_sess_setup+0x219/0x4f0 cifs_setup_session+0x248/0xaf0 cifs_get_smb_ses+0xf79/0x17e0 cifs_mount_get_session+0x7f/0x3a0 cifs_mount+0xb4/0xcf0 cifs_smb3_do_mount+0x23a/0x1500 smb3_get_tree+0x3b0/0x630 vfs_get_tree+0x82/0x2d0 fc_mount+0x10/0x1b0 path_mount+0x50d/0x1de0 __x64_sys_mount+0x20b/0x270 do_syscall_64+0xee/0x590 entry_SYSCALL_64_after_hwframe+0x77/0x7f </TASK> Allocated by task 93: kmem_cache_alloc_noprof+0x106/0x380 mempool_alloc_noprof+0x116/0x1e0 cifs_small_buf_get+0x31/0x80 allocate_buffers+0x10d/0x2b0 cifs_demultiplex_thread+0x1d5/0x1d50 kthread+0x2c6/0x390 ret_from_fork+0x36e/0x5a0 ret_from_fork_asm+0x1a/0x30 The buggy address is located 120 bytes inside of allocated 448-byte region [ffff88800726c600, ffff88800726c7c0) which belongs to the cache cifs_small_rq of size 448 Restrict the +1 exemption to responses that have no data area, so that it still covers the bcc[0] omission it was meant for. When a data area is present, the +1 discrepancy instead means the reported data length overruns the received buffer, so the response must be rejected. Fixes:093b2bdad3("CIFS: Make demultiplex_thread work with SMB2 code") Cc: stable@vger.kernel.org Signed-off-by: Shoichiro Miyamoto <shoichiro.miyamoto@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
committed by
Greg Kroah-Hartman
parent
adc8b9c30d
commit
573e502d14
@@ -18,6 +18,8 @@
|
||||
#include "nterr.h"
|
||||
#include "cached_dir.h"
|
||||
|
||||
static unsigned int __smb2_calc_size(void *buf, bool *have_data);
|
||||
|
||||
static int
|
||||
check_smb2_hdr(struct smb2_hdr *shdr, __u64 mid)
|
||||
{
|
||||
@@ -143,6 +145,7 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *server)
|
||||
int command;
|
||||
__u32 calc_len; /* calculated length */
|
||||
__u64 mid;
|
||||
bool have_data;
|
||||
|
||||
/* If server is a channel, select the primary channel */
|
||||
pserver = SERVER_IS_CHAN(server) ? server->primary_server : server;
|
||||
@@ -226,7 +229,8 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *server)
|
||||
}
|
||||
}
|
||||
|
||||
calc_len = smb2_calc_size(buf);
|
||||
have_data = false;
|
||||
calc_len = __smb2_calc_size(buf, &have_data);
|
||||
|
||||
/* For SMB2_IOCTL, OutputOffset and OutputLength are optional, so might
|
||||
* be 0, and not a real miscalculation */
|
||||
@@ -245,8 +249,13 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *server)
|
||||
/* Windows 7 server returns 24 bytes more */
|
||||
if (calc_len + 24 == len && command == SMB2_OPLOCK_BREAK_HE)
|
||||
return 0;
|
||||
/* server can return one byte more due to implied bcc[0] */
|
||||
if (calc_len == len + 1)
|
||||
/*
|
||||
* Server can return one byte more due to implied bcc[0].
|
||||
* Allow it only when there is no data area; if data_length > 0
|
||||
* the +1 gap indicates an overreported data length rather than
|
||||
* the bcc[0] omission.
|
||||
*/
|
||||
if (calc_len == len + 1 && !have_data)
|
||||
return 0;
|
||||
|
||||
/*
|
||||
@@ -407,14 +416,17 @@ smb2_get_data_area_len(int *off, int *len, struct smb2_hdr *shdr)
|
||||
/*
|
||||
* Calculate the size of the SMB message based on the fixed header
|
||||
* portion, the number of word parameters and the data portion of the message.
|
||||
* If have_data is non-NULL, it is set to true when a non-empty data area was
|
||||
* found (data_length > 0), allowing callers to distinguish the implied bcc[0]
|
||||
* case (no data area) from an overreported data length.
|
||||
*/
|
||||
unsigned int
|
||||
smb2_calc_size(void *buf)
|
||||
static unsigned int
|
||||
__smb2_calc_size(void *buf, bool *have_data)
|
||||
{
|
||||
struct smb2_pdu *pdu = buf;
|
||||
struct smb2_hdr *shdr = &pdu->hdr;
|
||||
int offset; /* the offset from the beginning of SMB to data area */
|
||||
int data_length; /* the length of the variable length data area */
|
||||
int data_length = 0; /* the length of the variable length data area */
|
||||
/* Structure Size has already been checked to make sure it is 64 */
|
||||
int len = le16_to_cpu(shdr->StructureSize);
|
||||
|
||||
@@ -447,9 +459,17 @@ smb2_calc_size(void *buf)
|
||||
}
|
||||
calc_size_exit:
|
||||
cifs_dbg(FYI, "SMB2 len %d\n", len);
|
||||
if (have_data)
|
||||
*have_data = (data_length > 0);
|
||||
return len;
|
||||
}
|
||||
|
||||
unsigned int
|
||||
smb2_calc_size(void *buf)
|
||||
{
|
||||
return __smb2_calc_size(buf, NULL);
|
||||
}
|
||||
|
||||
/* Note: caller must free return buffer */
|
||||
__le16 *
|
||||
cifs_convert_path_to_utf16(const char *from, struct cifs_sb_info *cifs_sb)
|
||||
|
||||
Reference in New Issue
Block a user