[PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[paul.gortmaker]

From: Paul Gortmaker <paul.gortmaker@windriver.com>

This is a bit long, but I've never touched this code and all I can do is
compile test it.  So the below basically represents a capture of my
thought process in fixing this for the v5.15.y-stable branch.

I am hoping the folks who normally work with this code can double check
that I didn't get off-track somewhere...


CVE-2023-38431 points at commit 368ba06881c3 ("ksmbd: check the
validation of pdu_size in ksmbd_conn_handler_loop") as the fix:

https://nvd.nist.gov/vuln/detail/CVE-2023-38431

For convenience, here is a link to the fix:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/smb/server?id=368ba06881c3

It was added in v6.4

git describe --contains 368ba06881c3
v6.4-rc6~2^2~1

...and backported to several stable releases.  But just not v5.15.

Why not v5.15?  If we look at the code the fix patches with "git blame"
we get commit 0626e6641f6b4 ("cifsd: add server handler for central
processing and tranport layers")

$git describe --contains 0626e6641f6b4
v5.15-rc1~183^2~94

So that would have been the commit the "Fixes:" line would have pointed
at if it had one.

Applying the fix to v5.15 reveals two problems.  The 1st is a trivial
file rename (fs/smb/server/connection.c --> fs/ksmbd/connection.c for
v5.15) and then the commit *applies*.   The 2nd problem is only revealed
at compile time...

The compile fails because the v5.15 baseline does not have smb2_get_msg().
Where does that come from?

commit cb4517201b8acdb5fd5314494aaf86c267f22345
Author: Namjae Jeon <linkinjeon@kernel.org>
Date:   Wed Nov 3 08:08:44 2021 +0900

    ksmbd: remove smb2_buf_length in smb2_hdr

git describe --contains cb4517201b8a
v5.16-rc1~21^2~6

So now we see why v5.15 didn't get a linux-stable backport by default.
In cb4517201b8a we see:

+static inline void *smb2_get_msg(void *buf)
+{
+       return buf + 4;
+}

However we can't just take that context free of the rest of the commit,
and then glue it into v5.15.  The whole reason the function exists is
because a length field of 4 was removed from the front of a struct.
If we look at the typical changes the struct change caused, we see:

-       struct smb2_hdr *rcv_hdr2 = work->request_buf;
+       struct smb2_hdr *rcv_hdr2 = smb2_get_msg(work->request_buf);

If we manually inline that, we obviously get:

-       struct smb2_hdr *rcv_hdr2 = work->request_buf;
+       struct smb2_hdr *rcv_hdr2 = work->request_buf + 4;

Now consider the lines added in the fix which is post struct reduction:

+#define SMB2_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr) + 4)

+               if (((struct smb2_hdr *)smb2_get_msg(conn->request_buf))->ProtocolId ==
+                   SMB2_PROTO_NUMBER) {
+                       if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
+                               break;
+               }

...and if we inline/expand everything, we get:

+               if (((struct smb2_hdr *)(conn->request_buf + 4))->ProtocolId ==
+                   SMB2_PROTO_NUMBER) {
+                       if (pdu_size < (sizeof(struct smb2_hdr) + 4))
+                               break;
+               }

And so, by extension the v5.15 code, which is *pre* struct reduction, would
simply not have the "+4" and hence be:

+               if (((struct smb2_hdr *)(conn->request_buf))->ProtocolId ==
+                   SMB2_PROTO_NUMBER) {
+                       if (pdu_size < (sizeof(struct smb2_hdr)))
+                               break;
+               }

If we then put the macro back (without the 4), the v5.15 version would be:

+#define SMB2_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr))

+               if (((struct smb2_hdr *)(conn->request_buf))->ProtocolId ==
+                   SMB2_PROTO_NUMBER) {
+                       if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
+                               break;
+               }

And so that is what I convinced myself is right to put in the backport.

If you read/reviewed this far - thanks!
Paul.

---

Namjae Jeon (1):
  ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop

 fs/ksmbd/connection.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

-- 
2.40.0

[PATCH 1/1] ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop

[paul.gortmaker]

From: Namjae Jeon <linkinjeon@kernel.org>

commit 368ba06881c395f1c9a7ba22203cf8d78b4addc0 upstream.

The length field of netbios header must be greater than the SMB header
sizes(smb1 or smb2 header), otherwise the packet is an invalid SMB packet.

If `pdu_size` is 0, ksmbd allocates a 4 bytes chunk to `conn->request_buf`.
In the function `get_smb2_cmd_val` ksmbd will read cmd from
`rcv_hdr->Command`, which is `conn->request_buf + 12`, causing the KASAN
detector to print the following error message:

[    7.205018] BUG: KASAN: slab-out-of-bounds in get_smb2_cmd_val+0x45/0x60
[    7.205423] Read of size 2 at addr ffff8880062d8b50 by task ksmbd:42632/248
...
[    7.207125]  <TASK>
[    7.209191]  get_smb2_cmd_val+0x45/0x60
[    7.209426]  ksmbd_conn_enqueue_request+0x3a/0x100
[    7.209712]  ksmbd_server_process_request+0x72/0x160
[    7.210295]  ksmbd_conn_handler_loop+0x30c/0x550
[    7.212280]  kthread+0x160/0x190
[    7.212762]  ret_from_fork+0x1f/0x30
[    7.212981]  </TASK>

Cc: stable@vger.kernel.org
Reported-by: Chih-Yen Chang <cc85nod@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
[PG: fs/smb/server/connection.c --> fs/ksmbd/connection.c for v5.15.
 Also no smb2_get_msg() as no +4 from cb4517201b8a in v5.15 baseline.]
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 fs/ksmbd/connection.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index cab274b77727..7f6fdafa240d 100644
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -263,6 +263,9 @@ bool ksmbd_conn_alive(struct ksmbd_conn *conn)
 	return true;
 }
 
+#define SMB1_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb_hdr))
+#define SMB2_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr))
+
 /**
  * ksmbd_conn_handler_loop() - session thread to listen on new smb requests
  * @p:		connection instance
@@ -319,6 +322,9 @@ int ksmbd_conn_handler_loop(void *p)
 		if (pdu_size > MAX_STREAM_PROT_LEN)
 			break;
 
+		if (pdu_size < SMB1_MIN_SUPPORTED_HEADER_SIZE)
+			break;
+
 		/* 4 for rfc1002 length field */
 		/* 1 for implied bcc[0] */
 		size = pdu_size + 4 + 1;
@@ -346,6 +352,12 @@ int ksmbd_conn_handler_loop(void *p)
 			continue;
 		}
 
+		if (((struct smb2_hdr *)(conn->request_buf))->ProtocolId ==
+		    SMB2_PROTO_NUMBER) {
+			if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
+				break;
+		}
+
 		if (!default_conn_ops.process_fn) {
 			pr_err("No connection request callback\n");
 			break;
-- 
2.40.0

Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Greg KH]

On Tue, Dec 12, 2023 at 01:47:44PM -0500, paul.gortmaker@windriver.com wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> This is a bit long, but I've never touched this code and all I can do is
> compile test it.  So the below basically represents a capture of my
> thought process in fixing this for the v5.15.y-stable branch.

Nice work, but really, given that there are _SO_ many ksmb patches that
have NOT been backported to 5.15.y, I would strongly recommend that we
just mark the thing as depending on BROKEN there for now as your one
backport here is not going to make a dent in the fixes that need to be
applied there to resolve the known issues that the codebase currently
has resolved in newer kernels.

Do you use this codebase on 5.15.y?  What drove you to want to backport
this, just the presence of a random CVE identifier?  If that's all it
takes to get companies to actually do backports, maybe I should go
allocate more of them :)

