[PATCH] smb: client: fix zero length for mkdir POSIX create context

[PATCH] smb: client: fix zero length for mkdir POSIX create context

[Jethro Donaldson]

smb: client: fix zero length for mkdir POSIX create context

SMB create requests issued via smb311_posix_mkdir() have an incorrect
length of zero bytes for the POSIX create context data. A ksmbd server
rejects such requests and logs "cli req too short" causing mkdir to fail
with "invalid argument" on the client side.

Inspection of packets sent by cifs.ko using wireshark show valid data for
the SMB2_POSIX_CREATE_CONTEXT is appended with the correct offset, but
with an incorrect length of zero bytes. Fails with ksmbd+cifs.ko only as
Windows server/client does not use POSIX extensions.

Fix smb311_posix_mkdir() to set req->CreateContextsLength as part of
appending the POSIX creation context to the request.

Signed-off-by: Jethro Donaldson <devel@jro.nz>
---

Tested as far as mkdir now works as expected.

Patch is against stable tree at v6.14.4 tag (first patch - unsure if I've
correctly done the base-commit thing, sorry).

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 163b8fea47e8..e7118501fdcc 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2920,6 +2920,7 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
 		req->CreateContextsOffset = cpu_to_le32(
 			sizeof(struct smb2_create_req) +
 			iov[1].iov_len);
+		le32_add_cpu(&req->CreateContextsLength, iov[n_iov-1].iov_len);
 		pc_buf = iov[n_iov-1].iov_base;
 	}


base-commit: ea061bad207e1ba693b5488ba64c663f7ca03f50
--
2.49.0

Re: [PATCH] smb: client: fix zero length for mkdir POSIX create context

[Steve French]

Good catch.  I did verify that this fixes posix mkdir to ksmbd.  It
didn't fail to Samba with posix extensions because Samba didn't check
for the incorrect length field.   The fix also avoids another problem,
an rmmod crash.  See below.

I added Cc: stable, and added the patch to cifs-2.6.git for-next


[ 1249.919717] RIP: 0010:__slab_err+0x1d/0x30
[ 1249.919719] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44
00 00 55 48 89 e5 e8 72 ff ff ff be 01 00 00 00 bf 05 00 00 00 e8 33
b2 1c 00 <0f> 0b 5d 31 f6 31 ff c3 cc cc cc cc 0f 1f 80 00 00 00 00 90
90 90
[ 1249.919721] RSP: 0018:ffffcf3041b0bab8 EFLAGS: 00010046
[ 1249.919723] RAX: 0000000000000000 RBX: ffffcf3041b0bb00 RCX: 0000000000000000
[ 1249.919724] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 1249.919725] RBP: ffffcf3041b0bab8 R08: 0000000000000000 R09: 0000000000000000
[ 1249.919727] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8c1b664fed00
[ 1249.919728] R13: ffff8c1b9cda6600 R14: dead000000000122 R15: ffff8c1b9cda6600
[ 1249.919729] FS:  00007d4b43e26080(0000) GS:ffff8c2312c9b000(0000)
knlGS:0000000000000000
[ 1249.919730] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1249.919732] CR2: 0000634aa6374a88 CR3: 00000002b21fe006 CR4: 00000000003726f0
[ 1249.919733] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1249.919734] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1249.919735] Call Trace:
[ 1249.919737]  <TASK>
[ 1249.919739]  free_partial.cold+0x137/0x191
[ 1249.919743]  __kmem_cache_shutdown+0x46/0xa0
[ 1249.919746]  kmem_cache_destroy+0x3e/0x1c0
[ 1249.919750]  cifs_destroy_request_bufs+0x39/0x50 [cifs]
[ 1249.919814]  exit_cifs+0x3a/0xcc0 [cifs]
[ 1249.919873]  __do_sys_delete_module.isra.0+0x19d/0x2e0
[ 1249.919877]  __x64_sys_delete_module+0x12/0x20

