Re: [PATCH 13/13] smb: move some SMB1 definitions into common/smb1pdu.h

Steve French
2025-12-09T02:53:13Z

One option to consider is moving smb1 definitions into a client only
(fs/smb/client) header since ksmbd doesn't use SMB1 technically they
aren't 'common'

Did you find ksmbd server cases where it depends on any of these?

On Mon, Dec 8, 2025 at 7:12 PM <chenxiaosong.chenxiaosong@linux.dev> wrote:
>
> From: ZhangGuoDong <zhangguodong@kylinos.cn>
>
> These definitions are only used by SMB1, so move them into the new
> common/smb1pdu.h.
>
> Co-developed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
> Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
> ---
> fs/smb/client/cifspdu.h | 2 +-
> fs/smb/common/smb1pdu.h | 59 ++++++++++++++++++++++++++++++++++++++
> fs/smb/common/smb2pdu.h | 44 ----------------------------
> fs/smb/common/smbglob.h | 2 --
> fs/smb/server/smb_common.h | 1 +
> 5 files changed, 61 insertions(+), 47 deletions(-)
> create mode 100644 fs/smb/common/smb1pdu.h
>
> diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
> index 758ea29769da..bf6329cb4fd4 100644
> --- a/fs/smb/client/cifspdu.h
> +++ b/fs/smb/client/cifspdu.h
> @@ -12,7 +12,7 @@
> #include <net/sock.h>
> #include <linux/unaligned.h>
> #include "../common/smbfsctl.h"
> -#include "../common/smb2pdu.h"
> +#include "../common/smb1pdu.h"
>
> #define CIFS_PROT 0
> #define POSIX_PROT (CIFS_PROT+1)
> diff --git a/fs/smb/common/smb1pdu.h b/fs/smb/common/smb1pdu.h
> new file mode 100644
> index 000000000000..11797471b2eb
> --- /dev/null
> +++ b/fs/smb/common/smb1pdu.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: LGPL-2.1 */
> +/*
> + *
> + * Copyright (c) International Business Machines Corp., 2002,2009
> + * 2018 Samsung Electronics Co., Ltd.
> + * Author(s): Steve French (sfrench@us.ibm.com)
> + * Namjae Jeon (linkinjeon@kernel.org)
> + *
> + */
> +
> +#ifndef _COMMON_SMB1_PDU_H
> +#define _COMMON_SMB1_PDU_H
> +
> +#define SMB1_PROTO_NUMBER cpu_to_le32(0x424d53ff)
> +
> +/*
> + * See MS-CIFS 2.2.3.1
> + * MS-SMB 2.2.3.1
> + */
> +struct smb_hdr {
> + __be32 smb_buf_length; /* BB length is only two (rarely three) bytes,
> + with one or two byte "type" preceding it that will be
> + zero - we could mask the type byte off */
> + __u8 Protocol[4];
> + __u8 Command;
> + union {
> + struct {
> + __u8 ErrorClass;
> + __u8 Reserved;
> + __le16 Error;
> + } __packed DosError;
> + __le32 CifsError;
> + } __packed Status;
> + __u8 Flags;
> + __le16 Flags2; /* note: le */
> + __le16 PidHigh;
> + union {
> + struct {
> + __le32 SequenceNumber; /* le */
> + __u32 Reserved; /* zero */
> + } __packed Sequence;
> + __u8 SecuritySignature[8]; /* le */
> + } __packed Signature;
> + __u8 pad[2];
> + __u16 Tid;
> + __le16 Pid;
> + __u16 Uid;
> + __le16 Mid;
> + __u8 WordCount;
> +} __packed;
> +
> +/* See MS-CIFS 2.2.4.52.1 */
> +typedef struct smb_negotiate_req {
> + struct smb_hdr hdr; /* wct = 0 */
> + __le16 ByteCount;
> + unsigned char DialectsArray[];
> +} __packed SMB_NEGOTIATE_REQ;
> +
> +#endif /* _COMMON_SMB1_PDU_H */
> diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
> index 2d68bd24f3bd..098f147680c5 100644
> --- a/fs/smb/common/smb2pdu.h
> +++ b/fs/smb/common/smb2pdu.h
> @@ -1642,42 +1642,6 @@ struct smb2_lease_ack {
> __le64 LeaseDuration;
> } __packed;
>
> -/*
> - * See MS-CIFS 2.2.3.1
> - * MS-SMB 2.2.3.1
> - */
> -struct smb_hdr {
> - __be32 smb_buf_length; /* BB length is only two (rarely three) bytes,
> - with one or two byte "type" preceding it that will be
> - zero - we could mask the type byte off */
> - __u8 Protocol[4];
> - __u8 Command;
> - union {
> - struct {
> - __u8 ErrorClass;
> - __u8 Reserved;
> - __le16 Error;
> - } __packed DosError;
> - __le32 CifsError;
> - } __packed Status;
> - __u8 Flags;
> - __le16 Flags2; /* note: le */
> - __le16 PidHigh;
> - union {
> - struct {
> - __le32 SequenceNumber; /* le */
> - __u32 Reserved; /* zero */
> - } __packed Sequence;
> - __u8 SecuritySignature[8]; /* le */
> - } __packed Signature;
> - __u8 pad[2];
> - __u16 Tid;
> - __le16 Pid;
> - __u16 Uid;
> - __le16 Mid;
> - __u8 WordCount;
> -} __packed;
> -
> #define OP_BREAK_STRUCT_SIZE_20 24
> #define OP_BREAK_STRUCT_SIZE_21 36
>
> @@ -1782,14 +1746,6 @@ struct smb_hdr {
> #define SET_MINIMUM_RIGHTS (FILE_READ_EA | FILE_READ_ATTRIBUTES \
> | READ_CONTROL | SYNCHRONIZE)
>
> -/* See MS-CIFS 2.2.4.52.1 */
> -typedef struct smb_negotiate_req {
> - struct smb_hdr hdr; /* wct = 0 */
> - __le16 ByteCount;
> - unsigned char DialectsArray[];
> -} __packed SMB_NEGOTIATE_REQ;
> -
> -
> /*
> * [POSIX-SMB2] SMB3 POSIX Extensions
> * Link: https://gitlab.com/samba-team/smb3-posix-spec/-/blob/master/smb3_posix_extensions.md
> diff --git a/fs/smb/common/smbglob.h b/fs/smb/common/smbglob.h
> index 7853b5771128..353dc4f0971a 100644
> --- a/fs/smb/common/smbglob.h
> +++ b/fs/smb/common/smbglob.h
> @@ -11,8 +11,6 @@
> #ifndef _COMMON_SMB_GLOB_H
> #define _COMMON_SMB_GLOB_H
>
> -#define SMB1_PROTO_NUMBER cpu_to_le32(0x424d53ff)
> -
> struct smb_version_values {
> char *version_string;
> __u16 protocol_id;
> diff --git a/fs/smb/server/smb_common.h b/fs/smb/server/smb_common.h
> index b8da31cdbfd1..f47ce4a6719c 100644
> --- a/fs/smb/server/smb_common.h
> +++ b/fs/smb/server/smb_common.h
> @@ -11,6 +11,7 @@
> #include "glob.h"
> #include "nterr.h"
> #include "../common/smbglob.h"
> +#include "../common/smb1pdu.h"
> #include "../common/smb2pdu.h"
> #include "../common/fscc.h"
> #include "smb2pdu.h"
> --
> 2.43.0
>

--
Thanks,

Steve