thanks,

greg k-h

RE: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Steven French]

Out of curiosity, has there been an alternative approach for some backports, where someone backports most fixes and features (and safe cleanup) but does not backport any of the changesets which have dependencies outside the module (e.g. VFS changes, netfs or mm changes etc.)  to reduce patch dependency risk (ie 70-80% backport instead of the typical 10-20% that are picked up by stable)?

For example, we (on the client) ran into issues with 5.15 kernel (for the client) missing so many important fixes and features (and sometimes hard to distinguish when a new feature is also a 'fix') that I did a "full backport" for cifs.ko again a few months ago for 5.15 (leaving out about 10% of the patches, those with dependencies or that would be risky).

There are arguments to be made for larger backports when test infrastructure is good and lots of good functional tests (due to risk of subtle dependencies when cherrypicking 1 patch out of 5 to backport).   In general, Namjae has access to excellent functional/regression suites for SMB server (not just from Samba "smbtorture") so it is theoretically possible to do larger "very safe" backports for ksmbd (or at least make these available as an alternative for users who hit serious roadblocks on older kernels), if the test automation were well trusted.   Is there a precedent for this?

-----Original Message-----
From: Greg KH <gregkh@linuxfoundation.org> 
Sent: Tuesday, December 12, 2023 2:05 PM
To: paul.gortmaker@windriver.com
Cc: Namjae Jeon <linkinjeon@kernel.org>; Steven French <Steven.French@microsoft.com>; stable@vger.kernel.org
Subject: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

On Tue, Dec 12, 2023 at 01:47:44PM -0500, paul.gortmaker@windriver.com wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> This is a bit long, but I've never touched this code and all I can do 
> is compile test it.  So the below basically represents a capture of my 
> thought process in fixing this for the v5.15.y-stable branch.

Nice work, but really, given that there are _SO_ many ksmb patches that have NOT been backported to 5.15.y, I would strongly recommend that we just mark the thing as depending on BROKEN there for now as your one backport here is not going to make a dent in the fixes that need to be applied there to resolve the known issues that the codebase currently has resolved in newer kernels.

Do you use this codebase on 5.15.y?  What drove you to want to backport this, just the presence of a random CVE identifier?  If that's all it takes to get companies to actually do backports, maybe I should go allocate more of them :)

thanks,

greg k-h

Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Paul Gortmaker]

[Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 12/12/2023 (Tue 21:04) Greg KH wrote:

> On Tue, Dec 12, 2023 at 01:47:44PM -0500, paul.gortmaker@windriver.com wrote:
> > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > 
> > This is a bit long, but I've never touched this code and all I can do is
> > compile test it.  So the below basically represents a capture of my
> > thought process in fixing this for the v5.15.y-stable branch.
> 
> Nice work, but really, given that there are _SO_ many ksmb patches that
> have NOT been backported to 5.15.y, I would strongly recommend that we
> just mark the thing as depending on BROKEN there for now as your one

I'd be 100% fine with that.  Can't speak for anyone else though.

> backport here is not going to make a dent in the fixes that need to be
> applied there to resolve the known issues that the codebase currently
> has resolved in newer kernels.
> 
> Do you use this codebase on 5.15.y?  What drove you to want to backport

I don't use it, and I don't know of anyone who does.

> this, just the presence of a random CVE identifier?  If that's all it
> takes to get companies to actually do backports, maybe I should go
> allocate more of them :)

I wouldn't have normally touched it but someone else was looking at
doing the backport and essentially declared it "too hard" and so I
couldn't let that stand.  :-P

Paul.
--

> 
> thanks,
> 
> greg k-h

Re: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Paul Gortmaker]

