[next] cifs: smb2pdu.h: Replace zero-length array with flexible-array member
diff mbox series

Message ID 20200309154451.GA31357@embeddedor
State New
Headers show
Series
  • [next] cifs: smb2pdu.h: Replace zero-length array with flexible-array member
Related show

Commit Message

Gustavo A. R. Silva March 9, 2020, 3:44 p.m. UTC
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:

struct foo {
        int stuff;
        struct boo array[];
};

By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.

Also, notice that, dynamic memory allocations won't be affected by
this change:

"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]

This issue was found with the help of Coccinelle.

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 fs/cifs/smb2pdu.h | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

Comments

Steve French March 9, 2020, 9:41 p.m. UTC | #1
merged into cifs-2.6.git for-next

On Mon, Mar 9, 2020 at 11:04 AM Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
>
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
>         int stuff;
>         struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure, which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  fs/cifs/smb2pdu.h | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index ad14b8505b4d..817bc0531536 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -182,7 +182,7 @@ struct smb2_symlink_err_rsp {
>         __le16 PrintNameOffset;
>         __le16 PrintNameLength;
>         __le32 Flags;
> -       __u8  PathBuffer[0];
> +       __u8  PathBuffer[];
>  } __packed;
>
>  /* SMB 3.1.1 and later dialects. See MS-SMB2 section 2.2.2.1 */
> @@ -210,7 +210,7 @@ struct share_redirect_error_context_rsp {
>         __le16 Flags;
>         __le16 TargetType;
>         __le32 IPAddrCount;
> -       struct move_dst_ipaddr IpAddrMoveList[0];
> +       struct move_dst_ipaddr IpAddrMoveList[];
>         /* __u8 ResourceName[] */ /* Name of share as counted Unicode string */
>  } __packed;
>
> @@ -326,7 +326,7 @@ struct smb2_netname_neg_context {
>         __le16  ContextType; /* 0x100 */
>         __le16  DataLength;
>         __le32  Reserved;
> -       __le16  NetName[0]; /* hostname of target converted to UCS-2 */
> +       __le16  NetName[]; /* hostname of target converted to UCS-2 */
>  } __packed;
>
>  #define POSIX_CTXT_DATA_LEN    16
> @@ -421,13 +421,13 @@ struct tree_connect_contexts {
>         __le16 ContextType;
>         __le16 DataLength;
>         __le32 Reserved;
> -       __u8   Data[0];
> +       __u8   Data[];
>  } __packed;
>
>  /* Remoted identity tree connect context structures - see MS-SMB2 2.2.9.2.1 */
>  struct smb3_blob_data {
>         __le16 BlobSize;
> -       __u8   BlobData[0];
> +       __u8   BlobData[];
>  } __packed;
>
>  /* Valid values for Attr */
> @@ -477,14 +477,14 @@ struct remoted_identity_tcon_context {
>         __le16 DeviceGroups; /* offset to SID_ARRAY_DATA struct */
>         __le16 UserClaims; /* offset to BLOB_DATA struct */
>         __le16 DeviceClaims; /* offset to BLOB_DATA struct */
> -       __u8   TicketInfo[0]; /* variable length buf - remoted identity data */
> +       __u8   TicketInfo[]; /* variable length buf - remoted identity data */
>  } __packed;
>
>  struct smb2_tree_connect_req_extension {
>         __le32 TreeConnectContextOffset;
>         __le16 TreeConnectContextCount;
>         __u8  Reserved[10];
> -       __u8  PathName[0]; /* variable sized array */
> +       __u8  PathName[]; /* variable sized array */
>         /* followed by array of TreeConnectContexts */
>  } __packed;
>
> @@ -689,7 +689,7 @@ struct smb2_create_req {
>         __le16 NameLength;
>         __le32 CreateContextsOffset;
>         __le32 CreateContextsLength;
> -       __u8   Buffer[0];
> +       __u8   Buffer[];
>  } __packed;
>
>  /*
> @@ -727,7 +727,7 @@ struct create_context {
>         __le16 Reserved;
>         __le16 DataOffset;
>         __le32 DataLength;
> -       __u8 Buffer[0];
> +       __u8 Buffer[];
>  } __packed;
>
>  #define SMB2_LEASE_READ_CACHING_HE     0x01
> @@ -869,7 +869,7 @@ struct crt_sd_ctxt {
>  struct resume_key_req {
>         char ResumeKey[COPY_CHUNK_RES_KEY_SIZE];
>         __le32  ContextLength;  /* MBZ */
> -       char    Context[0];     /* ignored, Windows sets to 4 bytes of zero */
> +       char    Context[];      /* ignored, Windows sets to 4 bytes of zero */
>  } __packed;
>
>  /* this goes in the ioctl buffer when doing a copychunk request */
> @@ -931,7 +931,7 @@ struct reparse_data_buffer {
>         __le32  ReparseTag;
>         __le16  ReparseDataLength;
>         __u16   Reserved;
> -       __u8    DataBuffer[0]; /* Variable Length */
> +       __u8    DataBuffer[]; /* Variable Length */
>  } __packed;
>
>  struct reparse_guid_data_buffer {
> @@ -939,7 +939,7 @@ struct reparse_guid_data_buffer {
>         __le16  ReparseDataLength;
>         __u16   Reserved;
>         __u8    ReparseGuid[16];
> -       __u8    DataBuffer[0]; /* Variable Length */
> +       __u8    DataBuffer[]; /* Variable Length */
>  } __packed;
>
>  struct reparse_mount_point_data_buffer {
> @@ -950,7 +950,7 @@ struct reparse_mount_point_data_buffer {
>         __le16  SubstituteNameLength;
>         __le16  PrintNameOffset;
>         __le16  PrintNameLength;
> -       __u8    PathBuffer[0]; /* Variable Length */
> +       __u8    PathBuffer[]; /* Variable Length */
>  } __packed;
>
>  #define SYMLINK_FLAG_RELATIVE 0x00000001
> @@ -964,7 +964,7 @@ struct reparse_symlink_data_buffer {
>         __le16  PrintNameOffset;
>         __le16  PrintNameLength;
>         __le32  Flags;
> -       __u8    PathBuffer[0]; /* Variable Length */
> +       __u8    PathBuffer[]; /* Variable Length */
>  } __packed;
>
>  /* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_posix_data */
> @@ -1066,7 +1066,7 @@ struct smb2_ioctl_req {
>         __le32 MaxOutputResponse;
>         __le32 Flags;
>         __u32  Reserved2;
> -       __u8   Buffer[0];
> +       __u8   Buffer[];
>  } __packed;
>
>  struct smb2_ioctl_rsp {
> @@ -1469,7 +1469,7 @@ struct smb3_fs_vol_info {
>         __le32  VolumeLabelLength; /* includes trailing null */
>         __u8    SupportsObjects; /* True if eg like NTFS, supports objects */
>         __u8    Reserved;
> -       __u8    VolumeLabel[0]; /* variable len */
> +       __u8    VolumeLabel[]; /* variable len */
>  } __packed;
>
>  /* partial list of QUERY INFO levels */
> @@ -1531,7 +1531,7 @@ struct smb2_file_rename_info { /* encoding of request for level 10 */
>         __u8   Reserved[7];
>         __u64  RootDirectory;  /* MBZ for network operations (why says spec?) */
>         __le32 FileNameLength;
> -       char   FileName[0];     /* New name to be assigned */
> +       char   FileName[];     /* New name to be assigned */
>  } __packed; /* level 10 Set */
>
>  struct smb2_file_link_info { /* encoding of request for level 11 */
> @@ -1540,7 +1540,7 @@ struct smb2_file_link_info { /* encoding of request for level 11 */
>         __u8   Reserved[7];
>         __u64  RootDirectory;  /* MBZ for network operations (why says spec?) */
>         __le32 FileNameLength;
> -       char   FileName[0];     /* Name to be assigned to new link */
> +       char   FileName[];     /* Name to be assigned to new link */
>  } __packed; /* level 11 Set */
>
>  struct smb2_file_full_ea_info { /* encoding of response for level 15 */
> @@ -1548,7 +1548,7 @@ struct smb2_file_full_ea_info { /* encoding of response for level 15 */
>         __u8   flags;
>         __u8   ea_name_length;
>         __le16 ea_value_length;
> -       char   ea_data[0]; /* \0 terminated name plus value */
> +       char   ea_data[]; /* \0 terminated name plus value */
>  } __packed; /* level 15 Set */
>
>  /*
> --
> 2.25.0
>

Patch
diff mbox series

diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index ad14b8505b4d..817bc0531536 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -182,7 +182,7 @@  struct smb2_symlink_err_rsp {
 	__le16 PrintNameOffset;
 	__le16 PrintNameLength;
 	__le32 Flags;
-	__u8  PathBuffer[0];
+	__u8  PathBuffer[];
 } __packed;
 
 /* SMB 3.1.1 and later dialects. See MS-SMB2 section 2.2.2.1 */
@@ -210,7 +210,7 @@  struct share_redirect_error_context_rsp {
 	__le16 Flags;
 	__le16 TargetType;
 	__le32 IPAddrCount;
-	struct move_dst_ipaddr IpAddrMoveList[0];
+	struct move_dst_ipaddr IpAddrMoveList[];
 	/* __u8 ResourceName[] */ /* Name of share as counted Unicode string */
 } __packed;
 
@@ -326,7 +326,7 @@  struct smb2_netname_neg_context {
 	__le16	ContextType; /* 0x100 */
 	__le16	DataLength;
 	__le32	Reserved;
-	__le16	NetName[0]; /* hostname of target converted to UCS-2 */
+	__le16	NetName[]; /* hostname of target converted to UCS-2 */
 } __packed;
 
 #define POSIX_CTXT_DATA_LEN	16
@@ -421,13 +421,13 @@  struct tree_connect_contexts {
 	__le16 ContextType;
 	__le16 DataLength;
 	__le32 Reserved;
-	__u8   Data[0];
+	__u8   Data[];
 } __packed;
 
 /* Remoted identity tree connect context structures - see MS-SMB2 2.2.9.2.1 */
 struct smb3_blob_data {
 	__le16 BlobSize;
-	__u8   BlobData[0];
+	__u8   BlobData[];
 } __packed;
 
 /* Valid values for Attr */
@@ -477,14 +477,14 @@  struct remoted_identity_tcon_context {
 	__le16 DeviceGroups; /* offset to SID_ARRAY_DATA struct */
 	__le16 UserClaims; /* offset to BLOB_DATA struct */
 	__le16 DeviceClaims; /* offset to BLOB_DATA struct */
-	__u8   TicketInfo[0]; /* variable length buf - remoted identity data */
+	__u8   TicketInfo[]; /* variable length buf - remoted identity data */
 } __packed;
 
 struct smb2_tree_connect_req_extension {
 	__le32 TreeConnectContextOffset;
 	__le16 TreeConnectContextCount;
 	__u8  Reserved[10];
-	__u8  PathName[0]; /* variable sized array */
+	__u8  PathName[]; /* variable sized array */
 	/* followed by array of TreeConnectContexts */
 } __packed;
 
@@ -689,7 +689,7 @@  struct smb2_create_req {
 	__le16 NameLength;
 	__le32 CreateContextsOffset;
 	__le32 CreateContextsLength;
-	__u8   Buffer[0];
+	__u8   Buffer[];
 } __packed;
 
 /*
@@ -727,7 +727,7 @@  struct create_context {
 	__le16 Reserved;
 	__le16 DataOffset;
 	__le32 DataLength;
-	__u8 Buffer[0];
+	__u8 Buffer[];
 } __packed;
 
 #define SMB2_LEASE_READ_CACHING_HE	0x01
@@ -869,7 +869,7 @@  struct crt_sd_ctxt {
 struct resume_key_req {
 	char ResumeKey[COPY_CHUNK_RES_KEY_SIZE];
 	__le32	ContextLength;	/* MBZ */
-	char	Context[0];	/* ignored, Windows sets to 4 bytes of zero */
+	char	Context[];	/* ignored, Windows sets to 4 bytes of zero */
 } __packed;
 
 /* this goes in the ioctl buffer when doing a copychunk request */
@@ -931,7 +931,7 @@  struct reparse_data_buffer {
 	__le32	ReparseTag;
 	__le16	ReparseDataLength;
 	__u16	Reserved;
-	__u8	DataBuffer[0]; /* Variable Length */
+	__u8	DataBuffer[]; /* Variable Length */
 } __packed;
 
 struct reparse_guid_data_buffer {
@@ -939,7 +939,7 @@  struct reparse_guid_data_buffer {
 	__le16	ReparseDataLength;
 	__u16	Reserved;
 	__u8	ReparseGuid[16];
-	__u8	DataBuffer[0]; /* Variable Length */
+	__u8	DataBuffer[]; /* Variable Length */
 } __packed;
 
 struct reparse_mount_point_data_buffer {
@@ -950,7 +950,7 @@  struct reparse_mount_point_data_buffer {
 	__le16	SubstituteNameLength;
 	__le16	PrintNameOffset;
 	__le16	PrintNameLength;
-	__u8	PathBuffer[0]; /* Variable Length */
+	__u8	PathBuffer[]; /* Variable Length */
 } __packed;
 
 #define SYMLINK_FLAG_RELATIVE 0x00000001
@@ -964,7 +964,7 @@  struct reparse_symlink_data_buffer {
 	__le16	PrintNameOffset;
 	__le16	PrintNameLength;
 	__le32	Flags;
-	__u8	PathBuffer[0]; /* Variable Length */
+	__u8	PathBuffer[]; /* Variable Length */
 } __packed;
 
 /* See MS-FSCC 2.1.2.6 and cifspdu.h for struct reparse_posix_data */
@@ -1066,7 +1066,7 @@  struct smb2_ioctl_req {
 	__le32 MaxOutputResponse;
 	__le32 Flags;
 	__u32  Reserved2;
-	__u8   Buffer[0];
+	__u8   Buffer[];
 } __packed;
 
 struct smb2_ioctl_rsp {
@@ -1469,7 +1469,7 @@  struct smb3_fs_vol_info {
 	__le32	VolumeLabelLength; /* includes trailing null */
 	__u8	SupportsObjects; /* True if eg like NTFS, supports objects */
 	__u8	Reserved;
-	__u8	VolumeLabel[0]; /* variable len */
+	__u8	VolumeLabel[]; /* variable len */
 } __packed;
 
 /* partial list of QUERY INFO levels */
@@ -1531,7 +1531,7 @@  struct smb2_file_rename_info { /* encoding of request for level 10 */
 	__u8   Reserved[7];
 	__u64  RootDirectory;  /* MBZ for network operations (why says spec?) */
 	__le32 FileNameLength;
-	char   FileName[0];     /* New name to be assigned */
+	char   FileName[];     /* New name to be assigned */
 } __packed; /* level 10 Set */
 
 struct smb2_file_link_info { /* encoding of request for level 11 */
@@ -1540,7 +1540,7 @@  struct smb2_file_link_info { /* encoding of request for level 11 */
 	__u8   Reserved[7];
 	__u64  RootDirectory;  /* MBZ for network operations (why says spec?) */
 	__le32 FileNameLength;
-	char   FileName[0];     /* Name to be assigned to new link */
+	char   FileName[];     /* Name to be assigned to new link */
 } __packed; /* level 11 Set */
 
 struct smb2_file_full_ea_info { /* encoding of response for level 15 */
@@ -1548,7 +1548,7 @@  struct smb2_file_full_ea_info { /* encoding of response for level 15 */
 	__u8   flags;
 	__u8   ea_name_length;
 	__le16 ea_value_length;
-	char   ea_data[0]; /* \0 terminated name plus value */
+	char   ea_data[]; /* \0 terminated name plus value */
 } __packed; /* level 15 Set */
 
 /*