Hello,
I am sending this patch for inclusion in the stable tree, as it fixes a critical stack-out-of-bounds bug in the cifs module related to the `smb2_set_next_command()` function.
Problem Summary: A problem was observed in the `statfs` system call for cifs, where it failed with a "Resource temporarily unavailable" message. Further investigation with KASAN revealed a stack-out-of-bounds error. The root cause was a miscalculation of the size of the `smb2_query_info_req` structure in the `SMB2_query_info_init()` function.
This situation arose due to a dependency on a prior commit (`eb3e28c1e89b`) that replaced a 1-element array with a flexible array member in the `smb2_query_info_req` structure. This commit was not backported to the 5.10.y and 5.15.y stable branch, leading to an incorrect size calculation after the backport of commit `33eae65c6f49`.
Fix Details: The patch corrects the size calculation to ensure the correct length is used when initializing the `smb2_query_info_req` structure. It has been tested and confirmed to resolve the issue without introducing any regressions.
Maybe the prior commit eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") should be backported to solve this problem directly. The patch does not seem to conflict.
Best regards, ZhaoLong Wang
ZhaoLong Wang (1): cifs: Fix stack-out-of-bounds in smb2_set_next_command()
fs/cifs/smb2pdu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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@huawei.com --- 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 6714e9db0ee8..63058e12dbbb 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -3423,7 +3423,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;
Hi ZhaoLong,
+CC Kovalev, Mohamed (who also worked on this issue)
On 07/02/24 5:17 pm, ZhaoLong Wang wrote:
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
This is true but there are other backporting efforts on this and 5.15.y
The latest is to backport eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays") to 5.15.y and pull out a similar one liner fix out of the stable-queue from 5.15.-stable queue and 5.10.stable-queue
Reference threads: 1. https://lore.kernel.org/all/7903fc0a-d0c5-20bf-20cc-d9f092e5c498@basealt.ru/
2. https://lore.kernel.org/all/20240206161111.454699-1-kovalev@altlinux.org/
Applying Kovalev's recent backport[2] most likely will fix this issue.
Thanks, Harshit
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
linux-stable-mirror@lists.linaro.org
-
Harshit Mogalapalli -
ZhaoLong Wang