[RE: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 12/12/2023 (Tue 20:13) Steven French wrote:

> Out of curiosity, has there been an alternative approach for some backports, where someone backports most fixes and features (and safe cleanup) but does not backport any of the changesets which have dependencies outside the module (e.g. VFS changes, netfs or mm changes etc.)  to reduce patch dependency risk (ie 70-80% backport instead of the typical 10-20% that are picked up by stable)?
> 
> For example, we (on the client) ran into issues with 5.15 kernel (for the client) missing so many important fixes and features (and sometimes hard to distinguish when a new feature is also a 'fix') that I did a "full backport" for cifs.ko again a few months ago for 5.15 (leaving out about 10% of the patches, those with dependencies or that would be risky).
> 
> There are arguments to be made for larger backports when test infrastructure is good and lots of good functional tests (due to risk of subtle dependencies when cherrypicking 1 patch out of 5 to backport).   In general, Namjae has access to excellent functional/regression suites for SMB server (not just from Samba "smbtorture") so it is theoretically possible to do larger "very safe" backports for ksmbd (or at least make these available as an alternative for users who hit serious roadblocks on older kernels), if the test automation were well trusted.   Is there a precedent for this?

Larger backports like that can make sense for a target audience who are
invested in some feature but don't want to move anything else - then it
becomes a deliverable in itself, typically from a professional services
group.  But linux-stable is definitely not the place for things like that.

Paul.
--

> 
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org> 
> Sent: Tuesday, December 12, 2023 2:05 PM
> To: paul.gortmaker@windriver.com
> Cc: Namjae Jeon <linkinjeon@kernel.org>; Steven French <Steven.French@microsoft.com>; stable@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431
> 
> On Tue, Dec 12, 2023 at 01:47:44PM -0500, paul.gortmaker@windriver.com wrote:
> > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > 
> > This is a bit long, but I've never touched this code and all I can do 
> > is compile test it.  So the below basically represents a capture of my 
> > thought process in fixing this for the v5.15.y-stable branch.
> 
> Nice work, but really, given that there are _SO_ many ksmb patches that have NOT been backported to 5.15.y, I would strongly recommend that we just mark the thing as depending on BROKEN there for now as your one backport here is not going to make a dent in the fixes that need to be applied there to resolve the known issues that the codebase currently has resolved in newer kernels.
> 
> Do you use this codebase on 5.15.y?  What drove you to want to backport this, just the presence of a random CVE identifier?  If that's all it takes to get companies to actually do backports, maybe I should go allocate more of them :)
> 
> thanks,
> 
> greg k-h

RE: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Steven French]

> Larger backports like that can make sense for a target audience who are invested in some feature but don't want to move anything else - then it becomes a deliverable in itself, typically from a professional services group.  But linux-stable is definitely not the place for things like that.

I wasn't so much thinking about it for stable, but for cases where it is safer to use an alternative to stable for a particular module (and for a particular older kernel).  For active components (where dozens of patches go in each release) stable can carry risks because of subtle patch dependencies.    Maybe not as much issue for the cifs.ko and ksmbd.ko cases because there are excellent testcase suites that could be automated for stable to catch the vast majority of stable backport problems for those, but that assumes that this (stable kernel component by component testing) is automated (which was a big topic of discussion at the most recent LSF/MM/Storage summit).   Obviously distros do this all the time, and backport more than stable would for the more active modules, but there are probably cases where Debian e.g. users would want a module that had various security features backported and so couldn't use stable (because stable would typically not include new features, even if critical)

Re: [PATCH 1/1] ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop

[Namjae Jeon]

2023-12-13 3:47 GMT+09:00, paul.gortmaker@windriver.com
<paul.gortmaker@windriver.com>:
> From: Namjae Jeon <linkinjeon@kernel.org>
>
> commit 368ba06881c395f1c9a7ba22203cf8d78b4addc0 upstream.
>
> The length field of netbios header must be greater than the SMB header
> sizes(smb1 or smb2 header), otherwise the packet is an invalid SMB packet.
>
> If `pdu_size` is 0, ksmbd allocates a 4 bytes chunk to `conn->request_buf`.
> In the function `get_smb2_cmd_val` ksmbd will read cmd from
> `rcv_hdr->Command`, which is `conn->request_buf + 12`, causing the KASAN
> detector to print the following error message:
>
> [    7.205018] BUG: KASAN: slab-out-of-bounds in get_smb2_cmd_val+0x45/0x60
> [    7.205423] Read of size 2 at addr ffff8880062d8b50 by task
> ksmbd:42632/248
> ...
> [    7.207125]  <TASK>
> [    7.209191]  get_smb2_cmd_val+0x45/0x60
> [    7.209426]  ksmbd_conn_enqueue_request+0x3a/0x100
> [    7.209712]  ksmbd_server_process_request+0x72/0x160
> [    7.210295]  ksmbd_conn_handler_loop+0x30c/0x550
> [    7.212280]  kthread+0x160/0x190
> [    7.212762]  ret_from_fork+0x1f/0x30
> [    7.212981]  </TASK>
>
> Cc: stable@vger.kernel.org
> Reported-by: Chih-Yen Chang <cc85nod@gmail.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> [PG: fs/smb/server/connection.c --> fs/ksmbd/connection.c for v5.15.
>  Also no smb2_get_msg() as no +4 from cb4517201b8a in v5.15 baseline.]
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Looks good to me:)
Thanks for your work!

Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Namjae Jeon]

