Skip to contentSkip to conversation

ath10k: Replace zero-length array with flexible-array

PATCH Superseded Gustavo A. R. Silva 2 participants 4 messages May 4, 2020 – May 7, 2020

Subsystems (3)
Networking Network drivers Qualcomm Atheros ATH10K wireless driver

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 ath10k: Replace zero-length array with flexible-array

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 <[email protected]>
---
 drivers/net/wireless/ath/ath10k/ce.h       |  2 +-
 drivers/net/wireless/ath/ath10k/core.h     |  2 +-
 drivers/net/wireless/ath/ath10k/coredump.h |  4 +--
 drivers/net/wireless/ath/ath10k/debug.h    |  2 +-
 drivers/net/wireless/ath/ath10k/htt.h      | 42 +++++++++++-----------
 drivers/net/wireless/ath/ath10k/hw.h       |  2 +-
 drivers/net/wireless/ath/ath10k/pci.h      |  2 +-
 drivers/net/wireless/ath/ath10k/wmi-tlv.h  |  6 ++--
 drivers/net/wireless/ath/ath10k/wmi.h      | 42 +++++++++++-----------
 9 files changed, 52 insertions(+), 52 deletions(-)
drivers/net/wireless/ath/ath10k/ce.h +1 -1
index a7478c240f78..41b5ea25ca57 100644
@@ -110,7 +110,7 @@ struct ath10k_ce_ring {
 	struct ce_desc_64 *shadow_base;
 
 	/* keep last */
-	void *per_transfer_context[0];
+	void *per_transfer_context[];
 };
 
 struct ath10k_ce_pipe {
drivers/net/wireless/ath/ath10k/core.h +1 -1
index bd8ef576c590..829c2f9244e8 100644
@@ -1228,7 +1228,7 @@ struct ath10k {
 	int coex_gpio_pin;
 
 	/* must be last */
-	u8 drv_priv[0] __aligned(sizeof(void *));
+	u8 drv_priv[] __aligned(sizeof(void *));
 };
 
 static inline bool ath10k_peer_stats_enabled(struct ath10k *ar)
drivers/net/wireless/ath/ath10k/coredump.h +2 -2
index 8bf03e8c1d3a..e760ce1a5f1e 100644
@@ -88,7 +88,7 @@ struct ath10k_dump_file_data {
 	u8 unused[128];
 
 	/* struct ath10k_tlv_dump_data + more */
-	u8 data[0];
+	u8 data[];
 } __packed;
 
 struct ath10k_dump_ram_data_hdr {

@@ -100,7 +100,7 @@ struct ath10k_dump_ram_data_hdr {
 	/* length of payload data, not including this header */
 	__le32 length;
 
-	u8 data[0];
+	u8 data[];
 };
 
 /* magic number to fill the holes not copied due to sections in regions */
drivers/net/wireless/ath/ath10k/debug.h +1 -1
index 82f7eb8583d9..099cc438e231 100644
@@ -65,7 +65,7 @@ struct ath10k_pktlog_hdr {
 	__le16 log_type; /* Type of log information foll this header */
 	__le16 size; /* Size of variable length log information in bytes */
 	__le32 timestamp;
-	u8 payload[0];
+	u8 payload[];
 } __packed;
 
 /* FIXME: How to calculate the buffer size sanely? */
drivers/net/wireless/ath/ath10k/htt.h +21 -21

165 lines not on this page · raw message

drivers/net/wireless/ath/ath10k/hw.h +1 -1
index 970c736ac6bb..077813bc6c7e 100644
@@ -165,7 +165,7 @@ enum qca9377_chip_id_rev {
 struct ath10k_fw_ie {
 	__le32 id;
 	__le32 len;
-	u8 data[0];
+	u8 data[];
 };
 
 enum ath10k_fw_ie_type {
drivers/net/wireless/ath/ath10k/pci.h +1 -1
index 4455ed6c5275..1254412f06dd 100644
@@ -182,7 +182,7 @@ struct ath10k_pci {
 	 * allocated (ahb support enabled case) in the continuation of
 	 * this struct.
 	 */
-	struct ath10k_ahb ahb[0];
+	struct ath10k_ahb ahb[];
 };
 
 static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
drivers/net/wireless/ath/ath10k/wmi-tlv.h +3 -3
index 4972dc12991c..4d49eb4b7f3d 100644
@@ -1623,7 +1623,7 @@ wmi_tlv_svc_map_ext(const __le32 *in, unsigned long *out, size_t len)
 struct wmi_tlv {
 	__le16 len;
 	__le16 tag;
-	u8 value[0];
+	u8 value[];
 } __packed;
 
 struct ath10k_mgmt_tx_pkt_addr {

@@ -2023,7 +2023,7 @@ struct wmi_tlv_bcn_tx_status_ev {
 struct wmi_tlv_bcn_prb_info {
 	__le32 caps;
 	__le32 erp;
-	u8 ies[0];
+	u8 ies[];
 } __packed;
 
 struct wmi_tlv_bcn_tmpl_cmd {

@@ -2054,7 +2054,7 @@ struct wmi_tlv_diag_item {
 	__le16 len;
 	__le32 timestamp;
 	__le32 code;
-	u8 payload[0];
+	u8 payload[];
 } __packed;
 
 struct wmi_tlv_diag_data_ev {
drivers/net/wireless/ath/ath10k/wmi.h +21 -21

164 lines not on this page · raw message

Signature · 7 lines
2.26.2


_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
#1 lore raw
Quoted 29 lines · 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.

Quoted 7 lines

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

Fails to apply, please rebase on top of ath.git master branch.

error: patch failed: drivers/net/wireless/ath/ath10k/pci.h:182
error: drivers/net/wireless/ath/ath10k/pci.h: patch does not apply stg import: Diff does not apply cleanly

Patch set to Changes Requested.

Signature · 8 lines
https://patchwork.kernel.org/patch/11527635/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k
#2 lore raw
In reply to Gustavo A. R. Silva
Quoted 29 lines · 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.

Quoted 7 lines

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

Fails to apply, please rebase on top of ath.git master branch.

error: patch failed: drivers/net/wireless/ath/ath10k/pci.h:182
error: drivers/net/wireless/ath/ath10k/pci.h: patch does not apply stg import: Diff does not apply cleanly

Patch set to Changes Requested.

Signature · 3 lines
https://patchwork.kernel.org/patch/11527635/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
author
#3 lore raw

Kalle,

Quoted 9 lines · Kalle Valo

Fails to apply, please rebase on top of ath.git master branch.

error: patch failed: drivers/net/wireless/ath/ath10k/pci.h:182
error: drivers/net/wireless/ath/ath10k/pci.h: patch does not apply stg import: Diff does not apply cleanly

Patch set to Changes Requested.

Sorry for the late reply. I've had some troubles with my email settings and LKML, recently.

I've just sent v2:

https://lore.kernel.org/lkml/20200507041127.GA31587@embeddedor/

Thanks
--
Gustavo