[PATCH 5.15.y] cifs: fix off-by-one in SMB2_query_info_init()

[PATCH 5.15.y] cifs: fix off-by-one in SMB2_query_info_init()

[Harshit Mogalapalli]

Bug: After mounting the cifs fs, it complains with Resource temporarily
unavailable messages.

[root@vm1 xfstests-dev]# ./check -g quick -s smb3
TEST_DEV=//<SERVER_IP>/TEST is mounted but not a type cifs filesystem
[root@vm1 xfstests-dev]# df
df: /mnt/test: Resource temporarily unavailable

Paul's analysis of the bug:

	Bug is related to an off-by-one in smb2_set_next_command() when
	the client attempts to pad SMB2_QUERY_INFO request -- since it isn't
	8 byte aligned -- even though smb2_query_info_compound() doesn't
	provide an extra iov for such padding.

	v5.15.y doesn't have

        eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")

	and the commit does

		if (unlikely(check_add_overflow(input_len, sizeof(*req), &len) ||
			     len > CIFSMaxBufSize))
			return -EINVAL;

	so sizeof(*req) will wrongly include the extra byte from
	smb2_query_info_req::Buffer making @len unaligned and therefore causing
	OOB in smb2_set_next_command().

Fixes: bfd18c0f570e4 ("smb: client: fix OOB in SMB2_query_info_init()")
Suggested-by: Paulo Alcantara <pc@manguebit.com>
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
This patch is only for 5.15.y stable kernel.
I have tested the patched kernel: after mounting it doesn't become
unavailable.
---
 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 6714e9db0ee83..b4b1d8132910c 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -3448,7 +3448,7 @@ SMB2_query_info_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server,
 
 	iov[0].iov_base = (char *)req;
 	/* 1 for Buffer */
-	iov[0].iov_len = len;
+	iov[0].iov_len = len - 1;
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH 5.15.y] cifs: fix off-by-one in SMB2_query_info_init()

[Greg KH]

On Sun, Jan 28, 2024 at 09:07:58AM -0800, Harshit Mogalapalli wrote:
> Bug: After mounting the cifs fs, it complains with Resource temporarily
> unavailable messages.
> 
> [root@vm1 xfstests-dev]# ./check -g quick -s smb3
> TEST_DEV=//<SERVER_IP>/TEST is mounted but not a type cifs filesystem
> [root@vm1 xfstests-dev]# df
> df: /mnt/test: Resource temporarily unavailable
> 
> Paul's analysis of the bug:
> 
> 	Bug is related to an off-by-one in smb2_set_next_command() when
> 	the client attempts to pad SMB2_QUERY_INFO request -- since it isn't
> 	8 byte aligned -- even though smb2_query_info_compound() doesn't
> 	provide an extra iov for such padding.
> 
> 	v5.15.y doesn't have
> 
>         eb3e28c1e89b ("smb3: Replace smb2pdu 1-element arrays with flex-arrays")
> 
> 	and the commit does
> 
> 		if (unlikely(check_add_overflow(input_len, sizeof(*req), &len) ||
> 			     len > CIFSMaxBufSize))
> 			return -EINVAL;
> 
> 	so sizeof(*req) will wrongly include the extra byte from
> 	smb2_query_info_req::Buffer making @len unaligned and therefore causing
> 	OOB in smb2_set_next_command().
> 
> Fixes: bfd18c0f570e4 ("smb: client: fix OOB in SMB2_query_info_init()")
> Suggested-by: Paulo Alcantara <pc@manguebit.com>
> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> ---
> This patch is only for 5.15.y stable kernel.
> I have tested the patched kernel: after mounting it doesn't become
> unavailable.

Now queued up, thanks.

greg k-h