2023-12-13 3:47 GMT+09:00, paul.gortmaker@windriver.com
<paul.gortmaker@windriver.com>:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> This is a bit long, but I've never touched this code and all I can do is
> compile test it.  So the below basically represents a capture of my
> thought process in fixing this for the v5.15.y-stable branch.
>
> I am hoping the folks who normally work with this code can double check
> that I didn't get off-track somewhere...
>
>
> CVE-2023-38431 points at commit 368ba06881c3 ("ksmbd: check the
> validation of pdu_size in ksmbd_conn_handler_loop") as the fix:
>
> https://nvd.nist.gov/vuln/detail/CVE-2023-38431
>
> For convenience, here is a link to the fix:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/smb/server?id=368ba06881c3
>
> It was added in v6.4
>
> git describe --contains 368ba06881c3
> v6.4-rc6~2^2~1
>
> ...and backported to several stable releases.  But just not v5.15.
>
> Why not v5.15?  If we look at the code the fix patches with "git blame"
> we get commit 0626e6641f6b4 ("cifsd: add server handler for central
> processing and tranport layers")
>
> $git describe --contains 0626e6641f6b4
> v5.15-rc1~183^2~94
>
> So that would have been the commit the "Fixes:" line would have pointed
> at if it had one.
>
> Applying the fix to v5.15 reveals two problems.  The 1st is a trivial
> file rename (fs/smb/server/connection.c --> fs/ksmbd/connection.c for
> v5.15) and then the commit *applies*.   The 2nd problem is only revealed
> at compile time...
>
> The compile fails because the v5.15 baseline does not have smb2_get_msg().
> Where does that come from?
>
> commit cb4517201b8acdb5fd5314494aaf86c267f22345
> Author: Namjae Jeon <linkinjeon@kernel.org>
> Date:   Wed Nov 3 08:08:44 2021 +0900
>
>     ksmbd: remove smb2_buf_length in smb2_hdr
>
> git describe --contains cb4517201b8a
> v5.16-rc1~21^2~6
>
> So now we see why v5.15 didn't get a linux-stable backport by default.
> In cb4517201b8a we see:
>
> +static inline void *smb2_get_msg(void *buf)
> +{
> +       return buf + 4;
> +}
>
> However we can't just take that context free of the rest of the commit,
> and then glue it into v5.15.  The whole reason the function exists is
> because a length field of 4 was removed from the front of a struct.
> If we look at the typical changes the struct change caused, we see:
>
> -       struct smb2_hdr *rcv_hdr2 = work->request_buf;
> +       struct smb2_hdr *rcv_hdr2 = smb2_get_msg(work->request_buf);
>
> If we manually inline that, we obviously get:
>
> -       struct smb2_hdr *rcv_hdr2 = work->request_buf;
> +       struct smb2_hdr *rcv_hdr2 = work->request_buf + 4;
>
> Now consider the lines added in the fix which is post struct reduction:
>
> +#define SMB2_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr) + 4)
>
> +               if (((struct smb2_hdr
> *)smb2_get_msg(conn->request_buf))->ProtocolId ==
> +                   SMB2_PROTO_NUMBER) {
> +                       if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
> +                               break;
> +               }
>
> ...and if we inline/expand everything, we get:
>
> +               if (((struct smb2_hdr *)(conn->request_buf + 4))->ProtocolId
> ==
> +                   SMB2_PROTO_NUMBER) {
> +                       if (pdu_size < (sizeof(struct smb2_hdr) + 4))
> +                               break;
> +               }
>
> And so, by extension the v5.15 code, which is *pre* struct reduction, would
> simply not have the "+4" and hence be:
>
> +               if (((struct smb2_hdr *)(conn->request_buf))->ProtocolId ==
> +                   SMB2_PROTO_NUMBER) {
> +                       if (pdu_size < (sizeof(struct smb2_hdr)))
> +                               break;
> +               }
>
> If we then put the macro back (without the 4), the v5.15 version would be:
>
> +#define SMB2_MIN_SUPPORTED_HEADER_SIZE (sizeof(struct smb2_hdr))
>
> +               if (((struct smb2_hdr *)(conn->request_buf))->ProtocolId ==
> +                   SMB2_PROTO_NUMBER) {
> +                       if (pdu_size < SMB2_MIN_SUPPORTED_HEADER_SIZE)
> +                               break;
> +               }
>
> And so that is what I convinced myself is right to put in the backport.
>
Hi Paul,
> If you read/reviewed this far - thanks!
Your backport patch looks good :), I have checked it work fine.

Thanks for your work!
> Paul.
>
> ---
>
> Namjae Jeon (1):
>   ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop
>
>  fs/ksmbd/connection.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> --
> 2.40.0
>
>

Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Greg KH]

On Tue, Dec 12, 2023 at 03:45:55PM -0500, Paul Gortmaker wrote:
> [Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 12/12/2023 (Tue 21:04) Greg KH wrote:
> 
> > On Tue, Dec 12, 2023 at 01:47:44PM -0500, paul.gortmaker@windriver.com wrote:
> > > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > > 
> > > This is a bit long, but I've never touched this code and all I can do is
> > > compile test it.  So the below basically represents a capture of my
> > > thought process in fixing this for the v5.15.y-stable branch.
> > 
> > Nice work, but really, given that there are _SO_ many ksmb patches that
> > have NOT been backported to 5.15.y, I would strongly recommend that we
> > just mark the thing as depending on BROKEN there for now as your one
> 
> I'd be 100% fine with that.  Can't speak for anyone else though.
> 
> > backport here is not going to make a dent in the fixes that need to be
> > applied there to resolve the known issues that the codebase currently
> > has resolved in newer kernels.
> > 
> > Do you use this codebase on 5.15.y?  What drove you to want to backport
> 
> I don't use it, and I don't know of anyone who does.

Then why are you all backporting stuff for it?

If no one steps up, I'll just mark the thing as broken, it is _so_ far
behind in patches that it's just sad.

thanks,

greg k-h

Re: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Greg KH]

On Tue, Dec 12, 2023 at 08:13:37PM +0000, Steven French wrote:
> Out of curiosity, has there been an alternative approach for some
> backports, where someone backports most fixes and features (and safe
> cleanup) but does not backport any of the changesets which have
> dependencies outside the module (e.g. VFS changes, netfs or mm changes
> etc.)  to reduce patch dependency risk (ie 70-80% backport instead of
> the typical 10-20% that are picked up by stable)?
> 
> For example, we (on the client) ran into issues with 5.15 kernel (for
> the client) missing so many important fixes and features (and
> sometimes hard to distinguish when a new feature is also a 'fix') that
> I did a "full backport" for cifs.ko again a few months ago for 5.15
> (leaving out about 10% of the patches, those with dependencies or that
> would be risky).

We did take a "big backport/sync" for io_uring in 5.15.y a while ago, so
there is precident for this.

But really, is anyone even using this feature in 5.15.y anyway?  I don't
know of any major distro using 5.15.y any more, and Android systems
based on 5.15.y don't use this specific filesystem, so what is left?
Can we just mark it broken and be done with it?

thanks,

greg k-h

Re: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Namjae Jeon]

2023-12-13 23:36 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> On Tue, Dec 12, 2023 at 08:13:37PM +0000, Steven French wrote:
>> Out of curiosity, has there been an alternative approach for some
>> backports, where someone backports most fixes and features (and safe
>> cleanup) but does not backport any of the changesets which have
>> dependencies outside the module (e.g. VFS changes, netfs or mm changes
>> etc.)  to reduce patch dependency risk (ie 70-80% backport instead of
>> the typical 10-20% that are picked up by stable)?
>>
>> For example, we (on the client) ran into issues with 5.15 kernel (for
>> the client) missing so many important fixes and features (and
>> sometimes hard to distinguish when a new feature is also a 'fix') that
>> I did a "full backport" for cifs.ko again a few months ago for 5.15
>> (leaving out about 10% of the patches, those with dependencies or that
>> would be risky).
>
> We did take a "big backport/sync" for io_uring in 5.15.y a while ago, so
> there is precident for this.
>
> But really, is anyone even using this feature in 5.15.y anyway?  I don't
> know of any major distro using 5.15.y any more, and Android systems
> based on 5.15.y don't use this specific filesystem, so what is left?
> Can we just mark it broken and be done with it?
As I know, ksmbd is enable in 5.15 kernel of some distros(opensuse,
ubuntu, etc) except redhat. And users can use this feature. I will
make the time for ksmbd backporting job. To facilitate backport, Can I
submit clean-up patches for ksmbd of 5.15 kernel or only bug fixes are
allowed?

