[PATCH 0/3] ksmbd: some potential bugfixes

[PATCH 0/3] ksmbd: some potential bugfixes

[Greg Kroah-Hartman]

I spent the time exercising some new fuzzing tools on the ksmbd and smb
code purely because it's something that is simple to set up and test
locally with virtual machines, and in doing so, potentially found some
minor problems for when you have an "untrusted" client.

Here's some fixes for what I happened to notice.  They pass my very
limited testing here, but please don't trust them at all and verify that
I'm not just making this all up before accepting them.

thanks!

greg k-h

Greg Kroah-Hartman (3):
  ksmbd: validate EaNameLength in smb2_get_ea()
  ksmbd: require 3 sub-authorities before reading sub_auth[2]
  ksmbd: fix mechToken leak when SPNEGO decode fails after token alloc

 fs/smb/server/connection.c | 1 +
 fs/smb/server/smb2pdu.c    | 7 ++++++-
 fs/smb/server/smbacl.c     | 3 ++-
 3 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.53.0

[PATCH 1/3] ksmbd: validate EaNameLength in smb2_get_ea()

[Greg Kroah-Hartman]

smb2_get_ea() reads ea_req->EaNameLength from the client request and
passes it directly to strncmp() as the comparison length without
verifying that the length of the name really is the size of the input
buffer recieved.

