Skip to content

Commit a8f68b1

Browse files
pcacjrSteve French
authored and
Steve French
committed
smb: client: fix OOB in cifsd when receiving compounded resps
Validate next header's offset in ->next_header() so that it isn't smaller than MID_HEADER_SIZE(server) and then standard_receive3() or ->receive() ends up writing off the end of the buffer because 'pdu_length - MID_HEADER_SIZE(server)' wraps up to a huge length: BUG: KASAN: slab-out-of-bounds in _copy_to_iter+0x4fc/0x840 Write of size 701 at addr ffff88800caf407f by task cifsd/1090 CPU: 0 PID: 1090 Comm: cifsd Not tainted 6.7.0-rc4 #5 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x4a/0x80 print_report+0xcf/0x650 ? srso_alias_return_thunk+0x5/0xfbef5 ? srso_alias_return_thunk+0x5/0xfbef5 ? __phys_addr+0x46/0x90 kasan_report+0xd8/0x110 ? _copy_to_iter+0x4fc/0x840 ? _copy_to_iter+0x4fc/0x840 kasan_check_range+0x105/0x1b0 __asan_memcpy+0x3c/0x60 _copy_to_iter+0x4fc/0x840 ? srso_alias_return_thunk+0x5/0xfbef5 ? hlock_class+0x32/0xc0 ? srso_alias_return_thunk+0x5/0xfbef5 ? __pfx__copy_to_iter+0x10/0x10 ? srso_alias_return_thunk+0x5/0xfbef5 ? lock_is_held_type+0x90/0x100 ? srso_alias_return_thunk+0x5/0xfbef5 ? __might_resched+0x278/0x360 ? __pfx___might_resched+0x10/0x10 ? srso_alias_return_thunk+0x5/0xfbef5 __skb_datagram_iter+0x2c2/0x460 ? __pfx_simple_copy_to_iter+0x10/0x10 skb_copy_datagram_iter+0x6c/0x110 tcp_recvmsg_locked+0x9be/0xf40 ? __pfx_tcp_recvmsg_locked+0x10/0x10 ? mark_held_locks+0x5d/0x90 ? srso_alias_return_thunk+0x5/0xfbef5 tcp_recvmsg+0xe2/0x310 ? __pfx_tcp_recvmsg+0x10/0x10 ? srso_alias_return_thunk+0x5/0xfbef5 ? srso_alias_return_thunk+0x5/0xfbef5 ? lock_acquire+0x14a/0x3a0 ? srso_alias_return_thunk+0x5/0xfbef5 inet_recvmsg+0xd0/0x370 ? __pfx_inet_recvmsg+0x10/0x10 ? __pfx_lock_release+0x10/0x10 ? do_raw_spin_trylock+0xd1/0x120 sock_recvmsg+0x10d/0x150 cifs_readv_from_socket+0x25a/0x490 [cifs] ? __pfx_cifs_readv_from_socket+0x10/0x10 [cifs] ? srso_alias_return_thunk+0x5/0xfbef5 cifs_read_from_socket+0xb5/0x100 [cifs] ? __pfx_cifs_read_from_socket+0x10/0x10 [cifs] ? __pfx_lock_release+0x10/0x10 ? do_raw_spin_trylock+0xd1/0x120 ? _raw_spin_unlock+0x23/0x40 ? srso_alias_return_thunk+0x5/0xfbef5 ? __smb2_find_mid+0x126/0x230 [cifs] cifs_demultiplex_thread+0xd39/0x1270 [cifs] ? __pfx_cifs_demultiplex_thread+0x10/0x10 [cifs] ? __pfx_lock_release+0x10/0x10 ? srso_alias_return_thunk+0x5/0xfbef5 ? mark_held_locks+0x1a/0x90 ? lockdep_hardirqs_on_prepare+0x136/0x210 ? srso_alias_return_thunk+0x5/0xfbef5 ? srso_alias_return_thunk+0x5/0xfbef5 ? __kthread_parkme+0xce/0xf0 ? __pfx_cifs_demultiplex_thread+0x10/0x10 [cifs] kthread+0x18d/0x1d0 ? kthread+0xdb/0x1d0 ? __pfx_kthread+0x10/0x10 ret_from_fork+0x34/0x60 ? __pfx_kthread+0x10/0x10 ret_from_fork_asm+0x1b/0x30 </TASK> Fixes: 8ce79ec ("cifs: update multiplex loop to handle compounded responses") Cc: stable@vger.kernel.org Reported-by: Robert Morris <rtm@csail.mit.edu> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent ceb6a6f commit a8f68b1

File tree

3 files changed

+20
-9
lines changed

3 files changed

+20
-9
lines changed

fs/smb/client/cifsglob.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,8 @@ struct smb_version_operations {
532532
struct mid_q_entry **, char **, int *);
533533
enum securityEnum (*select_sectype)(struct TCP_Server_Info *,
534534
enum securityEnum);
535-
int (*next_header)(char *);
535+
int (*next_header)(struct TCP_Server_Info *server, char *buf,
536+
unsigned int *noff);
536537
/* ioctl passthrough for query_info */
537538
int (*ioctl_query_info)(const unsigned int xid,
538539
struct cifs_tcon *tcon,

fs/smb/client/connect.c

+6-1
Original file line numberDiff line numberDiff line change
@@ -1201,7 +1201,12 @@ cifs_demultiplex_thread(void *p)
12011201
server->total_read += length;
12021202

12031203
if (server->ops->next_header) {
1204-
next_offset = server->ops->next_header(buf);
1204+
if (server->ops->next_header(server, buf, &next_offset)) {
1205+
cifs_dbg(VFS, "%s: malformed response (next_offset=%u)\n",
1206+
__func__, next_offset);
1207+
cifs_reconnect(server, true);
1208+
continue;
1209+
}
12051210
if (next_offset)
12061211
server->pdu_size = next_offset;
12071212
}

fs/smb/client/smb2ops.c

+12-7
Original file line numberDiff line numberDiff line change
@@ -5074,17 +5074,22 @@ smb3_handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid)
50745074
NULL, 0, false);
50755075
}
50765076

5077-
static int
5078-
smb2_next_header(char *buf)
5077+
static int smb2_next_header(struct TCP_Server_Info *server, char *buf,
5078+
unsigned int *noff)
50795079
{
50805080
struct smb2_hdr *hdr = (struct smb2_hdr *)buf;
50815081
struct smb2_transform_hdr *t_hdr = (struct smb2_transform_hdr *)buf;
50825082

5083-
if (hdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM)
5084-
return sizeof(struct smb2_transform_hdr) +
5085-
le32_to_cpu(t_hdr->OriginalMessageSize);
5086-
5087-
return le32_to_cpu(hdr->NextCommand);
5083+
if (hdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM) {
5084+
*noff = le32_to_cpu(t_hdr->OriginalMessageSize);
5085+
if (unlikely(check_add_overflow(*noff, sizeof(*t_hdr), noff)))
5086+
return -EINVAL;
5087+
} else {
5088+
*noff = le32_to_cpu(hdr->NextCommand);
5089+
}
5090+
if (unlikely(*noff && *noff < MID_HEADER_SIZE(server)))
5091+
return -EINVAL;
5092+
return 0;
50885093
}
50895094

50905095
int cifs_sfu_make_node(unsigned int xid, struct inode *inode,

0 commit comments

Comments
 (0)