Thanks.
>
> thanks,
>
> greg k-h
>

Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Paul Gortmaker]

[Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 13/12/2023 (Wed 15:34) Greg KH wrote:

> On Tue, Dec 12, 2023 at 03:45:55PM -0500, Paul Gortmaker wrote:
> > [Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 12/12/2023 (Tue 21:04) Greg KH wrote:
> > 
> > > On Tue, Dec 12, 2023 at 01:47:44PM -0500, paul.gortmaker@windriver.com wrote:
> > > > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > > > 
> > > > This is a bit long, but I've never touched this code and all I can do is
> > > > compile test it.  So the below basically represents a capture of my
> > > > thought process in fixing this for the v5.15.y-stable branch.
> > > 
> > > Nice work, but really, given that there are _SO_ many ksmb patches that
> > > have NOT been backported to 5.15.y, I would strongly recommend that we
> > > just mark the thing as depending on BROKEN there for now as your one
> > 
> > I'd be 100% fine with that.  Can't speak for anyone else though.
> > 
> > > backport here is not going to make a dent in the fixes that need to be
> > > applied there to resolve the known issues that the codebase currently
> > > has resolved in newer kernels.
> > > 
> > > Do you use this codebase on 5.15.y?  What drove you to want to backport
> > 
> > I don't use it, and I don't know of anyone who does.
> 
> Then why are you all backporting stuff for it?

Firstly, you've cut the context where I already explained that I did it
because others said it couldn't be done.  Of all people, I am sure you
can respect that.

The Yocto Project still offers v5.15 as an option, and whenever I can, I
help out to advance the Yocto Project as time permits.  Ask Richard.

> If no one steps up, I'll just mark the thing as broken, it is _so_ far
> behind in patches that it's just sad.

Again, in this case - I have no problem with that - but as a note of
record -- whenever linux-stable removes a Kconfig, either explicitly or
by a depends on BROKEN - it does trigger fallout for some people.

The Yocto/OE does an audit on the Kconfig output looking for options
that were explicitly set (or un-set) by the user, or by base templates.
If they don't land in the final .config file -- it lets you know.

Paul.
--

> thanks,
> 
> greg k-h

Re: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Greg KH]

On Thu, Dec 14, 2023 at 08:31:44AM +0900, Namjae Jeon wrote:
> 2023-12-13 23:36 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> > On Tue, Dec 12, 2023 at 08:13:37PM +0000, Steven French wrote:
> >> Out of curiosity, has there been an alternative approach for some
> >> backports, where someone backports most fixes and features (and safe
> >> cleanup) but does not backport any of the changesets which have
> >> dependencies outside the module (e.g. VFS changes, netfs or mm changes
> >> etc.)  to reduce patch dependency risk (ie 70-80% backport instead of
> >> the typical 10-20% that are picked up by stable)?
> >>
> >> For example, we (on the client) ran into issues with 5.15 kernel (for
> >> the client) missing so many important fixes and features (and
> >> sometimes hard to distinguish when a new feature is also a 'fix') that
> >> I did a "full backport" for cifs.ko again a few months ago for 5.15
> >> (leaving out about 10% of the patches, those with dependencies or that
> >> would be risky).
> >
> > We did take a "big backport/sync" for io_uring in 5.15.y a while ago, so
> > there is precident for this.
> >
> > But really, is anyone even using this feature in 5.15.y anyway?  I don't
> > know of any major distro using 5.15.y any more, and Android systems
> > based on 5.15.y don't use this specific filesystem, so what is left?
> > Can we just mark it broken and be done with it?
> As I know, ksmbd is enable in 5.15 kernel of some distros(opensuse,
> ubuntu, etc) except redhat.

But do any of them actually use the 5.15.y kernel tree and take updates
from there?  That's the key thing here.

> And users can use this feature. I will
> make the time for ksmbd backporting job. To facilitate backport, Can I
> submit clean-up patches for ksmbd of 5.15 kernel or only bug fixes are
> allowed?

If a fix relies on an upstream cleanup, that's fine to take.

But first, find out if anyone is actually using this before you take the
time here.

thanks,

greg k-h

Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Greg KH]

