All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] efi: Replace zero-length array with flexible-array
@ 2020-05-07 19:27 Gustavo A. R. Silva
  2020-05-08 17:26 ` Linus Torvalds
  0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2020-05-07 19:27 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel

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 {


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

* Re: [PATCH] efi: Replace zero-length array with flexible-array
  2020-05-07 19:27 [PATCH] efi: Replace zero-length array with flexible-array Gustavo A. R. Silva
@ 2020-05-08 17:26 ` Linus Torvalds
  2020-05-08 23:41   ` Gustavo A. R. Silva
  0 siblings, 1 reply; 3+ messages in thread
From: Linus Torvalds @ 2020-05-08 17:26 UTC (permalink / raw)
  To: Gustavo A. R. Silva; +Cc: Linux Kernel Mailing List

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

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

* Re: [PATCH] efi: Replace zero-length array with flexible-array
  2020-05-08 17:26 ` Linus Torvalds
@ 2020-05-08 23:41   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2020-05-08 23:41 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Kees Cook

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

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

end of thread, other threads:[~2020-05-08 23:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-07 19:27 [PATCH] efi: Replace zero-length array with flexible-array Gustavo A. R. Silva
2020-05-08 17:26 ` Linus Torvalds
2020-05-08 23:41   ` 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.