Skip to contentSkip to conversation

ksmbd: fix SMB 3.11 posix extension mount failure

PATCH In review Namjae Jeon 2 participants 3 messages Jan 22, 2022 – Jan 23, 2022

Subsystems (3)
Stable branch SMB client (CIFS) Kernel SMB3 server (ksmbd)
lore
Share & tools
lore

Namjae Jeon

author maintainer
#0 lore raw

PATCH 1/1 ksmbd: fix SMB 3.11 posix extension mount failure

cifs client set 4 to DataLength of create_posix context, which mean Mode variable of create_posix context is only available. So buffer validation of ksmbd should check only the size of Mode except for the size of Reserved variable.

Fixes 8f77150c15f8 ("ksmbd: add buffer validation for SMB2_CREATE_CONTEXT")
Cc [email protected] # v5.15+
Reported-by Steve French <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
---
 fs/ksmbd/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ksmbd/smb2pdu.c +1 -1
index 1866c81c5c99..3926ca18dca4 100644
@@ -2688,7 +2688,7 @@ int smb2_open(struct ksmbd_work *work)
 					(struct create_posix *)context;
 				if (le16_to_cpu(context->DataOffset) +
 				    le32_to_cpu(context->DataLength) <
-				    sizeof(struct create_posix)) {
+				    sizeof(struct create_posix) - 4) {
 					rc = -EINVAL;
 					goto err_out1;
 				}
Signature · 1 line
2.25.1
#1 lore raw

Looks good. I tested it and it works.

With this I can see some additional things work e.g. "stat /mnt/file" shows the correct mode bits, but the owner and group are reported as the default (0) instead of the actual uid/gid

Quoted 30 lines · Namjae Jeon

cifs client set 4 to DataLength of create_posix context, which mean Mode variable of create_posix context is only available. So buffer validation of ksmbd should check only the size of Mode except for the size of Reserved variable.

Fixes 8f77150c15f8 ("ksmbd: add buffer validation for SMB2_CREATE_CONTEXT")
Cc [email protected] # v5.15+
Reported-by Steve French <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
---
 fs/ksmbd/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ksmbd/smb2pdu.c +1 -2
index 1866c81c5c99..3926ca18dca4 100644
@@ -2688,7 +2688,7 @@ int smb2_open(struct ksmbd_work *work)
                                        (struct create_posix *)context;
                                if (le16_to_cpu(context->DataOffset) +
                                    le32_to_cpu(context->DataLength) <
-                                   sizeof(struct create_posix)) {
+                                   sizeof(struct create_posix) - 4) {
                                        rc = -EINVAL;
                                        goto err_out1;
                                }
--

2.25.1

Signature · 3 lines
Thanks,

Steve
author maintainer
#2 lore raw

2022-01-22 14:00 GMT+09:00, Steve French <[email protected]>:

Quoted 5 lines

Looks good. I tested it and it works.

With this I can see some additional things work e.g. "stat /mnt/file" shows the correct mode bits, but the owner and group are reported as the default (0) instead of the actual uid/gid

As we discussed on chat, we need to implement code to handle Sids[] in smb311_posix_qinfo on both ksmbd and cifs.

fattr->cf_uid = cifs_sb->ctx->linux_uid; /* TODO: map uid and gid from SID */ fattr->cf_gid = cifs_sb->ctx->linux_gid;

cifs client doesn't handle Sids[] from server. Instead, cifs client seems to use uid/gid from mount option. So the uid/gid will appear as
0. And I will also change ksmbd to set uid/gid to Sid in smb311_posix_qinfo.

Thanks!

Quoted 40 lines · Namjae Jeon
Quoted 29 lines

cifs client set 4 to DataLength of create_posix context, which mean Mode variable of create_posix context is only available. So buffer validation of ksmbd should check only the size of Mode except for the size of Reserved variable.

Fixes 8f77150c15f8 ("ksmbd: add buffer validation for

SMB2_CREATE_CONTEXT")

Cc [email protected] # v5.15+
Reported-by Steve French <[email protected]>
Signed-off-by Namjae Jeon <[email protected]>
---
 fs/ksmbd/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ksmbd/smb2pdu.c +1 -2
index 1866c81c5c99..3926ca18dca4 100644
@@ -2688,7 +2688,7 @@ int smb2_open(struct ksmbd_work *work)
                                        (struct create_posix *)context;
                                if (le16_to_cpu(context->DataOffset) +
                                    le32_to_cpu(context->DataLength) <
-                                   sizeof(struct create_posix)) {
+                                   sizeof(struct create_posix) - 4) {
                                        rc = -EINVAL;
                                        goto err_out1;
                                }
--
Quoted 2 lines

2.25.1

--
Thanks,

Steve