On Wed, Dec 13, 2023 at 10:28:53PM -0500, Paul Gortmaker wrote:
> [Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 13/12/2023 (Wed 15:34) Greg KH wrote:
> 
> > On Tue, Dec 12, 2023 at 03:45:55PM -0500, Paul Gortmaker wrote:
> > > [Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 12/12/2023 (Tue 21:04) Greg KH wrote:
> > > 
> > > > On Tue, Dec 12, 2023 at 01:47:44PM -0500, paul.gortmaker@windriver.com wrote:
> > > > > From: Paul Gortmaker <paul.gortmaker@windriver.com>
> > > > > 
> > > > > This is a bit long, but I've never touched this code and all I can do is
> > > > > compile test it.  So the below basically represents a capture of my
> > > > > thought process in fixing this for the v5.15.y-stable branch.
> > > > 
> > > > Nice work, but really, given that there are _SO_ many ksmb patches that
> > > > have NOT been backported to 5.15.y, I would strongly recommend that we
> > > > just mark the thing as depending on BROKEN there for now as your one
> > > 
> > > I'd be 100% fine with that.  Can't speak for anyone else though.
> > > 
> > > > backport here is not going to make a dent in the fixes that need to be
> > > > applied there to resolve the known issues that the codebase currently
> > > > has resolved in newer kernels.
> > > > 
> > > > Do you use this codebase on 5.15.y?  What drove you to want to backport
> > > 
> > > I don't use it, and I don't know of anyone who does.
> > 
> > Then why are you all backporting stuff for it?
> 
> Firstly, you've cut the context where I already explained that I did it
> because others said it couldn't be done.  Of all people, I am sure you
> can respect that.

Sure, I saw that, but I didn't understand why someone was doing it in
the first place.

> The Yocto Project still offers v5.15 as an option, and whenever I can, I
> help out to advance the Yocto Project as time permits.  Ask Richard.

As an option, but is it recommended and does anyone actually use it
there?  Does yocto systems expect to use this kernel option for the
5.15 kernel?

> > If no one steps up, I'll just mark the thing as broken, it is _so_ far
> > behind in patches that it's just sad.
> 
> Again, in this case - I have no problem with that - but as a note of
> record -- whenever linux-stable removes a Kconfig, either explicitly or
> by a depends on BROKEN - it does trigger fallout for some people.

In what way?  Just having to update default config options?

> The Yocto/OE does an audit on the Kconfig output looking for options
> that were explicitly set (or un-set) by the user, or by base templates.
> If they don't land in the final .config file -- it lets you know.

So defconfig type checks?

thanks,

greg k-h

Re: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Namjae Jeon]

2023-12-14 17:05 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> On Thu, Dec 14, 2023 at 08:31:44AM +0900, Namjae Jeon wrote:
>> 2023-12-13 23:36 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
>> > On Tue, Dec 12, 2023 at 08:13:37PM +0000, Steven French wrote:
>> >> Out of curiosity, has there been an alternative approach for some
>> >> backports, where someone backports most fixes and features (and safe
>> >> cleanup) but does not backport any of the changesets which have
>> >> dependencies outside the module (e.g. VFS changes, netfs or mm changes
>> >> etc.)  to reduce patch dependency risk (ie 70-80% backport instead of
>> >> the typical 10-20% that are picked up by stable)?
>> >>
>> >> For example, we (on the client) ran into issues with 5.15 kernel (for
>> >> the client) missing so many important fixes and features (and
>> >> sometimes hard to distinguish when a new feature is also a 'fix') that
>> >> I did a "full backport" for cifs.ko again a few months ago for 5.15
>> >> (leaving out about 10% of the patches, those with dependencies or that
>> >> would be risky).
>> >
>> > We did take a "big backport/sync" for io_uring in 5.15.y a while ago,
>> > so
>> > there is precident for this.
>> >
>> > But really, is anyone even using this feature in 5.15.y anyway?  I
>> > don't
>> > know of any major distro using 5.15.y any more, and Android systems
>> > based on 5.15.y don't use this specific filesystem, so what is left?
>> > Can we just mark it broken and be done with it?
>> As I know, ksmbd is enable in 5.15 kernel of some distros(opensuse,
>> ubuntu, etc) except redhat.
>
> But do any of them actually use the 5.15.y kernel tree and take updates
> from there?  That's the key thing here.
Yes, openWRT guy said that openWRT use ksmbd module of stable 5.15.y
kernel for their NAS function.
The most recent major release, 23.05.x, uses the 5.15 kernel, and the
kernel version is updated in minor releases.
https://github.com/openwrt/openwrt/commit/95ebd609ae7bdcdb48c74ad93d747f24c94d4a07

https://downloads.openwrt.org/releases/23.05.2/targets/x86/64/kmods/5.15.137-1-47964456485559d992fe6f536131fc64/

https://downloads.openwrt.org/releases/23.05.2/targets/x86/64/kmods/5.15.137-1-47964456485559d992fe6f536131fc64/kmod-fs-ksmbd_5.15.137-1_x86_64.ipk

https://github.com/openwrt/openwrt/blob/fcf08d9db6a50a3ca6f0b64d105d975ab896cc35/package/kernel/linux/modules/fs.mk#L349

>
>> And users can use this feature. I will
>> make the time for ksmbd backporting job. To facilitate backport, Can I
>> submit clean-up patches for ksmbd of 5.15 kernel or only bug fixes are
>> allowed?
>
> If a fix relies on an upstream cleanup, that's fine to take.
>
> But first, find out if anyone is actually using this before you take the
> time here.
Okay:) Thanks for your answer.

>
> thanks,
>
> greg k-h
>

Re: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Greg KH]

On Thu, Dec 14, 2023 at 08:33:48PM +0900, Namjae Jeon wrote:
> 2023-12-14 17:05 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> > On Thu, Dec 14, 2023 at 08:31:44AM +0900, Namjae Jeon wrote:
> >> 2023-12-13 23:36 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> >> > On Tue, Dec 12, 2023 at 08:13:37PM +0000, Steven French wrote:
> >> >> Out of curiosity, has there been an alternative approach for some
> >> >> backports, where someone backports most fixes and features (and safe
> >> >> cleanup) but does not backport any of the changesets which have
> >> >> dependencies outside the module (e.g. VFS changes, netfs or mm changes
> >> >> etc.)  to reduce patch dependency risk (ie 70-80% backport instead of
> >> >> the typical 10-20% that are picked up by stable)?
> >> >>
> >> >> For example, we (on the client) ran into issues with 5.15 kernel (for
> >> >> the client) missing so many important fixes and features (and
> >> >> sometimes hard to distinguish when a new feature is also a 'fix') that
> >> >> I did a "full backport" for cifs.ko again a few months ago for 5.15
> >> >> (leaving out about 10% of the patches, those with dependencies or that
> >> >> would be risky).
> >> >
> >> > We did take a "big backport/sync" for io_uring in 5.15.y a while ago,
> >> > so
> >> > there is precident for this.
> >> >
> >> > But really, is anyone even using this feature in 5.15.y anyway?  I
> >> > don't
> >> > know of any major distro using 5.15.y any more, and Android systems
> >> > based on 5.15.y don't use this specific filesystem, so what is left?
> >> > Can we just mark it broken and be done with it?
> >> As I know, ksmbd is enable in 5.15 kernel of some distros(opensuse,
> >> ubuntu, etc) except redhat.
> >
> > But do any of them actually use the 5.15.y kernel tree and take updates
> > from there?  That's the key thing here.
> Yes, openWRT guy said that openWRT use ksmbd module of stable 5.15.y
> kernel for their NAS function.
> The most recent major release, 23.05.x, uses the 5.15 kernel, and the
> kernel version is updated in minor releases.
> https://github.com/openwrt/openwrt/commit/95ebd609ae7bdcdb48c74ad93d747f24c94d4a07
> 
> https://downloads.openwrt.org/releases/23.05.2/targets/x86/64/kmods/5.15.137-1-47964456485559d992fe6f536131fc64/
> 
> https://downloads.openwrt.org/releases/23.05.2/targets/x86/64/kmods/5.15.137-1-47964456485559d992fe6f536131fc64/kmod-fs-ksmbd_5.15.137-1_x86_64.ipk
> 
> https://github.com/openwrt/openwrt/blob/fcf08d9db6a50a3ca6f0b64d105d975ab896cc35/package/kernel/linux/modules/fs.mk#L349