Fix this up by properly checking the size of the name based on the value
recieved and the overall size of the request, to prevent a later
strncmp() call to use the length as a "trusted" size of the buffer.
Without this check, uninitialized heap values might be slowly leaked to
the client.

Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Steve French <smfrench@gmail.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable <stable@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/smb/server/smb2pdu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 8e4cfdc0ba02..6f658dc20758 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -4716,6 +4716,11 @@ static int smb2_get_ea(struct ksmbd_work *work, struct ksmbd_file *fp,
 
 		ea_req = (struct smb2_ea_info_req *)((char *)req +
 						     le16_to_cpu(req->InputBufferOffset));
+
+		if (le32_to_cpu(req->InputBufferLength) <
+		    offsetof(struct smb2_ea_info_req, name) +
+		    ea_req->EaNameLength)
+			return -EINVAL;
 	} else {
 		/* need to send all EAs, if no specific EA is requested*/
 		if (le32_to_cpu(req->Flags) & SL_RETURN_SINGLE_ENTRY)
-- 
2.53.0

[PATCH 2/3] ksmbd: require 3 sub-authorities before reading sub_auth[2]

[Greg Kroah-Hartman]

parse_dacl() compares each ACE SID against sid_unix_NFS_mode and on
match reads sid.sub_auth[2] as the file mode.  If sid_unix_NFS_mode is
the prefix S-1-5-88-3 with num_subauth = 2 then compare_sids() compares
only min(num_subauth, 2) sub-authorities so a client SID with
num_subauth = 2 and sub_auth = {88, 3} will match.

If num_subauth = 2 and the ACE is placed at the very end of the security
descriptor, sub_auth[2] will be  4 bytes past end_of_acl.  The
out-of-band bytes will then be masked to the low 9 bits and applied as
the file's POSIX mode, probably not something that is good to have
happen.

Fix this up by forcing the SID to actually carry a third sub-authority
before reading it at all.

Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Steve French <smfrench@gmail.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable <stable@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/smb/server/smbacl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c
index c30d01877c41..061a305bf9c8 100644
--- a/fs/smb/server/smbacl.c
+++ b/fs/smb/server/smbacl.c
@@ -451,7 +451,8 @@ static void parse_dacl(struct mnt_idmap *idmap,
 		ppace[i]->access_req =
 			smb_map_generic_desired_access(ppace[i]->access_req);
 
-		if (!(compare_sids(&ppace[i]->sid, &sid_unix_NFS_mode))) {
+		if (ppace[i]->sid.num_subauth >= 3 &&
+		    !(compare_sids(&ppace[i]->sid, &sid_unix_NFS_mode))) {
 			fattr->cf_mode =
 				le32_to_cpu(ppace[i]->sid.sub_auth[2]);
 			break;
-- 
2.53.0

[PATCH 3/3] ksmbd: fix mechToken leak when SPNEGO decode fails after token alloc

[Greg Kroah-Hartman]

The kernel ASN.1 BER decoder calls action callbacks incrementally as it
walks the input.  When ksmbd_decode_negTokenInit() reaches the mechToken
[2] OCTET STRING element, ksmbd_neg_token_alloc() allocates
conn->mechToken immediately via kmemdup_nul().  If a later element in
the same blob is malformed, then the decoder will return nonzero after
the allocation is already live.  This could happen if mechListMIC [3]
overrunse the enclosing SEQUENCE.

decode_negotiation_token() then sets conn->use_spnego = false because
both the negTokenInit and negTokenTarg grammars failed.  The cleanup at
the bottom of smb2_sess_setup() is gated on use_spnego:

	if (conn->use_spnego && conn->mechToken) {
		kfree(conn->mechToken);
		conn->mechToken = NULL;
	}

so the kfree is skipped, causing the mechToken to never be freed.

This codepath is reachable pre-authentication, so untrusted clients can
cause slow memory leaks on a server without even being properly
authenticated.

Fix this up by not checking check for use_spnego, as it's not required,
so the memory will always be properly freed.  At the same time, always
free the memory in ksmbd_conn_free() incase some other failure path
forgot to free it.

Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Steve French <smfrench@gmail.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable <stable@kernel.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/smb/server/connection.c | 1 +
 fs/smb/server/smb2pdu.c    | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c
index 1bb2081c492c..26cfce344861 100644
--- a/fs/smb/server/connection.c
+++ b/fs/smb/server/connection.c
@@ -96,6 +96,7 @@ void ksmbd_conn_free(struct ksmbd_conn *conn)
 	xa_destroy(&conn->sessions);
 	kvfree(conn->request_buf);
 	kfree(conn->preauth_info);
+	kfree(conn->mechToken);
 	if (atomic_dec_and_test(&conn->refcnt)) {
 		conn->transport->ops->free_transport(conn->transport);
 		kfree(conn);
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 6f658dc20758..a344937595f4 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -1915,7 +1915,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
 	else if (rc)
 		rsp->hdr.Status = STATUS_LOGON_FAILURE;
 
-	if (conn->use_spnego && conn->mechToken) {
+	if (conn->mechToken) {
 		kfree(conn->mechToken);
 		conn->mechToken = NULL;
 	}
-- 
2.53.0

Re: [PATCH 0/3] ksmbd: some potential bugfixes

[Namjae Jeon]

On Mon, Apr 6, 2026 at 10:47 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> I spent the time exercising some new fuzzing tools on the ksmbd and smb
> code purely because it's something that is simple to set up and test
> locally with virtual machines, and in doing so, potentially found some
> minor problems for when you have an "untrusted" client.
>
> Here's some fixes for what I happened to notice.  They pass my very
> limited testing here, but please don't trust them at all and verify that
> I'm not just making this all up before accepting them.
>
> thanks!
>
> greg k-h
>
> Greg Kroah-Hartman (3):
>   ksmbd: validate EaNameLength in smb2_get_ea()
>   ksmbd: require 3 sub-authorities before reading sub_auth[2]
>   ksmbd: fix mechToken leak when SPNEGO decode fails after token alloc
Applied them to #ksmbd-for-next-next.
Thanks for the patches!

Re: [PATCH 0/3] ksmbd: some potential bugfixes

[Greg Kroah-Hartman]

On Mon, Apr 06, 2026 at 06:05:55PM -0500, Steve French wrote:
> Potentially some useful comments from Sashiko's AI review of this series:
> 
> https://sashiko.dev/#/patchset/2026040644-brussels-dab-6f99%40gregkh

Looks like it is pointing out other issues to potentially work on in the
future, but the current series seems to not have any review problems?

thanks,

greg k-h