[PATCH] efi: Replace zero-length array with flexible-array

[PATCH] efi: Replace zero-length array with flexible-array

[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]

sizeof(flexible-array-member) triggers a warning because flexible array
members have incomplete type[1]. There are some instances of code in
which the sizeof operator is being incorrectly/erroneously applied to
zero-length arrays and the result is zero. Such instances may be hiding
some bugs. So, this work (flexible-array member conversions) will also
help to get completely rid of those sorts of issues.

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 <gustavoars@kernel.org>
---
 include/linux/dma/ti-cppi5.h                   |    4 ++--
 include/linux/efi.h                            |    2 +-
 include/linux/mailbox/zynqmp-ipi-message.h     |    2 +-
 include/linux/platform_data/cros_ec_commands.h |    4 ++--
 include/linux/platform_data/cros_ec_proto.h    |    2 +-
 kernel/params.c                                |    2 +-
 kernel/tracepoint.c                            |    2 +-
 scripts/kallsyms.c                             |    2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/linux/dma/ti-cppi5.h b/include/linux/dma/ti-cppi5.h
index 579356ae447e..78c911269107 100644
--- a/include/linux/dma/ti-cppi5.h
+++ b/include/linux/dma/ti-cppi5.h
@@ -47,7 +47,7 @@ struct cppi5_host_desc_t {
 	u32 buf_info1;
 	u32 org_buf_len;
 	u64 org_buf_ptr;
-	u32 epib[0];
+	u32 epib[];
 } __packed;
 
 #define CPPI5_DESC_MIN_ALIGN			(16U)
@@ -139,7 +139,7 @@ struct cppi5_desc_epib_t {
  */
 struct cppi5_monolithic_desc_t {
 	struct cppi5_desc_hdr_t hdr;
-	u32 epib[0];
+	u32 epib[];
 };
 
 #define CPPI5_INFO2_MDESC_DATA_OFFSET_SHIFT	(18U)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 251f1f783cdf..a657850899f0 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1234,7 +1234,7 @@ struct linux_efi_memreserve {
 	struct {
 		phys_addr_t	base;
 		phys_addr_t	size;
-	} entry[0];
+	} entry[];
 };
 
 #define EFI_MEMRESERVE_SIZE(count) (sizeof(struct linux_efi_memreserve) + \
diff --git a/include/linux/mailbox/zynqmp-ipi-message.h b/include/linux/mailbox/zynqmp-ipi-message.h
index 9542b41eacfd..35ce84c8ca02 100644
--- a/include/linux/mailbox/zynqmp-ipi-message.h
+++ b/include/linux/mailbox/zynqmp-ipi-message.h
@@ -14,7 +14,7 @@
  */
 struct zynqmp_ipi_message {
 	size_t len;
-	u8 data[0];
+	u8 data[];
 };
 
 #endif /* _LINUX_ZYNQMP_IPI_MESSAGE_H_ */
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 69210881ebac..451885c697cc 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -5065,7 +5065,7 @@ struct ec_response_pd_log {
 	uint8_t type;       /* event type : see PD_EVENT_xx below */
 	uint8_t size_port;  /* [7:5] port number [4:0] payload size in bytes */
 	uint16_t data;      /* type-defined data payload */
-	uint8_t payload[0]; /* optional additional data payload: 0..16 bytes */
+	uint8_t payload[]; /* optional additional data payload: 0..16 bytes */
 } __ec_align4;
 
 /* The timestamp is the microsecond counter shifted to get about a ms. */
@@ -5691,7 +5691,7 @@ struct ec_response_fp_encryption_status {
 
 struct ec_response_tp_frame_info {
 	uint32_t n_frames;
-	uint32_t frame_sizes[0];
+	uint32_t frame_sizes[];
 } __ec_align4;
 
 /* Create a snapshot of current frame readings */
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 383243326676..7f03e02c48cd 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -69,7 +69,7 @@ struct cros_ec_command {
 	uint32_t outsize;
 	uint32_t insize;
 	uint32_t result;
-	uint8_t data[0];
+	uint8_t data[];
 };
 
 /**
diff --git a/kernel/params.c b/kernel/params.c
index 8e56f8b12d8f..55c07e3b9903 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -529,7 +529,7 @@ struct module_param_attrs
 {
 	unsigned int num;
 	struct attribute_group grp;
-	struct param_attribute attrs[0];
+	struct param_attribute attrs[];
 };
 
 #ifdef CONFIG_SYSFS
diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 73956eaff8a9..3ee46cda2692 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -50,7 +50,7 @@ static bool ok_to_free_tracepoints;
  */
 struct tp_probes {
 	struct rcu_head rcu;
-	struct tracepoint_func probes[0];
+	struct tracepoint_func probes[];
 };
 
 static inline void *allocate_probes(int count)
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 3e8dea6e0a95..6dc3078649fa 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -34,7 +34,7 @@ struct sym_entry {
 	unsigned int len;
 	unsigned int start_pos;
 	unsigned int percpu_absolute;
-	unsigned char sym[0];
+	unsigned char sym[];
 };
 
 struct addr_range {

Re: [PATCH] efi: Replace zero-length array with flexible-array

[Linus Torvalds]

On Thu, May 7, 2020 at 12:22 PM Gustavo A. R. Silva
<gustavoars@kernel.org> 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 [...]

Why is this called an "efi" patch, when it doesn't appear to be so at all:

>  include/linux/dma/ti-cppi5.h                   |    4 ++--
>  include/linux/efi.h                            |    2 +-
>  include/linux/mailbox/zynqmp-ipi-message.h     |    2 +-
>  include/linux/platform_data/cros_ec_commands.h |    4 ++--
>  include/linux/platform_data/cros_ec_proto.h    |    2 +-
>  kernel/params.c                                |    2 +-
>  kernel/tracepoint.c                            |    2 +-
>  scripts/kallsyms.c                             |    2 +-
>  8 files changed, 10 insertions(+), 10 deletions(-)

Yes, one line of the patch is EFI code. The rest are not.

                Linus

Re: [PATCH] efi: Replace zero-length array with flexible-array

[Gustavo A. R. Silva]

On Fri, May 08, 2020 at 10:26:40AM -0700, Linus Torvalds wrote:
> On Thu, May 7, 2020 at 12:22 PM Gustavo A. R. Silva
> <gustavoars@kernel.org> 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 [...]
> 
> Why is this called an "efi" patch, when it doesn't appear to be so at all:
> 
> >  include/linux/dma/ti-cppi5.h                   |    4 ++--
> >  include/linux/efi.h                            |    2 +-
> >  include/linux/mailbox/zynqmp-ipi-message.h     |    2 +-
> >  include/linux/platform_data/cros_ec_commands.h |    4 ++--
> >  include/linux/platform_data/cros_ec_proto.h    |    2 +-
> >  kernel/params.c                                |    2 +-
> >  kernel/tracepoint.c                            |    2 +-
> >  scripts/kallsyms.c                             |    2 +-
> >  8 files changed, 10 insertions(+), 10 deletions(-)
> 
> Yes, one line of the patch is EFI code. The rest are not.
> 

Yeah. It seems the script needs some improvement. I'll split this
patch up into multiple patches.

Thanks
--
Gustavo