Ok, thanks, that's good to know.  Also you might want to warn them that
it's missing loads of security fixes at this point in time and that they
might want to move to a newer kernel release :)

thanks,

greg k-h

Re: [EXTERNAL] Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Namjae Jeon]

2023-12-14 20:58 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> On Thu, Dec 14, 2023 at 08:33:48PM +0900, Namjae Jeon wrote:
>> 2023-12-14 17:05 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
>> > On Thu, Dec 14, 2023 at 08:31:44AM +0900, Namjae Jeon wrote:
>> >> 2023-12-13 23:36 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
>> >> > On Tue, Dec 12, 2023 at 08:13:37PM +0000, Steven French wrote:
>> >> >> Out of curiosity, has there been an alternative approach for some
>> >> >> backports, where someone backports most fixes and features (and
>> >> >> safe
>> >> >> cleanup) but does not backport any of the changesets which have
>> >> >> dependencies outside the module (e.g. VFS changes, netfs or mm
>> >> >> changes
>> >> >> etc.)  to reduce patch dependency risk (ie 70-80% backport instead
>> >> >> of
>> >> >> the typical 10-20% that are picked up by stable)?
>> >> >>
>> >> >> For example, we (on the client) ran into issues with 5.15 kernel
>> >> >> (for
>> >> >> the client) missing so many important fixes and features (and
>> >> >> sometimes hard to distinguish when a new feature is also a 'fix')
>> >> >> that
>> >> >> I did a "full backport" for cifs.ko again a few months ago for 5.15
>> >> >> (leaving out about 10% of the patches, those with dependencies or
>> >> >> that
>> >> >> would be risky).
>> >> >
>> >> > We did take a "big backport/sync" for io_uring in 5.15.y a while
>> >> > ago,
>> >> > so
>> >> > there is precident for this.
>> >> >
>> >> > But really, is anyone even using this feature in 5.15.y anyway?  I
>> >> > don't
>> >> > know of any major distro using 5.15.y any more, and Android systems
>> >> > based on 5.15.y don't use this specific filesystem, so what is left?
>> >> > Can we just mark it broken and be done with it?
>> >> As I know, ksmbd is enable in 5.15 kernel of some distros(opensuse,
>> >> ubuntu, etc) except redhat.
>> >
>> > But do any of them actually use the 5.15.y kernel tree and take updates
>> > from there?  That's the key thing here.
>> Yes, openWRT guy said that openWRT use ksmbd module of stable 5.15.y
>> kernel for their NAS function.
>> The most recent major release, 23.05.x, uses the 5.15 kernel, and the
>> kernel version is updated in minor releases.
>> https://github.com/openwrt/openwrt/commit/95ebd609ae7bdcdb48c74ad93d747f24c94d4a07
>>
>> https://downloads.openwrt.org/releases/23.05.2/targets/x86/64/kmods/5.15.137-1-47964456485559d992fe6f536131fc64/
>>
>> https://downloads.openwrt.org/releases/23.05.2/targets/x86/64/kmods/5.15.137-1-47964456485559d992fe6f536131fc64/kmod-fs-ksmbd_5.15.137-1_x86_64.ipk
>>
>> https://github.com/openwrt/openwrt/blob/fcf08d9db6a50a3ca6f0b64d105d975ab896cc35/package/kernel/linux/modules/fs.mk#L349
>
> Ok, thanks, that's good to know.  Also you might want to warn them that
> it's missing loads of security fixes at this point in time and that they
> might want to move to a newer kernel release :)
Okay, I will.
And I will check ksmbd in 6.1 LTS kernel as well as 5.15.
Thanks!
>
> thanks,
>
> greg k-h
>

Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431

[Paul Gortmaker]

[Re: [PATCH 0/1] RFC: linux-5.15.y ksmbd backport for CVE-2023-38431] On 14/12/2023 (Thu 09:20) Greg KH wrote:

[...]

> > > If no one steps up, I'll just mark the thing as broken, it is _so_ far
> > > behind in patches that it's just sad.
> > 
> > Again, in this case - I have no problem with that - but as a note of
> > record -- whenever linux-stable removes a Kconfig, either explicitly or
> > by a depends on BROKEN - it does trigger fallout for some people.
> 
> In what way?  Just having to update default config options?
> 
> > The Yocto/OE does an audit on the Kconfig output looking for options
> > that were explicitly set (or un-set) by the user, or by base templates.
> > If they don't land in the final .config file -- it lets you know.
> 
> So defconfig type checks?

Here is a recent example.

https://lists.yoctoproject.org/g/linux-yocto/message/13387

I am not saying linux-stable is wrong to do these kinds of changes, I'm
just saying there is an impact that people might not be aware of.

Paul.
--

> 
> thanks,
> 
> greg k-h

Re: [PATCH 1/1] ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop

[Greg KH]

On Tue, Dec 12, 2023 at 01:47:45PM -0500, paul.gortmaker@windriver.com wrote:
> From: Namjae Jeon <linkinjeon@kernel.org>
> 
> commit 368ba06881c395f1c9a7ba22203cf8d78b4addc0 upstream.
> 
> The length field of netbios header must be greater than the SMB header
> sizes(smb1 or smb2 header), otherwise the packet is an invalid SMB packet.
> 
> If `pdu_size` is 0, ksmbd allocates a 4 bytes chunk to `conn->request_buf`.
> In the function `get_smb2_cmd_val` ksmbd will read cmd from
> `rcv_hdr->Command`, which is `conn->request_buf + 12`, causing the KASAN
> detector to print the following error message:
> 
> [    7.205018] BUG: KASAN: slab-out-of-bounds in get_smb2_cmd_val+0x45/0x60
> [    7.205423] Read of size 2 at addr ffff8880062d8b50 by task ksmbd:42632/248
> ...
> [    7.207125]  <TASK>
> [    7.209191]  get_smb2_cmd_val+0x45/0x60
> [    7.209426]  ksmbd_conn_enqueue_request+0x3a/0x100
> [    7.209712]  ksmbd_server_process_request+0x72/0x160
> [    7.210295]  ksmbd_conn_handler_loop+0x30c/0x550
> [    7.212280]  kthread+0x160/0x190
> [    7.212762]  ret_from_fork+0x1f/0x30
> [    7.212981]  </TASK>
> 
> Cc: stable@vger.kernel.org
> Reported-by: Chih-Yen Chang <cc85nod@gmail.com>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> [PG: fs/smb/server/connection.c --> fs/ksmbd/connection.c for v5.15.
>  Also no smb2_get_msg() as no +4 from cb4517201b8a in v5.15 baseline.]
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  fs/ksmbd/connection.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Now queued up, thanks.

