* [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr
@ 2025-12-18 14:48 David Howells
2025-12-18 15:09 ` ChenXiaoSong
2025-12-18 16:21 ` Namjae Jeon
0 siblings, 2 replies; 7+ messages in thread
From: David Howells @ 2025-12-18 14:48 UTC (permalink / raw)
To: Namjae Jeon, Steve French
Cc: dhowells, Sergey Senozhatsky, Tom Talpey, Paulo Alcantara,
Shyam Prasad N, linux-cifs, netfs, linux-fsdevel, linux-kernel
Hi Namjae,
Does this (untested) patch fix the problem for you?
David
---
The commit that removed the RFC1002 header from struct smb_hdr didn't also
fix the places in ksmbd that use it in order to provide graceful rejection
of SMB1 protocol requests.
Fixes: 83bfbd0bb902 ("cifs: Remove the RFC1002 header from smb_hdr")
Reported-by: Namjae Jeon <linkinjeon@kernel.org>
Link: https://lore.kernel.org/r/CAKYAXd9Ju4MFkkH5Jxfi1mO0AWEr=R35M3vQ_Xa7Yw34JoNZ0A@mail.gmail.com/
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steve French <sfrench@samba.org>
cc: Sergey Senozhatsky <senozhatsky@chromium.org>
cc: Tom Talpey <tom@talpey.com>
cc: Paulo Alcantara <pc@manguebit.org>
cc: Shyam Prasad N <sprasad@microsoft.com>
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/smb/server/server.c | 2 +-
fs/smb/server/smb_common.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
index 3cea16050e4f..bedc8390b6db 100644
--- a/fs/smb/server/server.c
+++ b/fs/smb/server/server.c
@@ -95,7 +95,7 @@ static inline int check_conn_state(struct ksmbd_work *work)
if (ksmbd_conn_exiting(work->conn) ||
ksmbd_conn_need_reconnect(work->conn)) {
- rsp_hdr = work->response_buf;
+ rsp_hdr = smb2_get_msg(work->response_buf);
rsp_hdr->Status.CifsError = STATUS_CONNECTION_DISCONNECTED;
return 1;
}
diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index b23203a1c286..d6084580b59d 100644
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -140,7 +140,7 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
return ksmbd_smb2_check_message(work);
- hdr = work->request_buf;
+ hdr = smb2_get_msg(work->request_buf);
if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
hdr->Command == SMB_COM_NEGOTIATE) {
work->conn->outstanding_credits++;
@@ -278,7 +278,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
req->DialectCount);
}
- proto = *(__le32 *)((struct smb_hdr *)buf)->Protocol;
if (proto == SMB1_PROTO_NUMBER) {
struct smb_negotiate_req *req;
@@ -320,8 +319,8 @@ static u16 get_smb1_cmd_val(struct ksmbd_work *work)
*/
static int init_smb1_rsp_hdr(struct ksmbd_work *work)
{
- struct smb_hdr *rsp_hdr = (struct smb_hdr *)work->response_buf;
- struct smb_hdr *rcv_hdr = (struct smb_hdr *)work->request_buf;
+ struct smb_hdr *rsp_hdr = (struct smb_hdr *)smb2_get_msg(work->response_buf);
+ struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(work->request_buf);
rsp_hdr->Command = SMB_COM_NEGOTIATE;
*(__le32 *)rsp_hdr->Protocol = SMB1_PROTO_NUMBER;
@@ -412,9 +411,10 @@ static int init_smb1_server(struct ksmbd_conn *conn)
int ksmbd_init_smb_server(struct ksmbd_conn *conn)
{
+ struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(conn->request_buf);
__le32 proto;
- proto = *(__le32 *)((struct smb_hdr *)conn->request_buf)->Protocol;
+ proto = *(__le32 *)rcv_hdr->Protocol;
if (conn->need_neg == false) {
if (proto == SMB1_PROTO_NUMBER)
return -EINVAL;
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr
2025-12-18 14:48 [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr David Howells
@ 2025-12-18 15:09 ` ChenXiaoSong
2025-12-18 15:46 ` ChenXiaoSong
2025-12-18 16:01 ` David Howells
2025-12-18 16:21 ` Namjae Jeon
1 sibling, 2 replies; 7+ messages in thread
From: ChenXiaoSong @ 2025-12-18 15:09 UTC (permalink / raw)
To: David Howells, Namjae Jeon, Steve French
Cc: Sergey Senozhatsky, Tom Talpey, Paulo Alcantara, Shyam Prasad N,
linux-cifs, netfs, linux-fsdevel, linux-kernel
`ksmbd_conn_handler_loop()` calls `get_rfc1002_len()`. Does this need to
be updated as well?
Thanks,
ChenXiaoSong.
On 12/18/25 10:48 PM, David Howells wrote:
> Hi Namjae,
>
> Does this (untested) patch fix the problem for you?
>
> David
> ---
> The commit that removed the RFC1002 header from struct smb_hdr didn't also
> fix the places in ksmbd that use it in order to provide graceful rejection
> of SMB1 protocol requests.
>
> Fixes: 83bfbd0bb902 ("cifs: Remove the RFC1002 header from smb_hdr")
> Reported-by: Namjae Jeon <linkinjeon@kernel.org>
> Link: https://lore.kernel.org/r/CAKYAXd9Ju4MFkkH5Jxfi1mO0AWEr=R35M3vQ_Xa7Yw34JoNZ0A@mail.gmail.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Steve French <sfrench@samba.org>
> cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> cc: Tom Talpey <tom@talpey.com>
> cc: Paulo Alcantara <pc@manguebit.org>
> cc: Shyam Prasad N <sprasad@microsoft.com>
> cc: linux-cifs@vger.kernel.org
> cc: netfs@lists.linux.dev
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/smb/server/server.c | 2 +-
> fs/smb/server/smb_common.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
> index 3cea16050e4f..bedc8390b6db 100644
> --- a/fs/smb/server/server.c
> +++ b/fs/smb/server/server.c
> @@ -95,7 +95,7 @@ static inline int check_conn_state(struct ksmbd_work *work)
>
> if (ksmbd_conn_exiting(work->conn) ||
> ksmbd_conn_need_reconnect(work->conn)) {
> - rsp_hdr = work->response_buf;
> + rsp_hdr = smb2_get_msg(work->response_buf);
> rsp_hdr->Status.CifsError = STATUS_CONNECTION_DISCONNECTED;
> return 1;
> }
> diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
> index b23203a1c286..d6084580b59d 100644
> --- a/fs/smb/server/smb_common.c
> +++ b/fs/smb/server/smb_common.c
> @@ -140,7 +140,7 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
> if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
> return ksmbd_smb2_check_message(work);
>
> - hdr = work->request_buf;
> + hdr = smb2_get_msg(work->request_buf);
> if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
> hdr->Command == SMB_COM_NEGOTIATE) {
> work->conn->outstanding_credits++;
> @@ -278,7 +278,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
> req->DialectCount);
> }
>
> - proto = *(__le32 *)((struct smb_hdr *)buf)->Protocol;
> if (proto == SMB1_PROTO_NUMBER) {
> struct smb_negotiate_req *req;
>
> @@ -320,8 +319,8 @@ static u16 get_smb1_cmd_val(struct ksmbd_work *work)
> */
> static int init_smb1_rsp_hdr(struct ksmbd_work *work)
> {
> - struct smb_hdr *rsp_hdr = (struct smb_hdr *)work->response_buf;
> - struct smb_hdr *rcv_hdr = (struct smb_hdr *)work->request_buf;
> + struct smb_hdr *rsp_hdr = (struct smb_hdr *)smb2_get_msg(work->response_buf);
> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(work->request_buf);
>
> rsp_hdr->Command = SMB_COM_NEGOTIATE;
> *(__le32 *)rsp_hdr->Protocol = SMB1_PROTO_NUMBER;
> @@ -412,9 +411,10 @@ static int init_smb1_server(struct ksmbd_conn *conn)
>
> int ksmbd_init_smb_server(struct ksmbd_conn *conn)
> {
> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(conn->request_buf);
> __le32 proto;
>
> - proto = *(__le32 *)((struct smb_hdr *)conn->request_buf)->Protocol;
> + proto = *(__le32 *)rcv_hdr->Protocol;
> if (conn->need_neg == false) {
> if (proto == SMB1_PROTO_NUMBER)
> return -EINVAL;
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr
2025-12-18 15:09 ` ChenXiaoSong
@ 2025-12-18 15:46 ` ChenXiaoSong
2025-12-18 15:55 ` Namjae Jeon
2025-12-18 16:01 ` David Howells
1 sibling, 1 reply; 7+ messages in thread
From: ChenXiaoSong @ 2025-12-18 15:46 UTC (permalink / raw)
To: David Howells, Namjae Jeon, Steve French
Cc: Sergey Senozhatsky, Tom Talpey, Paulo Alcantara, Shyam Prasad N,
linux-cifs, netfs, linux-fsdevel, linux-kernel
Hi David,
Since the size of `struct smb_hdr` has changed, the value of
`SMB1_MIN_SUPPORTED_HEADER_SIZE` should also be updated to
`(sizeof(struct smb_hdr) + 4)`. `SMB1_MIN_SUPPORTED_HEADER_SIZE` is used
in `ksmbd_conn_handler_loop()`.
Thanks,
ChenXiaoSong.
On 12/18/25 11:09 PM, ChenXiaoSong wrote:
> `ksmbd_conn_handler_loop()` calls `get_rfc1002_len()`. Does this need to
> be updated as well?
>
> Thanks,
> ChenXiaoSong.
>
> On 12/18/25 10:48 PM, David Howells wrote:
>> Hi Namjae,
>>
>> Does this (untested) patch fix the problem for you?
>>
>> David
>> ---
>> The commit that removed the RFC1002 header from struct smb_hdr didn't
>> also
>> fix the places in ksmbd that use it in order to provide graceful
>> rejection
>> of SMB1 protocol requests.
>>
>> Fixes: 83bfbd0bb902 ("cifs: Remove the RFC1002 header from smb_hdr")
>> Reported-by: Namjae Jeon <linkinjeon@kernel.org>
>> Link: https://lore.kernel.org/r/
>> CAKYAXd9Ju4MFkkH5Jxfi1mO0AWEr=R35M3vQ_Xa7Yw34JoNZ0A@mail.gmail.com/
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> cc: Steve French <sfrench@samba.org>
>> cc: Sergey Senozhatsky <senozhatsky@chromium.org>
>> cc: Tom Talpey <tom@talpey.com>
>> cc: Paulo Alcantara <pc@manguebit.org>
>> cc: Shyam Prasad N <sprasad@microsoft.com>
>> cc: linux-cifs@vger.kernel.org
>> cc: netfs@lists.linux.dev
>> cc: linux-fsdevel@vger.kernel.org
>> ---
>> fs/smb/server/server.c | 2 +-
>> fs/smb/server/smb_common.c | 10 +++++-----
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
>> index 3cea16050e4f..bedc8390b6db 100644
>> --- a/fs/smb/server/server.c
>> +++ b/fs/smb/server/server.c
>> @@ -95,7 +95,7 @@ static inline int check_conn_state(struct ksmbd_work
>> *work)
>> if (ksmbd_conn_exiting(work->conn) ||
>> ksmbd_conn_need_reconnect(work->conn)) {
>> - rsp_hdr = work->response_buf;
>> + rsp_hdr = smb2_get_msg(work->response_buf);
>> rsp_hdr->Status.CifsError = STATUS_CONNECTION_DISCONNECTED;
>> return 1;
>> }
>> diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
>> index b23203a1c286..d6084580b59d 100644
>> --- a/fs/smb/server/smb_common.c
>> +++ b/fs/smb/server/smb_common.c
>> @@ -140,7 +140,7 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
>> if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
>> return ksmbd_smb2_check_message(work);
>> - hdr = work->request_buf;
>> + hdr = smb2_get_msg(work->request_buf);
>> if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
>> hdr->Command == SMB_COM_NEGOTIATE) {
>> work->conn->outstanding_credits++;
>> @@ -278,7 +278,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
>> req->DialectCount);
>> }
>> - proto = *(__le32 *)((struct smb_hdr *)buf)->Protocol;
>> if (proto == SMB1_PROTO_NUMBER) {
>> struct smb_negotiate_req *req;
>> @@ -320,8 +319,8 @@ static u16 get_smb1_cmd_val(struct ksmbd_work *work)
>> */
>> static int init_smb1_rsp_hdr(struct ksmbd_work *work)
>> {
>> - struct smb_hdr *rsp_hdr = (struct smb_hdr *)work->response_buf;
>> - struct smb_hdr *rcv_hdr = (struct smb_hdr *)work->request_buf;
>> + struct smb_hdr *rsp_hdr = (struct smb_hdr *)smb2_get_msg(work-
>> >response_buf);
>> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(work-
>> >request_buf);
>> rsp_hdr->Command = SMB_COM_NEGOTIATE;
>> *(__le32 *)rsp_hdr->Protocol = SMB1_PROTO_NUMBER;
>> @@ -412,9 +411,10 @@ static int init_smb1_server(struct ksmbd_conn *conn)
>> int ksmbd_init_smb_server(struct ksmbd_conn *conn)
>> {
>> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(conn-
>> >request_buf);
>> __le32 proto;
>> - proto = *(__le32 *)((struct smb_hdr *)conn->request_buf)->Protocol;
>> + proto = *(__le32 *)rcv_hdr->Protocol;
>> if (conn->need_neg == false) {
>> if (proto == SMB1_PROTO_NUMBER)
>> return -EINVAL;
>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr
2025-12-18 15:46 ` ChenXiaoSong
@ 2025-12-18 15:55 ` Namjae Jeon
0 siblings, 0 replies; 7+ messages in thread
From: Namjae Jeon @ 2025-12-18 15:55 UTC (permalink / raw)
To: ChenXiaoSong
Cc: David Howells, Steve French, Sergey Senozhatsky, Tom Talpey,
Paulo Alcantara, Shyam Prasad N, linux-cifs, netfs, linux-fsdevel,
linux-kernel
On Fri, Dec 19, 2025 at 12:47 AM ChenXiaoSong
<chenxiaosong.chenxiaosong@linux.dev> wrote:
>
> Hi David,
>
> Since the size of `struct smb_hdr` has changed, the value of
> `SMB1_MIN_SUPPORTED_HEADER_SIZE` should also be updated to
> `(sizeof(struct smb_hdr) + 4)`. `SMB1_MIN_SUPPORTED_HEADER_SIZE` is used
> in `ksmbd_conn_handler_loop()`.
Right. And there are other places that need to be changed as well, I
will check it.
Thanks!
>
> Thanks,
> ChenXiaoSong.
>
> On 12/18/25 11:09 PM, ChenXiaoSong wrote:
> > `ksmbd_conn_handler_loop()` calls `get_rfc1002_len()`. Does this need to
> > be updated as well?
> >
> > Thanks,
> > ChenXiaoSong.
> >
> > On 12/18/25 10:48 PM, David Howells wrote:
> >> Hi Namjae,
> >>
> >> Does this (untested) patch fix the problem for you?
> >>
> >> David
> >> ---
> >> The commit that removed the RFC1002 header from struct smb_hdr didn't
> >> also
> >> fix the places in ksmbd that use it in order to provide graceful
> >> rejection
> >> of SMB1 protocol requests.
> >>
> >> Fixes: 83bfbd0bb902 ("cifs: Remove the RFC1002 header from smb_hdr")
> >> Reported-by: Namjae Jeon <linkinjeon@kernel.org>
> >> Link: https://lore.kernel.org/r/
> >> CAKYAXd9Ju4MFkkH5Jxfi1mO0AWEr=R35M3vQ_Xa7Yw34JoNZ0A@mail.gmail.com/
> >> Signed-off-by: David Howells <dhowells@redhat.com>
> >> cc: Steve French <sfrench@samba.org>
> >> cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> >> cc: Tom Talpey <tom@talpey.com>
> >> cc: Paulo Alcantara <pc@manguebit.org>
> >> cc: Shyam Prasad N <sprasad@microsoft.com>
> >> cc: linux-cifs@vger.kernel.org
> >> cc: netfs@lists.linux.dev
> >> cc: linux-fsdevel@vger.kernel.org
> >> ---
> >> fs/smb/server/server.c | 2 +-
> >> fs/smb/server/smb_common.c | 10 +++++-----
> >> 2 files changed, 6 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
> >> index 3cea16050e4f..bedc8390b6db 100644
> >> --- a/fs/smb/server/server.c
> >> +++ b/fs/smb/server/server.c
> >> @@ -95,7 +95,7 @@ static inline int check_conn_state(struct ksmbd_work
> >> *work)
> >> if (ksmbd_conn_exiting(work->conn) ||
> >> ksmbd_conn_need_reconnect(work->conn)) {
> >> - rsp_hdr = work->response_buf;
> >> + rsp_hdr = smb2_get_msg(work->response_buf);
> >> rsp_hdr->Status.CifsError = STATUS_CONNECTION_DISCONNECTED;
> >> return 1;
> >> }
> >> diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
> >> index b23203a1c286..d6084580b59d 100644
> >> --- a/fs/smb/server/smb_common.c
> >> +++ b/fs/smb/server/smb_common.c
> >> @@ -140,7 +140,7 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
> >> if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
> >> return ksmbd_smb2_check_message(work);
> >> - hdr = work->request_buf;
> >> + hdr = smb2_get_msg(work->request_buf);
> >> if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
> >> hdr->Command == SMB_COM_NEGOTIATE) {
> >> work->conn->outstanding_credits++;
> >> @@ -278,7 +278,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
> >> req->DialectCount);
> >> }
> >> - proto = *(__le32 *)((struct smb_hdr *)buf)->Protocol;
> >> if (proto == SMB1_PROTO_NUMBER) {
> >> struct smb_negotiate_req *req;
> >> @@ -320,8 +319,8 @@ static u16 get_smb1_cmd_val(struct ksmbd_work *work)
> >> */
> >> static int init_smb1_rsp_hdr(struct ksmbd_work *work)
> >> {
> >> - struct smb_hdr *rsp_hdr = (struct smb_hdr *)work->response_buf;
> >> - struct smb_hdr *rcv_hdr = (struct smb_hdr *)work->request_buf;
> >> + struct smb_hdr *rsp_hdr = (struct smb_hdr *)smb2_get_msg(work-
> >> >response_buf);
> >> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(work-
> >> >request_buf);
> >> rsp_hdr->Command = SMB_COM_NEGOTIATE;
> >> *(__le32 *)rsp_hdr->Protocol = SMB1_PROTO_NUMBER;
> >> @@ -412,9 +411,10 @@ static int init_smb1_server(struct ksmbd_conn *conn)
> >> int ksmbd_init_smb_server(struct ksmbd_conn *conn)
> >> {
> >> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(conn-
> >> >request_buf);
> >> __le32 proto;
> >> - proto = *(__le32 *)((struct smb_hdr *)conn->request_buf)->Protocol;
> >> + proto = *(__le32 *)rcv_hdr->Protocol;
> >> if (conn->need_neg == false) {
> >> if (proto == SMB1_PROTO_NUMBER)
> >> return -EINVAL;
> >>
> >>
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr
2025-12-18 15:09 ` ChenXiaoSong
2025-12-18 15:46 ` ChenXiaoSong
@ 2025-12-18 16:01 ` David Howells
2025-12-18 16:49 ` ChenXiaoSong
1 sibling, 1 reply; 7+ messages in thread
From: David Howells @ 2025-12-18 16:01 UTC (permalink / raw)
To: ChenXiaoSong
Cc: dhowells, Namjae Jeon, Steve French, Sergey Senozhatsky,
Tom Talpey, Paulo Alcantara, Shyam Prasad N, linux-cifs, netfs,
linux-fsdevel, linux-kernel
ChenXiaoSong <chenxiaosong.chenxiaosong@linux.dev> wrote:
> `ksmbd_conn_handler_loop()` calls `get_rfc1002_len()`. Does this need to be
> updated as well?
I don't think so. It reads the RFC1002 header into a 4-byte array (hdr_buf)
and calls get_rfc1002_len() on that, so that should be unaffected.
> Since the size of `struct smb_hdr` has changed, the value of
> `SMB1_MIN_SUPPORTED_HEADER_SIZE` should also be updated to `(sizeof(struct
> smb_hdr) + 4)`. `SMB1_MIN_SUPPORTED_HEADER_SIZE` is used in
> `ksmbd_conn_handler_loop()`.
Actually, should SMB2_MIN_SUPPORTED_HEADER_SIZE include the +4 at all?
pdu_size is the length stored in the RFC1002 header, which does not include
itself.
David
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr
2025-12-18 16:01 ` David Howells
@ 2025-12-18 16:49 ` ChenXiaoSong
0 siblings, 0 replies; 7+ messages in thread
From: ChenXiaoSong @ 2025-12-18 16:49 UTC (permalink / raw)
To: David Howells, Namjae Jeon
Cc: Steve French, Sergey Senozhatsky, Tom Talpey, Paulo Alcantara,
Shyam Prasad N, linux-cifs, netfs, linux-fsdevel, linux-kernel
Hi Namjae,
It seems David is correct, the `LENGTH` field is described in RFC1002
section 4.3.1: https://www.rfc-editor.org/rfc/rfc1002#section-4.3.1
The LENGTH field is the number of bytes following the LENGTH
field. In other words, LENGTH is the combined size of the
TRAILER field(s).
Thanks,
ChenXiaoSong.
On 12/19/25 12:01 AM, David Howells wrote:
> ChenXiaoSong <chenxiaosong.chenxiaosong@linux.dev> wrote:
>
> Actually, should SMB2_MIN_SUPPORTED_HEADER_SIZE include the +4 at all?
> pdu_size is the length stored in the RFC1002 header, which does not include
> itself.
>
> David
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr
2025-12-18 14:48 [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr David Howells
2025-12-18 15:09 ` ChenXiaoSong
@ 2025-12-18 16:21 ` Namjae Jeon
1 sibling, 0 replies; 7+ messages in thread
From: Namjae Jeon @ 2025-12-18 16:21 UTC (permalink / raw)
To: David Howells
Cc: Steve French, Sergey Senozhatsky, Tom Talpey, Paulo Alcantara,
Shyam Prasad N, linux-cifs, netfs, linux-fsdevel, linux-kernel
On Thu, Dec 18, 2025 at 11:49 PM David Howells <dhowells@redhat.com> wrote:
>
> Hi Namjae,
Hi David,
>
> Does this (untested) patch fix the problem for you?
I sent the v2 patch to the list now. I have tested it with cifs.ko
and windows clients.
Thanks!
>
> David
> ---
> The commit that removed the RFC1002 header from struct smb_hdr didn't also
> fix the places in ksmbd that use it in order to provide graceful rejection
> of SMB1 protocol requests.
>
> Fixes: 83bfbd0bb902 ("cifs: Remove the RFC1002 header from smb_hdr")
> Reported-by: Namjae Jeon <linkinjeon@kernel.org>
> Link: https://lore.kernel.org/r/CAKYAXd9Ju4MFkkH5Jxfi1mO0AWEr=R35M3vQ_Xa7Yw34JoNZ0A@mail.gmail.com/
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Steve French <sfrench@samba.org>
> cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> cc: Tom Talpey <tom@talpey.com>
> cc: Paulo Alcantara <pc@manguebit.org>
> cc: Shyam Prasad N <sprasad@microsoft.com>
> cc: linux-cifs@vger.kernel.org
> cc: netfs@lists.linux.dev
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/smb/server/server.c | 2 +-
> fs/smb/server/smb_common.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
> index 3cea16050e4f..bedc8390b6db 100644
> --- a/fs/smb/server/server.c
> +++ b/fs/smb/server/server.c
> @@ -95,7 +95,7 @@ static inline int check_conn_state(struct ksmbd_work *work)
>
> if (ksmbd_conn_exiting(work->conn) ||
> ksmbd_conn_need_reconnect(work->conn)) {
> - rsp_hdr = work->response_buf;
> + rsp_hdr = smb2_get_msg(work->response_buf);
> rsp_hdr->Status.CifsError = STATUS_CONNECTION_DISCONNECTED;
> return 1;
> }
> diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
> index b23203a1c286..d6084580b59d 100644
> --- a/fs/smb/server/smb_common.c
> +++ b/fs/smb/server/smb_common.c
> @@ -140,7 +140,7 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
> if (smb2_hdr->ProtocolId == SMB2_PROTO_NUMBER)
> return ksmbd_smb2_check_message(work);
>
> - hdr = work->request_buf;
> + hdr = smb2_get_msg(work->request_buf);
> if (*(__le32 *)hdr->Protocol == SMB1_PROTO_NUMBER &&
> hdr->Command == SMB_COM_NEGOTIATE) {
> work->conn->outstanding_credits++;
> @@ -278,7 +278,6 @@ static int ksmbd_negotiate_smb_dialect(void *buf)
> req->DialectCount);
> }
>
> - proto = *(__le32 *)((struct smb_hdr *)buf)->Protocol;
> if (proto == SMB1_PROTO_NUMBER) {
> struct smb_negotiate_req *req;
>
> @@ -320,8 +319,8 @@ static u16 get_smb1_cmd_val(struct ksmbd_work *work)
> */
> static int init_smb1_rsp_hdr(struct ksmbd_work *work)
> {
> - struct smb_hdr *rsp_hdr = (struct smb_hdr *)work->response_buf;
> - struct smb_hdr *rcv_hdr = (struct smb_hdr *)work->request_buf;
> + struct smb_hdr *rsp_hdr = (struct smb_hdr *)smb2_get_msg(work->response_buf);
> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(work->request_buf);
>
> rsp_hdr->Command = SMB_COM_NEGOTIATE;
> *(__le32 *)rsp_hdr->Protocol = SMB1_PROTO_NUMBER;
> @@ -412,9 +411,10 @@ static int init_smb1_server(struct ksmbd_conn *conn)
>
> int ksmbd_init_smb_server(struct ksmbd_conn *conn)
> {
> + struct smb_hdr *rcv_hdr = (struct smb_hdr *)smb2_get_msg(conn->request_buf);
> __le32 proto;
>
> - proto = *(__le32 *)((struct smb_hdr *)conn->request_buf)->Protocol;
> + proto = *(__le32 *)rcv_hdr->Protocol;
> if (conn->need_neg == false) {
> if (proto == SMB1_PROTO_NUMBER)
> return -EINVAL;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-18 16:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-18 14:48 [PATCH] ksmbd: Fix to handle removal of rfc1002 header from smb_hdr David Howells
2025-12-18 15:09 ` ChenXiaoSong
2025-12-18 15:46 ` ChenXiaoSong
2025-12-18 15:55 ` Namjae Jeon
2025-12-18 16:01 ` David Howells
2025-12-18 16:49 ` ChenXiaoSong
2025-12-18 16:21 ` Namjae Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox