* [PATCH][next] cifs: cifspdu.h: Replace zero-length array with flexible-array member
@ 2020-03-06 22:17 Gustavo A. R. Silva
2020-03-07 20:04 ` Steve French
0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-06 22:17 UTC (permalink / raw)
To: Steve French
Cc: linux-cifs, samba-technical, linux-kernel, Gustavo A. R. Silva
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/cifspdu.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index 8e15887d1f1f..593d826820c3 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -1021,7 +1021,7 @@ typedef struct smb_com_writex_req {
__le16 ByteCount;
__u8 Pad; /* BB check for whether padded to DWORD
boundary and optimum performance here */
- char Data[0];
+ char Data[];
} __attribute__((packed)) WRITEX_REQ;
typedef struct smb_com_write_req {
@@ -1041,7 +1041,7 @@ typedef struct smb_com_write_req {
__le16 ByteCount;
__u8 Pad; /* BB check for whether padded to DWORD
boundary and optimum performance here */
- char Data[0];
+ char Data[];
} __attribute__((packed)) WRITE_REQ;
typedef struct smb_com_write_rsp {
@@ -1306,7 +1306,7 @@ typedef struct smb_com_ntransact_req {
/* SetupCount words follow then */
__le16 ByteCount;
__u8 Pad[3];
- __u8 Parms[0];
+ __u8 Parms[];
} __attribute__((packed)) NTRANSACT_REQ;
typedef struct smb_com_ntransact_rsp {
@@ -1523,7 +1523,7 @@ struct file_notify_information {
__le32 NextEntryOffset;
__le32 Action;
__le32 FileNameLength;
- __u8 FileName[0];
+ __u8 FileName[];
} __attribute__((packed));
/* For IO_REPARSE_TAG_SYMLINK */
@@ -1536,7 +1536,7 @@ struct reparse_symlink_data {
__le16 PrintNameOffset;
__le16 PrintNameLength;
__le32 Flags;
- char PathBuffer[0];
+ char PathBuffer[];
} __attribute__((packed));
/* Flag above */
@@ -1553,7 +1553,7 @@ struct reparse_posix_data {
__le16 ReparseDataLength;
__u16 Reserved;
__le64 InodeType; /* LNK, FIFO, CHR etc. */
- char PathBuffer[0];
+ char PathBuffer[];
} __attribute__((packed));
struct cifs_quota_data {
@@ -1762,7 +1762,7 @@ struct set_file_rename {
__le32 overwrite; /* 1 = overwrite dest */
__u32 root_fid; /* zero */
__le32 target_name_len;
- char target_name[0]; /* Must be unicode */
+ char target_name[]; /* Must be unicode */
} __attribute__((packed));
struct smb_com_transaction2_sfi_req {
@@ -2451,7 +2451,7 @@ struct cifs_posix_acl { /* access conrol list (ACL) */
__le16 version;
__le16 access_entry_count; /* access ACL - count of entries */
__le16 default_entry_count; /* default ACL - count of entries */
- struct cifs_posix_ace ace_array[0];
+ struct cifs_posix_ace ace_array[];
/* followed by
struct cifs_posix_ace default_ace_arraay[] */
} __attribute__((packed)); /* level 0x204 */
@@ -2757,7 +2757,7 @@ typedef struct file_xattr_info {
/* BB do we need another field for flags? BB */
__u32 xattr_name_len;
__u32 xattr_value_len;
- char xattr_name[0];
+ char xattr_name[];
/* followed by xattr_value[xattr_value_len], no pad */
} __attribute__((packed)) FILE_XATTR_INFO; /* extended attribute info
level 0x205 */
--
2.25.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH][next] cifs: cifspdu.h: Replace zero-length array with flexible-array member
2020-03-06 22:17 [PATCH][next] cifs: cifspdu.h: Replace zero-length array with flexible-array member Gustavo A. R. Silva
@ 2020-03-07 20:04 ` Steve French
2020-03-07 20:14 ` Gustavo A. R. Silva
0 siblings, 1 reply; 3+ messages in thread
From: Steve French @ 2020-03-07 20:04 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: CIFS, samba-technical, LKML
merged into cifs-2.6.git for-next
On Fri, Mar 6, 2020 at 5:01 PM 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/cifspdu.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
> index 8e15887d1f1f..593d826820c3 100644
> --- a/fs/cifs/cifspdu.h
> +++ b/fs/cifs/cifspdu.h
> @@ -1021,7 +1021,7 @@ typedef struct smb_com_writex_req {
> __le16 ByteCount;
> __u8 Pad; /* BB check for whether padded to DWORD
> boundary and optimum performance here */
> - char Data[0];
> + char Data[];
> } __attribute__((packed)) WRITEX_REQ;
>
> typedef struct smb_com_write_req {
> @@ -1041,7 +1041,7 @@ typedef struct smb_com_write_req {
> __le16 ByteCount;
> __u8 Pad; /* BB check for whether padded to DWORD
> boundary and optimum performance here */
> - char Data[0];
> + char Data[];
> } __attribute__((packed)) WRITE_REQ;
>
> typedef struct smb_com_write_rsp {
> @@ -1306,7 +1306,7 @@ typedef struct smb_com_ntransact_req {
> /* SetupCount words follow then */
> __le16 ByteCount;
> __u8 Pad[3];
> - __u8 Parms[0];
> + __u8 Parms[];
> } __attribute__((packed)) NTRANSACT_REQ;
>
> typedef struct smb_com_ntransact_rsp {
> @@ -1523,7 +1523,7 @@ struct file_notify_information {
> __le32 NextEntryOffset;
> __le32 Action;
> __le32 FileNameLength;
> - __u8 FileName[0];
> + __u8 FileName[];
> } __attribute__((packed));
>
> /* For IO_REPARSE_TAG_SYMLINK */
> @@ -1536,7 +1536,7 @@ struct reparse_symlink_data {
> __le16 PrintNameOffset;
> __le16 PrintNameLength;
> __le32 Flags;
> - char PathBuffer[0];
> + char PathBuffer[];
> } __attribute__((packed));
>
> /* Flag above */
> @@ -1553,7 +1553,7 @@ struct reparse_posix_data {
> __le16 ReparseDataLength;
> __u16 Reserved;
> __le64 InodeType; /* LNK, FIFO, CHR etc. */
> - char PathBuffer[0];
> + char PathBuffer[];
> } __attribute__((packed));
>
> struct cifs_quota_data {
> @@ -1762,7 +1762,7 @@ struct set_file_rename {
> __le32 overwrite; /* 1 = overwrite dest */
> __u32 root_fid; /* zero */
> __le32 target_name_len;
> - char target_name[0]; /* Must be unicode */
> + char target_name[]; /* Must be unicode */
> } __attribute__((packed));
>
> struct smb_com_transaction2_sfi_req {
> @@ -2451,7 +2451,7 @@ struct cifs_posix_acl { /* access conrol list (ACL) */
> __le16 version;
> __le16 access_entry_count; /* access ACL - count of entries */
> __le16 default_entry_count; /* default ACL - count of entries */
> - struct cifs_posix_ace ace_array[0];
> + struct cifs_posix_ace ace_array[];
> /* followed by
> struct cifs_posix_ace default_ace_arraay[] */
> } __attribute__((packed)); /* level 0x204 */
> @@ -2757,7 +2757,7 @@ typedef struct file_xattr_info {
> /* BB do we need another field for flags? BB */
> __u32 xattr_name_len;
> __u32 xattr_value_len;
> - char xattr_name[0];
> + char xattr_name[];
> /* followed by xattr_value[xattr_value_len], no pad */
> } __attribute__((packed)) FILE_XATTR_INFO; /* extended attribute info
> level 0x205 */
> --
> 2.25.0
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][next] cifs: cifspdu.h: Replace zero-length array with flexible-array member
2020-03-07 20:04 ` Steve French
@ 2020-03-07 20:14 ` Gustavo A. R. Silva
0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2020-03-07 20:14 UTC (permalink / raw)
To: Steve French; +Cc: CIFS, samba-technical, LKML
On 3/7/20 14:04, Steve French wrote:
> merged into cifs-2.6.git for-next
>
Thanks, Steve.
--
Gustavo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-07 20:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-06 22:17 [PATCH][next] cifs: cifspdu.h: Replace zero-length array with flexible-array member Gustavo A. R. Silva
2020-03-07 20:04 ` Steve French
2020-03-07 20:14 ` 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.