greg k-h

Re: [PATCH 1/1] ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop

[Namjae Jeon]

2023-12-18 19:38 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> On Tue, Dec 12, 2023 at 01:47:45PM -0500, paul.gortmaker@windriver.com
> wrote:
>> From: Namjae Jeon <linkinjeon@kernel.org>
>>
>> commit 368ba06881c395f1c9a7ba22203cf8d78b4addc0 upstream.
>>
>> The length field of netbios header must be greater than the SMB header
>> sizes(smb1 or smb2 header), otherwise the packet is an invalid SMB
>> packet.
>>
>> If `pdu_size` is 0, ksmbd allocates a 4 bytes chunk to
>> `conn->request_buf`.
>> In the function `get_smb2_cmd_val` ksmbd will read cmd from
>> `rcv_hdr->Command`, which is `conn->request_buf + 12`, causing the KASAN
>> detector to print the following error message:
>>
>> [    7.205018] BUG: KASAN: slab-out-of-bounds in
>> get_smb2_cmd_val+0x45/0x60
>> [    7.205423] Read of size 2 at addr ffff8880062d8b50 by task
>> ksmbd:42632/248
>> ...
>> [    7.207125]  <TASK>
>> [    7.209191]  get_smb2_cmd_val+0x45/0x60
>> [    7.209426]  ksmbd_conn_enqueue_request+0x3a/0x100
>> [    7.209712]  ksmbd_server_process_request+0x72/0x160
>> [    7.210295]  ksmbd_conn_handler_loop+0x30c/0x550
>> [    7.212280]  kthread+0x160/0x190
>> [    7.212762]  ret_from_fork+0x1f/0x30
>> [    7.212981]  </TASK>
>>
>> Cc: stable@vger.kernel.org
>> Reported-by: Chih-Yen Chang <cc85nod@gmail.com>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> Signed-off-by: Steve French <stfrench@microsoft.com>
>> [PG: fs/smb/server/connection.c --> fs/ksmbd/connection.c for v5.15.
>>  Also no smb2_get_msg() as no +4 from cb4517201b8a in v5.15 baseline.]
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>  fs/ksmbd/connection.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>
> Now queued up, thanks.
Could you please remove this patch in your queue ?
This patches in my ksmbd backport queue and Since I have backported
all patches(between 5.16 ~ 6.7-rc1), I included original patch that do
not need to be changed unlike this patch.

https://github.com/namjaejeon/stable-linux-5.15-ksmbd

I am testing them before sending it. I plan to send all patches within
this week.

Thanks.
>
> greg k-h
>

Re: [PATCH 1/1] ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loop

[Greg KH]

On Mon, Dec 18, 2023 at 08:28:17PM +0900, Namjae Jeon wrote:
> 2023-12-18 19:38 GMT+09:00, Greg KH <gregkh@linuxfoundation.org>:
> > On Tue, Dec 12, 2023 at 01:47:45PM -0500, paul.gortmaker@windriver.com
> > wrote:
> >> From: Namjae Jeon <linkinjeon@kernel.org>
> >>
> >> commit 368ba06881c395f1c9a7ba22203cf8d78b4addc0 upstream.
> >>
> >> The length field of netbios header must be greater than the SMB header
> >> sizes(smb1 or smb2 header), otherwise the packet is an invalid SMB
> >> packet.
> >>
> >> If `pdu_size` is 0, ksmbd allocates a 4 bytes chunk to
> >> `conn->request_buf`.
> >> In the function `get_smb2_cmd_val` ksmbd will read cmd from
> >> `rcv_hdr->Command`, which is `conn->request_buf + 12`, causing the KASAN
> >> detector to print the following error message:
> >>
> >> [    7.205018] BUG: KASAN: slab-out-of-bounds in
> >> get_smb2_cmd_val+0x45/0x60
> >> [    7.205423] Read of size 2 at addr ffff8880062d8b50 by task
> >> ksmbd:42632/248
> >> ...
> >> [    7.207125]  <TASK>
> >> [    7.209191]  get_smb2_cmd_val+0x45/0x60
> >> [    7.209426]  ksmbd_conn_enqueue_request+0x3a/0x100
> >> [    7.209712]  ksmbd_server_process_request+0x72/0x160
> >> [    7.210295]  ksmbd_conn_handler_loop+0x30c/0x550
> >> [    7.212280]  kthread+0x160/0x190
> >> [    7.212762]  ret_from_fork+0x1f/0x30
> >> [    7.212981]  </TASK>
> >>
> >> Cc: stable@vger.kernel.org
> >> Reported-by: Chih-Yen Chang <cc85nod@gmail.com>
> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> >> Signed-off-by: Steve French <stfrench@microsoft.com>
> >> [PG: fs/smb/server/connection.c --> fs/ksmbd/connection.c for v5.15.
> >>  Also no smb2_get_msg() as no +4 from cb4517201b8a in v5.15 baseline.]
> >> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> >> ---
> >>  fs/ksmbd/connection.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >
> > Now queued up, thanks.
> Could you please remove this patch in your queue ?
> This patches in my ksmbd backport queue and Since I have backported
> all patches(between 5.16 ~ 6.7-rc1), I included original patch that do
> not need to be changed unlike this patch.
> 
> https://github.com/namjaejeon/stable-linux-5.15-ksmbd
> 
> I am testing them before sending it. I plan to send all patches within
> this week.

Sure, will drop it now, thanks.

greg k-h