On Tue, Apr 29, 2025 at 8:17 AM Jethro Donaldson <devel@jro.nz> wrote:
>
> smb: client: fix zero length for mkdir POSIX create context
>
> SMB create requests issued via smb311_posix_mkdir() have an incorrect
> length of zero bytes for the POSIX create context data. A ksmbd server
> rejects such requests and logs "cli req too short" causing mkdir to fail
> with "invalid argument" on the client side.
>
> Inspection of packets sent by cifs.ko using wireshark show valid data for
> the SMB2_POSIX_CREATE_CONTEXT is appended with the correct offset, but
> with an incorrect length of zero bytes. Fails with ksmbd+cifs.ko only as
> Windows server/client does not use POSIX extensions.
>
> Fix smb311_posix_mkdir() to set req->CreateContextsLength as part of
> appending the POSIX creation context to the request.
>
> Signed-off-by: Jethro Donaldson <devel@jro.nz>
> ---
>
> Tested as far as mkdir now works as expected.
>
> Patch is against stable tree at v6.14.4 tag (first patch - unsure if I've
> correctly done the base-commit thing, sorry).
>
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index 163b8fea47e8..e7118501fdcc 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -2920,6 +2920,7 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode,
>                 req->CreateContextsOffset = cpu_to_le32(
>                         sizeof(struct smb2_create_req) +
>                         iov[1].iov_len);
> +               le32_add_cpu(&req->CreateContextsLength, iov[n_iov-1].iov_len);
>                 pc_buf = iov[n_iov-1].iov_base;
>         }
>
>
> base-commit: ea061bad207e1ba693b5488ba64c663f7ca03f50
> --
> 2.49.0
>


-- 
Thanks,

Steve

Re: [PATCH] smb: client: fix zero length for mkdir POSIX create context

[Jethro Donaldson]

On Tue, 29 Apr 2025 11:20:14 -0500
Steve French <smfrench@gmail.com> wrote:

> Good catch.  I did verify that this fixes posix mkdir to ksmbd.  It
> didn't fail to Samba with posix extensions because Samba didn't check
> for the incorrect length field.   The fix also avoids another problem,
> an rmmod crash.  See below.

Must admit I didn't ever see that crash as I mostly use module-less
kernels, so thanks for sharing this. It doesn't really seem like the fix
I'd submitted has any business resolving that?

I have checked I could reproduce the rmmod crash using a modular
defconfig[+smb] kernel in qemu, and then _with_ the fix applied to
cifs.ko I've added this snippet to ksmbd_smb2_check_message() in
fs/smb/server/smb2misc.c:

+       if (command == SMB2_CREATE) {
+               struct smb2_create_req *req = (struct smb2_create_req *)hdr;
+               if (le32_to_cpu(req->CreateDisposition) == FILE_CREATED)
+                       clc_len -= 40;
+       }

This reproduces the original behaviour in cifs.ko in spite of the fix,
forcing the client back down the error handling path. The rmmod crash then
still occurs. I suspect that won't be any surprise and that you'd chosen
your words deliberately when you say "avoids another problem", but felt I
should share this observation just in case.

I'm curious and will try and make some time to look at this closer, but I
suspect it's not so much of the low hanging fruit that the prior fix was.
Any tips or pointers to documentation welcome - very limited experience
with kernel or SMB stuff here.

Re: [PATCH] smb: client: fix zero length for mkdir POSIX create context

[Paulo Alcantara]

Jethro Donaldson <devel@jro.nz> writes:

> smb: client: fix zero length for mkdir POSIX create context
>
> SMB create requests issued via smb311_posix_mkdir() have an incorrect
> length of zero bytes for the POSIX create context data. A ksmbd server
> rejects such requests and logs "cli req too short" causing mkdir to fail
> with "invalid argument" on the client side.
>
> Inspection of packets sent by cifs.ko using wireshark show valid data for
> the SMB2_POSIX_CREATE_CONTEXT is appended with the correct offset, but
> with an incorrect length of zero bytes. Fails with ksmbd+cifs.ko only as
> Windows server/client does not use POSIX extensions.
>
> Fix smb311_posix_mkdir() to set req->CreateContextsLength as part of
> appending the POSIX creation context to the request.
>
> Signed-off-by: Jethro Donaldson <devel@jro.nz>
> ---

Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>

Re: [PATCH] smb: client: fix zero length for mkdir POSIX create context

[Namjae Jeon]

On Tue, Apr 29, 2025 at 10:13 PM Jethro Donaldson <devel@jro.nz> wrote:
>
> smb: client: fix zero length for mkdir POSIX create context
>
> SMB create requests issued via smb311_posix_mkdir() have an incorrect
> length of zero bytes for the POSIX create context data. A ksmbd server
> rejects such requests and logs "cli req too short" causing mkdir to fail
> with "invalid argument" on the client side.
>
> Inspection of packets sent by cifs.ko using wireshark show valid data for
> the SMB2_POSIX_CREATE_CONTEXT is appended with the correct offset, but
> with an incorrect length of zero bytes. Fails with ksmbd+cifs.ko only as
> Windows server/client does not use POSIX extensions.
>
> Fix smb311_posix_mkdir() to set req->CreateContextsLength as part of
> appending the POSIX creation context to the request.
>
> Signed-off-by: Jethro Donaldson <devel@jro.nz>
Looks good to me.
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!