[PATCH 5.10 1/1] cifs: Fix stack-out-of-bounds in smb2_set_next_command()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



After backporting the mainline commit 33eae65c6f49 ("smb: client: fix
OOB in SMB2_query_info_init()") to the linux-5.10.y stable branch,
an issue arose where the cifs statfs system call failed, resulting in:

  $ df /mnt
  df: /mnt: Resource temporarily unavailable

KASAN also reported a stack-out-of-bounds error as follows:

 ==================================================================
 BUG: KASAN: stack-out-of-bounds in smb2_set_next_command+0x247/0x280
 [cifs]
 Write of size 8 at addr ffff8881073ef830 by task df/533

 CPU: 4 PID: 533 Comm: df Not tainted 5.10.0+ #17
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.1-2.
 fc37 04/01/2014
 Call Trace:
  dump_stack+0xb3/0xf1
  print_address_description.constprop.0+0x1e/0x280
  __kasan_report.cold+0x6c/0x84
  kasan_report+0x3a/0x50
  smb2_set_next_command+0x247/0x280 [cifs]
  smb2_query_info_compound+0x3e9/0x5d0 [cifs]
  smb2_queryfs+0xb9/0x180 [cifs]
  smb311_queryfs+0x218/0x230 [cifs]
  cifs_statfs+0x161/0x340 [cifs]
  statfs_by_dentry+0xa8/0x100
  vfs_statfs+0x2f/0x180
  user_statfs+0x96/0x100
  __se_sys_statfs+0x6a/0xc0
  do_syscall_64+0x33/0x40
  entry_SYSCALL_64_after_hwframe+0x62/0xc7
 RIP: 0033:0x7ff427ad93b7
 Code: ff ff ff ff c3 66 0f 1f 44 00 00 48 8b 05 59 ba 0d 00 64 c7 00
 16 00 00 00 b8 ff ff ff ff c3 0f 1f 40 00 b8 89 00 00 00 0f 05 <48> 3d
 00 f0 ff ff 77 01 c3 48 8b 15 31 ba 0d 00 f7 d8 64 89 8
 RSP: 002b:00007ffd8371e158 EFLAGS: 00000246 ORIG_RAX: 0000000000000089
 RAX: ffffffffffffffda RBX: 00007ffd8371e200 RCX: 00007ff427ad93b7
 RDX: 0000000000000003 RSI: 00007ffd8371e160 RDI: 00007ffd8371ee4b
 RBP: 00007ffd8371e160 R08: 00007ffd8371e283 R09: 0000000000000032
 R10: 00007ff4279ed368 R11: 0000000000000246 R12: 00007ffd8371e200
 R13: 000056296a125dd0 R14: 0000000000000001 R15: 0000000000000000

 The buggy address belongs to the page:
 page:00000000862dac80 refcount:0 mapcount:0 mapping:0000000000000000
 index:0x0 pfn:0x1073ef
 flags: 0x17ffffc0000000(node=0|zone=2|lastcpupid=0x1fffff)
 raw: 0017ffffc0000000 0000000000000000 dead000000000122 0000000000000000
 raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
 page dumped because: kasan: bad access detected

 addr ffff8881073ef830 is located in stack of task df/533 at offset 112
 in frame:
  smb2_query_info_compound+0x0/0x5d0 [cifs]

 this frame has 9 objects:
  [48, 49) 'oplock'
  [64, 76) 'resp_buftype'
  [96, 112) 'qi_iov'
  [128, 144) 'close_iov'
  [160, 208) 'rsp_iov'
  [240, 296) 'oparms'
  [336, 456) 'rqst'
  [496, 624) 'open_iov'
  [656, 736) 'fid'

 Memory state around the buggy address:
  ffff8881073ef700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff8881073ef780: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f1 f1 01 f2
 >ffff8881073ef800: 00 04 f2 f2 00 00 f2 f2 00 00 f2 f2 00 00 00 00
                                      ^
  ffff8881073ef880: 00 00 f2 f2 f2 f2 00 00 00 00 00 00 00 f2 f2 f2
  ffff8881073ef900: f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ==================================================================
 Disabling lock debugging due to kernel taint

This issue was caused because the stable branch did not include the
prerequisite patch eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays
with flex-arrays"). The patch replaces the trailing 1-element array with
a flexible array in the smb2_query_info_req structure and modifies the
length calculation expression from `sizeof(smb2_query_info_req) - 1` to
`sizeof(smb2_query_info_req)`. Consequently, backporting only commit
33eae65c6f49 led to an incorrect length calculation for the
smb2_query_info_req structure within SMB2_query_info_init().

cifs_statfs
smb2_queryfs
  smb2_query_info_compound
    struct kvec qi_iov[1];
    rqst[1].rq_iov = qi_iov;
    rqst[1].rq_nvec = 1;
    SMB2_query_info_init
      # The length of len is incorrect because the value of sizeof(req)
      # is not decreased by 1.
      check_add_overflow(input_len, sizeof(*req), &len)
    smb2_set_next_command(tcon, &rqst[1]);
      # 1 byte greater than the actual length
      len = smb_rqst_len(server, rqst);
      # 'len' is not 8-byte aligned, then paddingg.
      # Access to .rq_iov[1] results in out-of-bounds array
      rqst->rq_iov[rqst->rq_nvec].iov_base = smb2_padding;
    compound_send_recv

cifs_demultiplex_thread
  cifs_read_from_socket
    cifs_readv_from_socket
      # The request may be invalid and no expected response.
      length = sock_recvmsg

This patch corrects the length calculation for the smb2_query_info_req
structure in SMB2_query_info_init() to address this problem.

Signed-off-by: ZhaoLong Wang <wangzhaolong1@xxxxxxxxxx>
---
 fs/cifs/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 76679dc4e632..8532cc416188 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -3354,7 +3354,7 @@ SMB2_query_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 	size_t len;
 	int rc;
 
-	if (unlikely(check_add_overflow(input_len, sizeof(*req), &len) ||
+	if (unlikely(check_add_overflow(input_len, sizeof(*req) - 1, &len) ||
 		     len > CIFSMaxBufSize))
 		return -EINVAL;
 
-- 
2.39.2





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux