All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings
@ 2024-04-11 15:35 Gustavo A. R. Silva
  2024-04-23 18:57 ` Gustavo A. R. Silva
  0 siblings, 1 reply; 7+ messages in thread
From: Gustavo A. R. Silva @ 2024-04-11 15:35 UTC (permalink / raw)
  To: Steve French, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N,
	Tom Talpey, Bharath SM, Namjae Jeon, Sergey Senozhatsky
  Cc: linux-cifs, samba-technical, linux-kernel, Gustavo A. R. Silva,
	linux-hardening

-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.

So, in order to avoid ending up with a flexible-array member in the
middle of multiple other structs, we use the `__struct_group()` helper
to separate the flexible array from the rest of the members in the
flexible structure, and use the tagged `struct create_context_hdr`
instead of `struct create_context`.

So, with these changes, fix 51 of the following warnings[1]:

fs/smb/client/../common/smb2pdu.h:1225:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Link: https://gist.github.com/GustavoARSilva/772526a39be3dd4db39e71497f0a9893 [1]
Link: https://github.com/KSPP/linux/issues/202
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 fs/smb/client/smb2pdu.h | 12 ++++++------
 fs/smb/common/smb2pdu.h | 33 ++++++++++++++++++---------------
 fs/smb/server/smb2pdu.h | 18 +++++++++---------
 3 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
index c72a3b2886b7..1a02bd9e0c00 100644
--- a/fs/smb/client/smb2pdu.h
+++ b/fs/smb/client/smb2pdu.h
@@ -145,7 +145,7 @@ struct durable_context_v2 {
 } __packed;
 
 struct create_durable_v2 {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	struct durable_context_v2 dcontext;
 } __packed;
@@ -167,7 +167,7 @@ struct durable_reconnect_context_v2_rsp {
 } __packed;
 
 struct create_durable_handle_reconnect_v2 {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	struct durable_reconnect_context_v2 dcontext;
 	__u8   Pad[4];
@@ -175,7 +175,7 @@ struct create_durable_handle_reconnect_v2 {
 
 /* See MS-SMB2 2.2.13.2.5 */
 struct crt_twarp_ctxt {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8	Name[8];
 	__le64	Timestamp;
 
@@ -183,12 +183,12 @@ struct crt_twarp_ctxt {
 
 /* See MS-SMB2 2.2.13.2.9 */
 struct crt_query_id_ctxt {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8	Name[8];
 } __packed;
 
 struct crt_sd_ctxt {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8	Name[8];
 	struct smb3_sd sd;
 } __packed;
@@ -415,7 +415,7 @@ struct smb2_posix_info_parsed {
 };
 
 struct smb2_create_ea_ctx {
-	struct create_context ctx;
+	struct create_context_hdr ctx;
 	__u8 name[8];
 	struct smb2_file_full_ea_info ea;
 } __packed;
diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
index 1b594307c9d5..eab9d49c63ba 100644
--- a/fs/smb/common/smb2pdu.h
+++ b/fs/smb/common/smb2pdu.h
@@ -1171,12 +1171,15 @@ struct smb2_server_client_notification {
 #define SMB2_CREATE_FLAG_REPARSEPOINT 0x01
 
 struct create_context {
-	__le32 Next;
-	__le16 NameOffset;
-	__le16 NameLength;
-	__le16 Reserved;
-	__le16 DataOffset;
-	__le32 DataLength;
+	/* New members must be added within the struct_group() macro below. */
+	__struct_group(create_context_hdr, hdr, __packed,
+		__le32 Next;
+		__le16 NameOffset;
+		__le16 NameLength;
+		__le16 Reserved;
+		__le16 DataOffset;
+		__le32 DataLength;
+	);
 	__u8 Buffer[];
 } __packed;
 
@@ -1222,7 +1225,7 @@ struct smb2_create_rsp {
 } __packed;
 
 struct create_posix {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8    Name[16];
 	__le32  Mode;
 	__u32   Reserved;
@@ -1230,7 +1233,7 @@ struct create_posix {
 
 /* See MS-SMB2 2.2.13.2.3 and MS-SMB2 2.2.13.2.4 */
 struct create_durable {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	union {
 		__u8  Reserved[16];
@@ -1243,14 +1246,14 @@ struct create_durable {
 
 /* See MS-SMB2 2.2.13.2.5 */
 struct create_mxac_req {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	__le64 Timestamp;
 } __packed;
 
 /* See MS-SMB2 2.2.14.2.5 */
 struct create_mxac_rsp {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	__le32 QueryStatus;
 	__le32 MaximalAccess;
@@ -1286,13 +1289,13 @@ struct lease_context_v2 {
 } __packed;
 
 struct create_lease {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	struct lease_context lcontext;
 } __packed;
 
 struct create_lease_v2 {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	struct lease_context_v2 lcontext;
 	__u8   Pad[4];
@@ -1300,7 +1303,7 @@ struct create_lease_v2 {
 
 /* See MS-SMB2 2.2.14.2.9 */
 struct create_disk_id_rsp {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	__le64 DiskFileId;
 	__le64 VolumeId;
@@ -1309,7 +1312,7 @@ struct create_disk_id_rsp {
 
 /* See MS-SMB2 2.2.13.2.13 */
 struct create_app_inst_id {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8 Name[16];
 	__le32 StructureSize; /* Must be 20 */
 	__u16 Reserved;
@@ -1318,7 +1321,7 @@ struct create_app_inst_id {
 
 /* See MS-SMB2 2.2.13.2.15 */
 struct create_app_inst_id_vers {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8 Name[16];
 	__le32 StructureSize; /* Must be 24 */
 	__u16 Reserved;
diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
index bd1d2a0e9203..643f5e1cfe35 100644
--- a/fs/smb/server/smb2pdu.h
+++ b/fs/smb/server/smb2pdu.h
@@ -64,7 +64,7 @@ struct preauth_integrity_info {
 #define SMB2_SESSION_TIMEOUT		(10 * HZ)
 
 struct create_durable_req_v2 {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	__le32 Timeout;
 	__le32 Flags;
@@ -73,7 +73,7 @@ struct create_durable_req_v2 {
 } __packed;
 
 struct create_durable_reconn_req {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	union {
 		__u8  Reserved[16];
@@ -85,7 +85,7 @@ struct create_durable_reconn_req {
 } __packed;
 
 struct create_durable_reconn_v2_req {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	struct {
 		__u64 PersistentFileId;
@@ -96,13 +96,13 @@ struct create_durable_reconn_v2_req {
 } __packed;
 
 struct create_alloc_size_req {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	__le64 AllocationSize;
 } __packed;
 
 struct create_durable_rsp {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	union {
 		__u8  Reserved[8];
@@ -114,7 +114,7 @@ struct create_durable_rsp {
 /* Flags */
 #define SMB2_DHANDLE_FLAG_PERSISTENT	0x00000002
 struct create_durable_v2_rsp {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	__le32 Timeout;
 	__le32 Flags;
@@ -122,7 +122,7 @@ struct create_durable_v2_rsp {
 
 /* equivalent of the contents of SMB3.1.1 POSIX open context response */
 struct create_posix_rsp {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8    Name[16];
 	__le32 nlink;
 	__le32 reparse_tag;
@@ -381,13 +381,13 @@ struct smb2_ea_info {
 } __packed; /* level 15 Query */
 
 struct create_ea_buf_req {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	struct smb2_ea_info ea;
 } __packed;
 
 struct create_sd_buf_req {
-	struct create_context ccontext;
+	struct create_context_hdr ccontext;
 	__u8   Name[8];
 	struct smb_ntsd ntsd;
 } __packed;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings
  2024-04-11 15:35 [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
@ 2024-04-23 18:57 ` Gustavo A. R. Silva
  2024-04-23 20:15   ` Steve French
  0 siblings, 1 reply; 7+ messages in thread
From: Gustavo A. R. Silva @ 2024-04-23 18:57 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	Namjae Jeon, Sergey Senozhatsky
  Cc: linux-cifs, samba-technical, linux-kernel, linux-hardening, Kees Cook

Hi all,

Friendly ping: who can take this, please?

Thanks
--
Gustavo

On 11/04/24 09:35, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> ready to enable it globally.
> 
> So, in order to avoid ending up with a flexible-array member in the
> middle of multiple other structs, we use the `__struct_group()` helper
> to separate the flexible array from the rest of the members in the
> flexible structure, and use the tagged `struct create_context_hdr`
> instead of `struct create_context`.
> 
> So, with these changes, fix 51 of the following warnings[1]:
> 
> fs/smb/client/../common/smb2pdu.h:1225:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> Link: https://gist.github.com/GustavoARSilva/772526a39be3dd4db39e71497f0a9893 [1]
> Link: https://github.com/KSPP/linux/issues/202
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   fs/smb/client/smb2pdu.h | 12 ++++++------
>   fs/smb/common/smb2pdu.h | 33 ++++++++++++++++++---------------
>   fs/smb/server/smb2pdu.h | 18 +++++++++---------
>   3 files changed, 33 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
> index c72a3b2886b7..1a02bd9e0c00 100644
> --- a/fs/smb/client/smb2pdu.h
> +++ b/fs/smb/client/smb2pdu.h
> @@ -145,7 +145,7 @@ struct durable_context_v2 {
>   } __packed;
>   
>   struct create_durable_v2 {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	struct durable_context_v2 dcontext;
>   } __packed;
> @@ -167,7 +167,7 @@ struct durable_reconnect_context_v2_rsp {
>   } __packed;
>   
>   struct create_durable_handle_reconnect_v2 {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	struct durable_reconnect_context_v2 dcontext;
>   	__u8   Pad[4];
> @@ -175,7 +175,7 @@ struct create_durable_handle_reconnect_v2 {
>   
>   /* See MS-SMB2 2.2.13.2.5 */
>   struct crt_twarp_ctxt {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8	Name[8];
>   	__le64	Timestamp;
>   
> @@ -183,12 +183,12 @@ struct crt_twarp_ctxt {
>   
>   /* See MS-SMB2 2.2.13.2.9 */
>   struct crt_query_id_ctxt {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8	Name[8];
>   } __packed;
>   
>   struct crt_sd_ctxt {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8	Name[8];
>   	struct smb3_sd sd;
>   } __packed;
> @@ -415,7 +415,7 @@ struct smb2_posix_info_parsed {
>   };
>   
>   struct smb2_create_ea_ctx {
> -	struct create_context ctx;
> +	struct create_context_hdr ctx;
>   	__u8 name[8];
>   	struct smb2_file_full_ea_info ea;
>   } __packed;
> diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
> index 1b594307c9d5..eab9d49c63ba 100644
> --- a/fs/smb/common/smb2pdu.h
> +++ b/fs/smb/common/smb2pdu.h
> @@ -1171,12 +1171,15 @@ struct smb2_server_client_notification {
>   #define SMB2_CREATE_FLAG_REPARSEPOINT 0x01
>   
>   struct create_context {
> -	__le32 Next;
> -	__le16 NameOffset;
> -	__le16 NameLength;
> -	__le16 Reserved;
> -	__le16 DataOffset;
> -	__le32 DataLength;
> +	/* New members must be added within the struct_group() macro below. */
> +	__struct_group(create_context_hdr, hdr, __packed,
> +		__le32 Next;
> +		__le16 NameOffset;
> +		__le16 NameLength;
> +		__le16 Reserved;
> +		__le16 DataOffset;
> +		__le32 DataLength;
> +	);
>   	__u8 Buffer[];
>   } __packed;
>   
> @@ -1222,7 +1225,7 @@ struct smb2_create_rsp {
>   } __packed;
>   
>   struct create_posix {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8    Name[16];
>   	__le32  Mode;
>   	__u32   Reserved;
> @@ -1230,7 +1233,7 @@ struct create_posix {
>   
>   /* See MS-SMB2 2.2.13.2.3 and MS-SMB2 2.2.13.2.4 */
>   struct create_durable {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	union {
>   		__u8  Reserved[16];
> @@ -1243,14 +1246,14 @@ struct create_durable {
>   
>   /* See MS-SMB2 2.2.13.2.5 */
>   struct create_mxac_req {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	__le64 Timestamp;
>   } __packed;
>   
>   /* See MS-SMB2 2.2.14.2.5 */
>   struct create_mxac_rsp {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	__le32 QueryStatus;
>   	__le32 MaximalAccess;
> @@ -1286,13 +1289,13 @@ struct lease_context_v2 {
>   } __packed;
>   
>   struct create_lease {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	struct lease_context lcontext;
>   } __packed;
>   
>   struct create_lease_v2 {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	struct lease_context_v2 lcontext;
>   	__u8   Pad[4];
> @@ -1300,7 +1303,7 @@ struct create_lease_v2 {
>   
>   /* See MS-SMB2 2.2.14.2.9 */
>   struct create_disk_id_rsp {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	__le64 DiskFileId;
>   	__le64 VolumeId;
> @@ -1309,7 +1312,7 @@ struct create_disk_id_rsp {
>   
>   /* See MS-SMB2 2.2.13.2.13 */
>   struct create_app_inst_id {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8 Name[16];
>   	__le32 StructureSize; /* Must be 20 */
>   	__u16 Reserved;
> @@ -1318,7 +1321,7 @@ struct create_app_inst_id {
>   
>   /* See MS-SMB2 2.2.13.2.15 */
>   struct create_app_inst_id_vers {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8 Name[16];
>   	__le32 StructureSize; /* Must be 24 */
>   	__u16 Reserved;
> diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
> index bd1d2a0e9203..643f5e1cfe35 100644
> --- a/fs/smb/server/smb2pdu.h
> +++ b/fs/smb/server/smb2pdu.h
> @@ -64,7 +64,7 @@ struct preauth_integrity_info {
>   #define SMB2_SESSION_TIMEOUT		(10 * HZ)
>   
>   struct create_durable_req_v2 {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	__le32 Timeout;
>   	__le32 Flags;
> @@ -73,7 +73,7 @@ struct create_durable_req_v2 {
>   } __packed;
>   
>   struct create_durable_reconn_req {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	union {
>   		__u8  Reserved[16];
> @@ -85,7 +85,7 @@ struct create_durable_reconn_req {
>   } __packed;
>   
>   struct create_durable_reconn_v2_req {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	struct {
>   		__u64 PersistentFileId;
> @@ -96,13 +96,13 @@ struct create_durable_reconn_v2_req {
>   } __packed;
>   
>   struct create_alloc_size_req {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	__le64 AllocationSize;
>   } __packed;
>   
>   struct create_durable_rsp {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	union {
>   		__u8  Reserved[8];
> @@ -114,7 +114,7 @@ struct create_durable_rsp {
>   /* Flags */
>   #define SMB2_DHANDLE_FLAG_PERSISTENT	0x00000002
>   struct create_durable_v2_rsp {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	__le32 Timeout;
>   	__le32 Flags;
> @@ -122,7 +122,7 @@ struct create_durable_v2_rsp {
>   
>   /* equivalent of the contents of SMB3.1.1 POSIX open context response */
>   struct create_posix_rsp {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8    Name[16];
>   	__le32 nlink;
>   	__le32 reparse_tag;
> @@ -381,13 +381,13 @@ struct smb2_ea_info {
>   } __packed; /* level 15 Query */
>   
>   struct create_ea_buf_req {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	struct smb2_ea_info ea;
>   } __packed;
>   
>   struct create_sd_buf_req {
> -	struct create_context ccontext;
> +	struct create_context_hdr ccontext;
>   	__u8   Name[8];
>   	struct smb_ntsd ntsd;
>   } __packed;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings
  2024-04-23 18:57 ` Gustavo A. R. Silva
@ 2024-04-23 20:15   ` Steve French
  2024-04-23 20:47     ` Gustavo A. R. Silva
  0 siblings, 1 reply; 7+ messages in thread
From: Steve French @ 2024-04-23 20:15 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Gustavo A. R. Silva, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	Namjae Jeon, Sergey Senozhatsky, linux-cifs, samba-technical,
	linux-kernel, linux-hardening, Kees Cook

This looks reasonably safe (running the usual regression tests on it now).

Reminds me though that we have to be careful (e.g. the recent fix for
regression caused by cleanup).

Thoughts about whether should be sent in rc6 or wait till 6.10?  51
warnings does sound
distracting though so might be worth going in sooner rather than later.

commit 0268a7cc7fdc47d90b6c18859de7718d5059f6f1
Author: Namjae Jeon <linkinjeon@kernel.org>
Date:   Fri Apr 19 23:46:34 2024 +0900

    ksmbd: common: use struct_group_attr instead of struct_group for
network_open_info

    4byte padding cause the connection issue with the applications of MacOS.
    smb2_close response size increases by 4 bytes by padding, And the smb
    client of MacOS check it and stop the connection. This patch use
    struct_group_attr instead of struct_group for network_open_info to use
     __packed to avoid padding.


On Tue, Apr 23, 2024 at 1:58 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
> Hi all,
>
> Friendly ping: who can take this, please?
>
> Thanks
> --
> Gustavo
>
> On 11/04/24 09:35, Gustavo A. R. Silva wrote:
> > -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> > ready to enable it globally.
> >
> > So, in order to avoid ending up with a flexible-array member in the
> > middle of multiple other structs, we use the `__struct_group()` helper
> > to separate the flexible array from the rest of the members in the
> > flexible structure, and use the tagged `struct create_context_hdr`
> > instead of `struct create_context`.
> >
> > So, with these changes, fix 51 of the following warnings[1]:
> >
> > fs/smb/client/../common/smb2pdu.h:1225:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> >
> > Link: https://gist.github.com/GustavoARSilva/772526a39be3dd4db39e71497f0a9893 [1]
> > Link: https://github.com/KSPP/linux/issues/202
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > ---
> >   fs/smb/client/smb2pdu.h | 12 ++++++------
> >   fs/smb/common/smb2pdu.h | 33 ++++++++++++++++++---------------
> >   fs/smb/server/smb2pdu.h | 18 +++++++++---------
> >   3 files changed, 33 insertions(+), 30 deletions(-)
> >
> > diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
> > index c72a3b2886b7..1a02bd9e0c00 100644
> > --- a/fs/smb/client/smb2pdu.h
> > +++ b/fs/smb/client/smb2pdu.h
> > @@ -145,7 +145,7 @@ struct durable_context_v2 {
> >   } __packed;
> >
> >   struct create_durable_v2 {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       struct durable_context_v2 dcontext;
> >   } __packed;
> > @@ -167,7 +167,7 @@ struct durable_reconnect_context_v2_rsp {
> >   } __packed;
> >
> >   struct create_durable_handle_reconnect_v2 {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       struct durable_reconnect_context_v2 dcontext;
> >       __u8   Pad[4];
> > @@ -175,7 +175,7 @@ struct create_durable_handle_reconnect_v2 {
> >
> >   /* See MS-SMB2 2.2.13.2.5 */
> >   struct crt_twarp_ctxt {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8    Name[8];
> >       __le64  Timestamp;
> >
> > @@ -183,12 +183,12 @@ struct crt_twarp_ctxt {
> >
> >   /* See MS-SMB2 2.2.13.2.9 */
> >   struct crt_query_id_ctxt {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8    Name[8];
> >   } __packed;
> >
> >   struct crt_sd_ctxt {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8    Name[8];
> >       struct smb3_sd sd;
> >   } __packed;
> > @@ -415,7 +415,7 @@ struct smb2_posix_info_parsed {
> >   };
> >
> >   struct smb2_create_ea_ctx {
> > -     struct create_context ctx;
> > +     struct create_context_hdr ctx;
> >       __u8 name[8];
> >       struct smb2_file_full_ea_info ea;
> >   } __packed;
> > diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
> > index 1b594307c9d5..eab9d49c63ba 100644
> > --- a/fs/smb/common/smb2pdu.h
> > +++ b/fs/smb/common/smb2pdu.h
> > @@ -1171,12 +1171,15 @@ struct smb2_server_client_notification {
> >   #define SMB2_CREATE_FLAG_REPARSEPOINT 0x01
> >
> >   struct create_context {
> > -     __le32 Next;
> > -     __le16 NameOffset;
> > -     __le16 NameLength;
> > -     __le16 Reserved;
> > -     __le16 DataOffset;
> > -     __le32 DataLength;
> > +     /* New members must be added within the struct_group() macro below. */
> > +     __struct_group(create_context_hdr, hdr, __packed,
> > +             __le32 Next;
> > +             __le16 NameOffset;
> > +             __le16 NameLength;
> > +             __le16 Reserved;
> > +             __le16 DataOffset;
> > +             __le32 DataLength;
> > +     );
> >       __u8 Buffer[];
> >   } __packed;
> >
> > @@ -1222,7 +1225,7 @@ struct smb2_create_rsp {
> >   } __packed;
> >
> >   struct create_posix {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8    Name[16];
> >       __le32  Mode;
> >       __u32   Reserved;
> > @@ -1230,7 +1233,7 @@ struct create_posix {
> >
> >   /* See MS-SMB2 2.2.13.2.3 and MS-SMB2 2.2.13.2.4 */
> >   struct create_durable {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       union {
> >               __u8  Reserved[16];
> > @@ -1243,14 +1246,14 @@ struct create_durable {
> >
> >   /* See MS-SMB2 2.2.13.2.5 */
> >   struct create_mxac_req {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       __le64 Timestamp;
> >   } __packed;
> >
> >   /* See MS-SMB2 2.2.14.2.5 */
> >   struct create_mxac_rsp {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       __le32 QueryStatus;
> >       __le32 MaximalAccess;
> > @@ -1286,13 +1289,13 @@ struct lease_context_v2 {
> >   } __packed;
> >
> >   struct create_lease {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       struct lease_context lcontext;
> >   } __packed;
> >
> >   struct create_lease_v2 {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       struct lease_context_v2 lcontext;
> >       __u8   Pad[4];
> > @@ -1300,7 +1303,7 @@ struct create_lease_v2 {
> >
> >   /* See MS-SMB2 2.2.14.2.9 */
> >   struct create_disk_id_rsp {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       __le64 DiskFileId;
> >       __le64 VolumeId;
> > @@ -1309,7 +1312,7 @@ struct create_disk_id_rsp {
> >
> >   /* See MS-SMB2 2.2.13.2.13 */
> >   struct create_app_inst_id {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8 Name[16];
> >       __le32 StructureSize; /* Must be 20 */
> >       __u16 Reserved;
> > @@ -1318,7 +1321,7 @@ struct create_app_inst_id {
> >
> >   /* See MS-SMB2 2.2.13.2.15 */
> >   struct create_app_inst_id_vers {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8 Name[16];
> >       __le32 StructureSize; /* Must be 24 */
> >       __u16 Reserved;
> > diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
> > index bd1d2a0e9203..643f5e1cfe35 100644
> > --- a/fs/smb/server/smb2pdu.h
> > +++ b/fs/smb/server/smb2pdu.h
> > @@ -64,7 +64,7 @@ struct preauth_integrity_info {
> >   #define SMB2_SESSION_TIMEOUT                (10 * HZ)
> >
> >   struct create_durable_req_v2 {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       __le32 Timeout;
> >       __le32 Flags;
> > @@ -73,7 +73,7 @@ struct create_durable_req_v2 {
> >   } __packed;
> >
> >   struct create_durable_reconn_req {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       union {
> >               __u8  Reserved[16];
> > @@ -85,7 +85,7 @@ struct create_durable_reconn_req {
> >   } __packed;
> >
> >   struct create_durable_reconn_v2_req {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       struct {
> >               __u64 PersistentFileId;
> > @@ -96,13 +96,13 @@ struct create_durable_reconn_v2_req {
> >   } __packed;
> >
> >   struct create_alloc_size_req {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       __le64 AllocationSize;
> >   } __packed;
> >
> >   struct create_durable_rsp {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       union {
> >               __u8  Reserved[8];
> > @@ -114,7 +114,7 @@ struct create_durable_rsp {
> >   /* Flags */
> >   #define SMB2_DHANDLE_FLAG_PERSISTENT        0x00000002
> >   struct create_durable_v2_rsp {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       __le32 Timeout;
> >       __le32 Flags;
> > @@ -122,7 +122,7 @@ struct create_durable_v2_rsp {
> >
> >   /* equivalent of the contents of SMB3.1.1 POSIX open context response */
> >   struct create_posix_rsp {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8    Name[16];
> >       __le32 nlink;
> >       __le32 reparse_tag;
> > @@ -381,13 +381,13 @@ struct smb2_ea_info {
> >   } __packed; /* level 15 Query */
> >
> >   struct create_ea_buf_req {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       struct smb2_ea_info ea;
> >   } __packed;
> >
> >   struct create_sd_buf_req {
> > -     struct create_context ccontext;
> > +     struct create_context_hdr ccontext;
> >       __u8   Name[8];
> >       struct smb_ntsd ntsd;
> >   } __packed;
>


--
Thanks,

Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings
  2024-04-23 20:15   ` Steve French
@ 2024-04-23 20:47     ` Gustavo A. R. Silva
  2024-04-23 21:08       ` Gustavo A. R. Silva
  2024-04-23 21:09       ` Steve French
  0 siblings, 2 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2024-04-23 20:47 UTC (permalink / raw)
  To: Steve French
  Cc: Gustavo A. R. Silva, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	Namjae Jeon, Sergey Senozhatsky, linux-cifs, samba-technical,
	linux-kernel, linux-hardening, Kees Cook



On 23/04/24 14:15, Steve French wrote:
> This looks reasonably safe (running the usual regression tests on it now).
> 
> Reminds me though that we have to be careful (e.g. the recent fix for
> regression caused by cleanup).

mmh... it seems that the offending commit was never CC'd to the linux-hardening
list, hence it wasn't reviewed by us.

After reviewing both, the offending commit and the fix, both seem to be wrong.

for __packed structs, you should use __struct_group():

__struct_group(network_open_info, group_name, __packed, struct_members);

The _packed in the commit 0268a7cc7fdc is not an attribute, it's the name
for the group. So, it's not actually doing what the submitter thinks it does.

> 
> Thoughts about whether should be sent in rc6 or wait till 6.10?  51
> warnings does sound
> distracting though so might be worth going in sooner rather than later.

There is actually no hurry. :)

Thanks
--
Gustavo

> 
> commit 0268a7cc7fdc47d90b6c18859de7718d5059f6f1
> Author: Namjae Jeon <linkinjeon@kernel.org>
> Date:   Fri Apr 19 23:46:34 2024 +0900
> 
>      ksmbd: common: use struct_group_attr instead of struct_group for
> network_open_info
> 
>      4byte padding cause the connection issue with the applications of MacOS.
>      smb2_close response size increases by 4 bytes by padding, And the smb
>      client of MacOS check it and stop the connection. This patch use
>      struct_group_attr instead of struct_group for network_open_info to use
>       __packed to avoid padding.
> 
> 
> On Tue, Apr 23, 2024 at 1:58 PM Gustavo A. R. Silva
> <gustavo@embeddedor.com> wrote:
>>
>> Hi all,
>>
>> Friendly ping: who can take this, please?
>>
>> Thanks
>> --
>> Gustavo
>>
>> On 11/04/24 09:35, Gustavo A. R. Silva wrote:
>>> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
>>> ready to enable it globally.
>>>
>>> So, in order to avoid ending up with a flexible-array member in the
>>> middle of multiple other structs, we use the `__struct_group()` helper
>>> to separate the flexible array from the rest of the members in the
>>> flexible structure, and use the tagged `struct create_context_hdr`
>>> instead of `struct create_context`.
>>>
>>> So, with these changes, fix 51 of the following warnings[1]:
>>>
>>> fs/smb/client/../common/smb2pdu.h:1225:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>>
>>> Link: https://gist.github.com/GustavoARSilva/772526a39be3dd4db39e71497f0a9893 [1]
>>> Link: https://github.com/KSPP/linux/issues/202
>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>> ---
>>>    fs/smb/client/smb2pdu.h | 12 ++++++------
>>>    fs/smb/common/smb2pdu.h | 33 ++++++++++++++++++---------------
>>>    fs/smb/server/smb2pdu.h | 18 +++++++++---------
>>>    3 files changed, 33 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
>>> index c72a3b2886b7..1a02bd9e0c00 100644
>>> --- a/fs/smb/client/smb2pdu.h
>>> +++ b/fs/smb/client/smb2pdu.h
>>> @@ -145,7 +145,7 @@ struct durable_context_v2 {
>>>    } __packed;
>>>
>>>    struct create_durable_v2 {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        struct durable_context_v2 dcontext;
>>>    } __packed;
>>> @@ -167,7 +167,7 @@ struct durable_reconnect_context_v2_rsp {
>>>    } __packed;
>>>
>>>    struct create_durable_handle_reconnect_v2 {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        struct durable_reconnect_context_v2 dcontext;
>>>        __u8   Pad[4];
>>> @@ -175,7 +175,7 @@ struct create_durable_handle_reconnect_v2 {
>>>
>>>    /* See MS-SMB2 2.2.13.2.5 */
>>>    struct crt_twarp_ctxt {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8    Name[8];
>>>        __le64  Timestamp;
>>>
>>> @@ -183,12 +183,12 @@ struct crt_twarp_ctxt {
>>>
>>>    /* See MS-SMB2 2.2.13.2.9 */
>>>    struct crt_query_id_ctxt {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8    Name[8];
>>>    } __packed;
>>>
>>>    struct crt_sd_ctxt {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8    Name[8];
>>>        struct smb3_sd sd;
>>>    } __packed;
>>> @@ -415,7 +415,7 @@ struct smb2_posix_info_parsed {
>>>    };
>>>
>>>    struct smb2_create_ea_ctx {
>>> -     struct create_context ctx;
>>> +     struct create_context_hdr ctx;
>>>        __u8 name[8];
>>>        struct smb2_file_full_ea_info ea;
>>>    } __packed;
>>> diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
>>> index 1b594307c9d5..eab9d49c63ba 100644
>>> --- a/fs/smb/common/smb2pdu.h
>>> +++ b/fs/smb/common/smb2pdu.h
>>> @@ -1171,12 +1171,15 @@ struct smb2_server_client_notification {
>>>    #define SMB2_CREATE_FLAG_REPARSEPOINT 0x01
>>>
>>>    struct create_context {
>>> -     __le32 Next;
>>> -     __le16 NameOffset;
>>> -     __le16 NameLength;
>>> -     __le16 Reserved;
>>> -     __le16 DataOffset;
>>> -     __le32 DataLength;
>>> +     /* New members must be added within the struct_group() macro below. */
>>> +     __struct_group(create_context_hdr, hdr, __packed,
>>> +             __le32 Next;
>>> +             __le16 NameOffset;
>>> +             __le16 NameLength;
>>> +             __le16 Reserved;
>>> +             __le16 DataOffset;
>>> +             __le32 DataLength;
>>> +     );
>>>        __u8 Buffer[];
>>>    } __packed;
>>>
>>> @@ -1222,7 +1225,7 @@ struct smb2_create_rsp {
>>>    } __packed;
>>>
>>>    struct create_posix {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8    Name[16];
>>>        __le32  Mode;
>>>        __u32   Reserved;
>>> @@ -1230,7 +1233,7 @@ struct create_posix {
>>>
>>>    /* See MS-SMB2 2.2.13.2.3 and MS-SMB2 2.2.13.2.4 */
>>>    struct create_durable {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        union {
>>>                __u8  Reserved[16];
>>> @@ -1243,14 +1246,14 @@ struct create_durable {
>>>
>>>    /* See MS-SMB2 2.2.13.2.5 */
>>>    struct create_mxac_req {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        __le64 Timestamp;
>>>    } __packed;
>>>
>>>    /* See MS-SMB2 2.2.14.2.5 */
>>>    struct create_mxac_rsp {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        __le32 QueryStatus;
>>>        __le32 MaximalAccess;
>>> @@ -1286,13 +1289,13 @@ struct lease_context_v2 {
>>>    } __packed;
>>>
>>>    struct create_lease {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        struct lease_context lcontext;
>>>    } __packed;
>>>
>>>    struct create_lease_v2 {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        struct lease_context_v2 lcontext;
>>>        __u8   Pad[4];
>>> @@ -1300,7 +1303,7 @@ struct create_lease_v2 {
>>>
>>>    /* See MS-SMB2 2.2.14.2.9 */
>>>    struct create_disk_id_rsp {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        __le64 DiskFileId;
>>>        __le64 VolumeId;
>>> @@ -1309,7 +1312,7 @@ struct create_disk_id_rsp {
>>>
>>>    /* See MS-SMB2 2.2.13.2.13 */
>>>    struct create_app_inst_id {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8 Name[16];
>>>        __le32 StructureSize; /* Must be 20 */
>>>        __u16 Reserved;
>>> @@ -1318,7 +1321,7 @@ struct create_app_inst_id {
>>>
>>>    /* See MS-SMB2 2.2.13.2.15 */
>>>    struct create_app_inst_id_vers {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8 Name[16];
>>>        __le32 StructureSize; /* Must be 24 */
>>>        __u16 Reserved;
>>> diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
>>> index bd1d2a0e9203..643f5e1cfe35 100644
>>> --- a/fs/smb/server/smb2pdu.h
>>> +++ b/fs/smb/server/smb2pdu.h
>>> @@ -64,7 +64,7 @@ struct preauth_integrity_info {
>>>    #define SMB2_SESSION_TIMEOUT                (10 * HZ)
>>>
>>>    struct create_durable_req_v2 {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        __le32 Timeout;
>>>        __le32 Flags;
>>> @@ -73,7 +73,7 @@ struct create_durable_req_v2 {
>>>    } __packed;
>>>
>>>    struct create_durable_reconn_req {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        union {
>>>                __u8  Reserved[16];
>>> @@ -85,7 +85,7 @@ struct create_durable_reconn_req {
>>>    } __packed;
>>>
>>>    struct create_durable_reconn_v2_req {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        struct {
>>>                __u64 PersistentFileId;
>>> @@ -96,13 +96,13 @@ struct create_durable_reconn_v2_req {
>>>    } __packed;
>>>
>>>    struct create_alloc_size_req {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        __le64 AllocationSize;
>>>    } __packed;
>>>
>>>    struct create_durable_rsp {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        union {
>>>                __u8  Reserved[8];
>>> @@ -114,7 +114,7 @@ struct create_durable_rsp {
>>>    /* Flags */
>>>    #define SMB2_DHANDLE_FLAG_PERSISTENT        0x00000002
>>>    struct create_durable_v2_rsp {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        __le32 Timeout;
>>>        __le32 Flags;
>>> @@ -122,7 +122,7 @@ struct create_durable_v2_rsp {
>>>
>>>    /* equivalent of the contents of SMB3.1.1 POSIX open context response */
>>>    struct create_posix_rsp {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8    Name[16];
>>>        __le32 nlink;
>>>        __le32 reparse_tag;
>>> @@ -381,13 +381,13 @@ struct smb2_ea_info {
>>>    } __packed; /* level 15 Query */
>>>
>>>    struct create_ea_buf_req {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        struct smb2_ea_info ea;
>>>    } __packed;
>>>
>>>    struct create_sd_buf_req {
>>> -     struct create_context ccontext;
>>> +     struct create_context_hdr ccontext;
>>>        __u8   Name[8];
>>>        struct smb_ntsd ntsd;
>>>    } __packed;
>>
> 
> 
> --
> Thanks,
> 
> Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings
  2024-04-23 20:47     ` Gustavo A. R. Silva
@ 2024-04-23 21:08       ` Gustavo A. R. Silva
  2024-04-23 21:09       ` Steve French
  1 sibling, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2024-04-23 21:08 UTC (permalink / raw)
  To: Steve French
  Cc: Gustavo A. R. Silva, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	Namjae Jeon, Sergey Senozhatsky, linux-cifs, samba-technical,
	linux-kernel, linux-hardening, Kees Cook



On 23/04/24 14:47, Gustavo A. R. Silva wrote:
> 
> 
> On 23/04/24 14:15, Steve French wrote:
>> This looks reasonably safe (running the usual regression tests on it now).
>>
>> Reminds me though that we have to be careful (e.g. the recent fix for
>> regression caused by cleanup).
> 
> mmh... it seems that the offending commit was never CC'd to the linux-hardening
> list, hence it wasn't reviewed by us.
> 
> After reviewing both, the offending commit and the fix, both seem to be wrong.
> 
> for __packed structs, you should use __struct_group():
> 
> __struct_group(network_open_info, group_name, __packed, struct_members);
> 
> The _packed in the commit 0268a7cc7fdc is not an attribute, it's the name
> for the group. So, it's not actually doing what the submitter thinks it does.

Sorry about this, I'm still too sleep deprived, aghr... I confused the suffix
_attr with _tag.

The fix is correct!

Thanks!
--
Gustavo

> 
>>
>> Thoughts about whether should be sent in rc6 or wait till 6.10?  51
>> warnings does sound
>> distracting though so might be worth going in sooner rather than later.
> 
> There is actually no hurry. :)
> 
> Thanks
> -- 
> Gustavo
> 
>>
>> commit 0268a7cc7fdc47d90b6c18859de7718d5059f6f1
>> Author: Namjae Jeon <linkinjeon@kernel.org>
>> Date:   Fri Apr 19 23:46:34 2024 +0900
>>
>>      ksmbd: common: use struct_group_attr instead of struct_group for
>> network_open_info
>>
>>      4byte padding cause the connection issue with the applications of MacOS.
>>      smb2_close response size increases by 4 bytes by padding, And the smb
>>      client of MacOS check it and stop the connection. This patch use
>>      struct_group_attr instead of struct_group for network_open_info to use
>>       __packed to avoid padding.
>>
>>
>> On Tue, Apr 23, 2024 at 1:58 PM Gustavo A. R. Silva
>> <gustavo@embeddedor.com> wrote:
>>>
>>> Hi all,
>>>
>>> Friendly ping: who can take this, please?
>>>
>>> Thanks
>>> -- 
>>> Gustavo
>>>
>>> On 11/04/24 09:35, Gustavo A. R. Silva wrote:
>>>> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
>>>> ready to enable it globally.
>>>>
>>>> So, in order to avoid ending up with a flexible-array member in the
>>>> middle of multiple other structs, we use the `__struct_group()` helper
>>>> to separate the flexible array from the rest of the members in the
>>>> flexible structure, and use the tagged `struct create_context_hdr`
>>>> instead of `struct create_context`.
>>>>
>>>> So, with these changes, fix 51 of the following warnings[1]:
>>>>
>>>> fs/smb/client/../common/smb2pdu.h:1225:31: warning: structure containing a flexible array member is not at the end of another structure 
>>>> [-Wflex-array-member-not-at-end]
>>>>
>>>> Link: https://gist.github.com/GustavoARSilva/772526a39be3dd4db39e71497f0a9893 [1]
>>>> Link: https://github.com/KSPP/linux/issues/202
>>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>>> ---
>>>>    fs/smb/client/smb2pdu.h | 12 ++++++------
>>>>    fs/smb/common/smb2pdu.h | 33 ++++++++++++++++++---------------
>>>>    fs/smb/server/smb2pdu.h | 18 +++++++++---------
>>>>    3 files changed, 33 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
>>>> index c72a3b2886b7..1a02bd9e0c00 100644
>>>> --- a/fs/smb/client/smb2pdu.h
>>>> +++ b/fs/smb/client/smb2pdu.h
>>>> @@ -145,7 +145,7 @@ struct durable_context_v2 {
>>>>    } __packed;
>>>>
>>>>    struct create_durable_v2 {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        struct durable_context_v2 dcontext;
>>>>    } __packed;
>>>> @@ -167,7 +167,7 @@ struct durable_reconnect_context_v2_rsp {
>>>>    } __packed;
>>>>
>>>>    struct create_durable_handle_reconnect_v2 {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        struct durable_reconnect_context_v2 dcontext;
>>>>        __u8   Pad[4];
>>>> @@ -175,7 +175,7 @@ struct create_durable_handle_reconnect_v2 {
>>>>
>>>>    /* See MS-SMB2 2.2.13.2.5 */
>>>>    struct crt_twarp_ctxt {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8    Name[8];
>>>>        __le64  Timestamp;
>>>>
>>>> @@ -183,12 +183,12 @@ struct crt_twarp_ctxt {
>>>>
>>>>    /* See MS-SMB2 2.2.13.2.9 */
>>>>    struct crt_query_id_ctxt {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8    Name[8];
>>>>    } __packed;
>>>>
>>>>    struct crt_sd_ctxt {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8    Name[8];
>>>>        struct smb3_sd sd;
>>>>    } __packed;
>>>> @@ -415,7 +415,7 @@ struct smb2_posix_info_parsed {
>>>>    };
>>>>
>>>>    struct smb2_create_ea_ctx {
>>>> -     struct create_context ctx;
>>>> +     struct create_context_hdr ctx;
>>>>        __u8 name[8];
>>>>        struct smb2_file_full_ea_info ea;
>>>>    } __packed;
>>>> diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
>>>> index 1b594307c9d5..eab9d49c63ba 100644
>>>> --- a/fs/smb/common/smb2pdu.h
>>>> +++ b/fs/smb/common/smb2pdu.h
>>>> @@ -1171,12 +1171,15 @@ struct smb2_server_client_notification {
>>>>    #define SMB2_CREATE_FLAG_REPARSEPOINT 0x01
>>>>
>>>>    struct create_context {
>>>> -     __le32 Next;
>>>> -     __le16 NameOffset;
>>>> -     __le16 NameLength;
>>>> -     __le16 Reserved;
>>>> -     __le16 DataOffset;
>>>> -     __le32 DataLength;
>>>> +     /* New members must be added within the struct_group() macro below. */
>>>> +     __struct_group(create_context_hdr, hdr, __packed,
>>>> +             __le32 Next;
>>>> +             __le16 NameOffset;
>>>> +             __le16 NameLength;
>>>> +             __le16 Reserved;
>>>> +             __le16 DataOffset;
>>>> +             __le32 DataLength;
>>>> +     );
>>>>        __u8 Buffer[];
>>>>    } __packed;
>>>>
>>>> @@ -1222,7 +1225,7 @@ struct smb2_create_rsp {
>>>>    } __packed;
>>>>
>>>>    struct create_posix {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8    Name[16];
>>>>        __le32  Mode;
>>>>        __u32   Reserved;
>>>> @@ -1230,7 +1233,7 @@ struct create_posix {
>>>>
>>>>    /* See MS-SMB2 2.2.13.2.3 and MS-SMB2 2.2.13.2.4 */
>>>>    struct create_durable {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        union {
>>>>                __u8  Reserved[16];
>>>> @@ -1243,14 +1246,14 @@ struct create_durable {
>>>>
>>>>    /* See MS-SMB2 2.2.13.2.5 */
>>>>    struct create_mxac_req {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        __le64 Timestamp;
>>>>    } __packed;
>>>>
>>>>    /* See MS-SMB2 2.2.14.2.5 */
>>>>    struct create_mxac_rsp {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        __le32 QueryStatus;
>>>>        __le32 MaximalAccess;
>>>> @@ -1286,13 +1289,13 @@ struct lease_context_v2 {
>>>>    } __packed;
>>>>
>>>>    struct create_lease {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        struct lease_context lcontext;
>>>>    } __packed;
>>>>
>>>>    struct create_lease_v2 {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        struct lease_context_v2 lcontext;
>>>>        __u8   Pad[4];
>>>> @@ -1300,7 +1303,7 @@ struct create_lease_v2 {
>>>>
>>>>    /* See MS-SMB2 2.2.14.2.9 */
>>>>    struct create_disk_id_rsp {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        __le64 DiskFileId;
>>>>        __le64 VolumeId;
>>>> @@ -1309,7 +1312,7 @@ struct create_disk_id_rsp {
>>>>
>>>>    /* See MS-SMB2 2.2.13.2.13 */
>>>>    struct create_app_inst_id {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8 Name[16];
>>>>        __le32 StructureSize; /* Must be 20 */
>>>>        __u16 Reserved;
>>>> @@ -1318,7 +1321,7 @@ struct create_app_inst_id {
>>>>
>>>>    /* See MS-SMB2 2.2.13.2.15 */
>>>>    struct create_app_inst_id_vers {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8 Name[16];
>>>>        __le32 StructureSize; /* Must be 24 */
>>>>        __u16 Reserved;
>>>> diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
>>>> index bd1d2a0e9203..643f5e1cfe35 100644
>>>> --- a/fs/smb/server/smb2pdu.h
>>>> +++ b/fs/smb/server/smb2pdu.h
>>>> @@ -64,7 +64,7 @@ struct preauth_integrity_info {
>>>>    #define SMB2_SESSION_TIMEOUT                (10 * HZ)
>>>>
>>>>    struct create_durable_req_v2 {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        __le32 Timeout;
>>>>        __le32 Flags;
>>>> @@ -73,7 +73,7 @@ struct create_durable_req_v2 {
>>>>    } __packed;
>>>>
>>>>    struct create_durable_reconn_req {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        union {
>>>>                __u8  Reserved[16];
>>>> @@ -85,7 +85,7 @@ struct create_durable_reconn_req {
>>>>    } __packed;
>>>>
>>>>    struct create_durable_reconn_v2_req {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        struct {
>>>>                __u64 PersistentFileId;
>>>> @@ -96,13 +96,13 @@ struct create_durable_reconn_v2_req {
>>>>    } __packed;
>>>>
>>>>    struct create_alloc_size_req {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        __le64 AllocationSize;
>>>>    } __packed;
>>>>
>>>>    struct create_durable_rsp {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        union {
>>>>                __u8  Reserved[8];
>>>> @@ -114,7 +114,7 @@ struct create_durable_rsp {
>>>>    /* Flags */
>>>>    #define SMB2_DHANDLE_FLAG_PERSISTENT        0x00000002
>>>>    struct create_durable_v2_rsp {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        __le32 Timeout;
>>>>        __le32 Flags;
>>>> @@ -122,7 +122,7 @@ struct create_durable_v2_rsp {
>>>>
>>>>    /* equivalent of the contents of SMB3.1.1 POSIX open context response */
>>>>    struct create_posix_rsp {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8    Name[16];
>>>>        __le32 nlink;
>>>>        __le32 reparse_tag;
>>>> @@ -381,13 +381,13 @@ struct smb2_ea_info {
>>>>    } __packed; /* level 15 Query */
>>>>
>>>>    struct create_ea_buf_req {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        struct smb2_ea_info ea;
>>>>    } __packed;
>>>>
>>>>    struct create_sd_buf_req {
>>>> -     struct create_context ccontext;
>>>> +     struct create_context_hdr ccontext;
>>>>        __u8   Name[8];
>>>>        struct smb_ntsd ntsd;
>>>>    } __packed;
>>>
>>
>>
>> -- 
>> Thanks,
>>
>> Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings
  2024-04-23 20:47     ` Gustavo A. R. Silva
  2024-04-23 21:08       ` Gustavo A. R. Silva
@ 2024-04-23 21:09       ` Steve French
  2024-04-23 21:26         ` Gustavo A. R. Silva
  1 sibling, 1 reply; 7+ messages in thread
From: Steve French @ 2024-04-23 21:09 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Gustavo A. R. Silva, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	Namjae Jeon, Sergey Senozhatsky, linux-cifs, samba-technical,
	linux-kernel, linux-hardening, Kees Cook

On Tue, Apr 23, 2024 at 3:48 PM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
>
>
> On 23/04/24 14:15, Steve French wrote:
> > This looks reasonably safe (running the usual regression tests on it now).
> >
> > Reminds me though that we have to be careful (e.g. the recent fix for
> > regression caused by cleanup).
>
> mmh... it seems that the offending commit was never CC'd to the linux-hardening
> list, hence it wasn't reviewed by us.
>
> After reviewing both, the offending commit and the fix, both seem to be wrong.
>
> for __packed structs, you should use __struct_group():
>
> __struct_group(network_open_info, group_name, __packed, struct_members);
>
> The _packed in the commit 0268a7cc7fdc is not an attribute, it's the name
> for the group. So, it's not actually doing what the submitter thinks it does.

Do you want to submit a followup fix to fix this?  Or let Namjae fix it?





> > Thoughts about whether should be sent in rc6 or wait till 6.10?  51
> > warnings does sound
> > distracting though so might be worth going in sooner rather than later.
>
> There is actually no hurry. :)
>
> Thanks
> --
> Gustavo
>
> >
> > commit 0268a7cc7fdc47d90b6c18859de7718d5059f6f1
> > Author: Namjae Jeon <linkinjeon@kernel.org>
> > Date:   Fri Apr 19 23:46:34 2024 +0900
> >
> >      ksmbd: common: use struct_group_attr instead of struct_group for
> > network_open_info
> >
> >      4byte padding cause the connection issue with the applications of MacOS.
> >      smb2_close response size increases by 4 bytes by padding, And the smb
> >      client of MacOS check it and stop the connection. This patch use
> >      struct_group_attr instead of struct_group for network_open_info to use
> >       __packed to avoid padding.
> >
> >
> > On Tue, Apr 23, 2024 at 1:58 PM Gustavo A. R. Silva
> > <gustavo@embeddedor.com> wrote:
> >>
> >> Hi all,
> >>
> >> Friendly ping: who can take this, please?
> >>
> >> Thanks
> >> --
> >> Gustavo
> >>
> >> On 11/04/24 09:35, Gustavo A. R. Silva wrote:
> >>> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> >>> ready to enable it globally.
> >>>
> >>> So, in order to avoid ending up with a flexible-array member in the
> >>> middle of multiple other structs, we use the `__struct_group()` helper
> >>> to separate the flexible array from the rest of the members in the
> >>> flexible structure, and use the tagged `struct create_context_hdr`
> >>> instead of `struct create_context`.
> >>>
> >>> So, with these changes, fix 51 of the following warnings[1]:
> >>>
> >>> fs/smb/client/../common/smb2pdu.h:1225:31: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> >>>
> >>> Link: https://gist.github.com/GustavoARSilva/772526a39be3dd4db39e71497f0a9893 [1]
> >>> Link: https://github.com/KSPP/linux/issues/202
> >>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> >>> ---
> >>>    fs/smb/client/smb2pdu.h | 12 ++++++------
> >>>    fs/smb/common/smb2pdu.h | 33 ++++++++++++++++++---------------
> >>>    fs/smb/server/smb2pdu.h | 18 +++++++++---------
> >>>    3 files changed, 33 insertions(+), 30 deletions(-)
> >>>
> >>> diff --git a/fs/smb/client/smb2pdu.h b/fs/smb/client/smb2pdu.h
> >>> index c72a3b2886b7..1a02bd9e0c00 100644
> >>> --- a/fs/smb/client/smb2pdu.h
> >>> +++ b/fs/smb/client/smb2pdu.h
> >>> @@ -145,7 +145,7 @@ struct durable_context_v2 {
> >>>    } __packed;
> >>>
> >>>    struct create_durable_v2 {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        struct durable_context_v2 dcontext;
> >>>    } __packed;
> >>> @@ -167,7 +167,7 @@ struct durable_reconnect_context_v2_rsp {
> >>>    } __packed;
> >>>
> >>>    struct create_durable_handle_reconnect_v2 {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        struct durable_reconnect_context_v2 dcontext;
> >>>        __u8   Pad[4];
> >>> @@ -175,7 +175,7 @@ struct create_durable_handle_reconnect_v2 {
> >>>
> >>>    /* See MS-SMB2 2.2.13.2.5 */
> >>>    struct crt_twarp_ctxt {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8    Name[8];
> >>>        __le64  Timestamp;
> >>>
> >>> @@ -183,12 +183,12 @@ struct crt_twarp_ctxt {
> >>>
> >>>    /* See MS-SMB2 2.2.13.2.9 */
> >>>    struct crt_query_id_ctxt {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8    Name[8];
> >>>    } __packed;
> >>>
> >>>    struct crt_sd_ctxt {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8    Name[8];
> >>>        struct smb3_sd sd;
> >>>    } __packed;
> >>> @@ -415,7 +415,7 @@ struct smb2_posix_info_parsed {
> >>>    };
> >>>
> >>>    struct smb2_create_ea_ctx {
> >>> -     struct create_context ctx;
> >>> +     struct create_context_hdr ctx;
> >>>        __u8 name[8];
> >>>        struct smb2_file_full_ea_info ea;
> >>>    } __packed;
> >>> diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
> >>> index 1b594307c9d5..eab9d49c63ba 100644
> >>> --- a/fs/smb/common/smb2pdu.h
> >>> +++ b/fs/smb/common/smb2pdu.h
> >>> @@ -1171,12 +1171,15 @@ struct smb2_server_client_notification {
> >>>    #define SMB2_CREATE_FLAG_REPARSEPOINT 0x01
> >>>
> >>>    struct create_context {
> >>> -     __le32 Next;
> >>> -     __le16 NameOffset;
> >>> -     __le16 NameLength;
> >>> -     __le16 Reserved;
> >>> -     __le16 DataOffset;
> >>> -     __le32 DataLength;
> >>> +     /* New members must be added within the struct_group() macro below. */
> >>> +     __struct_group(create_context_hdr, hdr, __packed,
> >>> +             __le32 Next;
> >>> +             __le16 NameOffset;
> >>> +             __le16 NameLength;
> >>> +             __le16 Reserved;
> >>> +             __le16 DataOffset;
> >>> +             __le32 DataLength;
> >>> +     );
> >>>        __u8 Buffer[];
> >>>    } __packed;
> >>>
> >>> @@ -1222,7 +1225,7 @@ struct smb2_create_rsp {
> >>>    } __packed;
> >>>
> >>>    struct create_posix {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8    Name[16];
> >>>        __le32  Mode;
> >>>        __u32   Reserved;
> >>> @@ -1230,7 +1233,7 @@ struct create_posix {
> >>>
> >>>    /* See MS-SMB2 2.2.13.2.3 and MS-SMB2 2.2.13.2.4 */
> >>>    struct create_durable {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        union {
> >>>                __u8  Reserved[16];
> >>> @@ -1243,14 +1246,14 @@ struct create_durable {
> >>>
> >>>    /* See MS-SMB2 2.2.13.2.5 */
> >>>    struct create_mxac_req {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        __le64 Timestamp;
> >>>    } __packed;
> >>>
> >>>    /* See MS-SMB2 2.2.14.2.5 */
> >>>    struct create_mxac_rsp {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        __le32 QueryStatus;
> >>>        __le32 MaximalAccess;
> >>> @@ -1286,13 +1289,13 @@ struct lease_context_v2 {
> >>>    } __packed;
> >>>
> >>>    struct create_lease {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        struct lease_context lcontext;
> >>>    } __packed;
> >>>
> >>>    struct create_lease_v2 {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        struct lease_context_v2 lcontext;
> >>>        __u8   Pad[4];
> >>> @@ -1300,7 +1303,7 @@ struct create_lease_v2 {
> >>>
> >>>    /* See MS-SMB2 2.2.14.2.9 */
> >>>    struct create_disk_id_rsp {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        __le64 DiskFileId;
> >>>        __le64 VolumeId;
> >>> @@ -1309,7 +1312,7 @@ struct create_disk_id_rsp {
> >>>
> >>>    /* See MS-SMB2 2.2.13.2.13 */
> >>>    struct create_app_inst_id {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8 Name[16];
> >>>        __le32 StructureSize; /* Must be 20 */
> >>>        __u16 Reserved;
> >>> @@ -1318,7 +1321,7 @@ struct create_app_inst_id {
> >>>
> >>>    /* See MS-SMB2 2.2.13.2.15 */
> >>>    struct create_app_inst_id_vers {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8 Name[16];
> >>>        __le32 StructureSize; /* Must be 24 */
> >>>        __u16 Reserved;
> >>> diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
> >>> index bd1d2a0e9203..643f5e1cfe35 100644
> >>> --- a/fs/smb/server/smb2pdu.h
> >>> +++ b/fs/smb/server/smb2pdu.h
> >>> @@ -64,7 +64,7 @@ struct preauth_integrity_info {
> >>>    #define SMB2_SESSION_TIMEOUT                (10 * HZ)
> >>>
> >>>    struct create_durable_req_v2 {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        __le32 Timeout;
> >>>        __le32 Flags;
> >>> @@ -73,7 +73,7 @@ struct create_durable_req_v2 {
> >>>    } __packed;
> >>>
> >>>    struct create_durable_reconn_req {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        union {
> >>>                __u8  Reserved[16];
> >>> @@ -85,7 +85,7 @@ struct create_durable_reconn_req {
> >>>    } __packed;
> >>>
> >>>    struct create_durable_reconn_v2_req {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        struct {
> >>>                __u64 PersistentFileId;
> >>> @@ -96,13 +96,13 @@ struct create_durable_reconn_v2_req {
> >>>    } __packed;
> >>>
> >>>    struct create_alloc_size_req {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        __le64 AllocationSize;
> >>>    } __packed;
> >>>
> >>>    struct create_durable_rsp {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        union {
> >>>                __u8  Reserved[8];
> >>> @@ -114,7 +114,7 @@ struct create_durable_rsp {
> >>>    /* Flags */
> >>>    #define SMB2_DHANDLE_FLAG_PERSISTENT        0x00000002
> >>>    struct create_durable_v2_rsp {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        __le32 Timeout;
> >>>        __le32 Flags;
> >>> @@ -122,7 +122,7 @@ struct create_durable_v2_rsp {
> >>>
> >>>    /* equivalent of the contents of SMB3.1.1 POSIX open context response */
> >>>    struct create_posix_rsp {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8    Name[16];
> >>>        __le32 nlink;
> >>>        __le32 reparse_tag;
> >>> @@ -381,13 +381,13 @@ struct smb2_ea_info {
> >>>    } __packed; /* level 15 Query */
> >>>
> >>>    struct create_ea_buf_req {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        struct smb2_ea_info ea;
> >>>    } __packed;
> >>>
> >>>    struct create_sd_buf_req {
> >>> -     struct create_context ccontext;
> >>> +     struct create_context_hdr ccontext;
> >>>        __u8   Name[8];
> >>>        struct smb_ntsd ntsd;
> >>>    } __packed;
> >>
> >
> >
> > --
> > Thanks,
> >
> > Steve



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings
  2024-04-23 21:09       ` Steve French
@ 2024-04-23 21:26         ` Gustavo A. R. Silva
  0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2024-04-23 21:26 UTC (permalink / raw)
  To: Steve French
  Cc: Gustavo A. R. Silva, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	Namjae Jeon, Sergey Senozhatsky, linux-cifs, samba-technical,
	linux-kernel, linux-hardening, Kees Cook


>> The _packed in the commit 0268a7cc7fdc is not an attribute, it's the name
>> for the group. So, it's not actually doing what the submitter thinks it does.
> 
> Do you want to submit a followup fix to fix this?  Or let Namjae fix it?
> 

The fix is correct. I'm sorry, I confused the suffix `_attr` with `_tagged` in
the struct_group() family of functions.

I've been in airports the last 24 hours, and I my brain needs some rest.

Thanks!
--
Gustavo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-04-23 21:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-11 15:35 [PATCH][next] smb: smb2pdu.h: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
2024-04-23 18:57 ` Gustavo A. R. Silva
2024-04-23 20:15   ` Steve French
2024-04-23 20:47     ` Gustavo A. R. Silva
2024-04-23 21:08       ` Gustavo A. R. Silva
2024-04-23 21:09       ` Steve French
2024-04-23 21:26         ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.