* [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
[not found] <20210826050458.1540622-1-keescook@chromium.org>
@ 2021-08-26 5:04 ` Kees Cook
2021-08-26 6:24 ` Marc Kleine-Budde
2021-08-26 7:36 ` Vincent MAILHOL
0 siblings, 2 replies; 8+ messages in thread
From: Kees Cook @ 2021-08-26 5:04 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Gustavo A. R. Silva, Arnd Bergmann, Ayush Sawal,
Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu, David S. Miller,
Kalle Valo, Jakub Kicinski, Stanislaw Gruszka, Luca Coelho,
James E.J. Bottomley, Martin K. Petersen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Johannes Berg,
Mordechay Goodstein, Lee Jones, Wolfgang Grandegger,
Marc Kleine-Budde, Arunachalam Santhanam, Vincent Mailhol,
Mikulas Patocka, linux-crypto, ath10k, linux-wireless, netdev,
linux-scsi, linux-can, bpf, Rasmus Villemoes, Keith Packard,
Dan Williams, Daniel Vetter, clang-built-linux, linux-hardening
In support of enabling -Warray-bounds and -Wzero-length-bounds and
correctly handling run-time memcpy() bounds checking, replace all
open-coded flexible arrays (i.e. 0-element arrays) in unions with the
flex_array() helper macro.
This fixes warnings such as:
fs/hpfs/anode.c: In function 'hpfs_add_sector_to_btree':
fs/hpfs/anode.c:209:27: warning: array subscript 0 is outside the bounds of an interior zero-length array 'struct bplus_internal_node[0]' [-Wzero-length-bounds]
209 | anode->btree.u.internal[0].down = cpu_to_le32(a);
| ~~~~~~~~~~~~~~~~~~~~~~~^~~
In file included from fs/hpfs/hpfs_fn.h:26,
from fs/hpfs/anode.c:10:
fs/hpfs/hpfs.h:412:32: note: while referencing 'internal'
412 | struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
| ^~~~~~~~
drivers/net/can/usb/etas_es58x/es58x_fd.c: In function 'es58x_fd_tx_can_msg':
drivers/net/can/usb/etas_es58x/es58x_fd.c:360:35: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned char[]'} [-Wzero-length-bounds]
360 | tx_can_msg = (typeof(tx_can_msg))&es58x_fd_urb_cmd->raw_msg[msg_len];
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/net/can/usb/etas_es58x/es58x_core.h:22,
from drivers/net/can/usb/etas_es58x/es58x_fd.c:17:
drivers/net/can/usb/etas_es58x/es58x_fd.h:231:6: note: while referencing 'raw_msg'
231 | u8 raw_msg[0];
| ^~~~~~~
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ayush Sawal <ayush.sawal@chelsio.com>
Cc: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
Cc: Rohit Maheshwari <rohitm@chelsio.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Stanislaw Gruszka <stf_xl@wp.pl>
Cc: Luca Coelho <luciano.coelho@intel.com>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Johannes Berg <johannes.berg@intel.com>
Cc: Mordechay Goodstein <mordechay.goodstein@intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Arunachalam Santhanam <arunachalam.santhanam@in.bosch.com>
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
Cc: linux-crypto@vger.kernel.org
Cc: ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-can@vger.kernel.org
Cc: bpf@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/crypto/chelsio/chcr_crypto.h | 14 +++++++++-----
drivers/net/can/usb/etas_es58x/es581_4.h | 2 +-
drivers/net/can/usb/etas_es58x/es58x_fd.h | 2 +-
drivers/net/wireless/ath/ath10k/htt.h | 7 +++++--
drivers/net/wireless/intel/iwlegacy/commands.h | 6 ++++--
drivers/net/wireless/intel/iwlwifi/dvm/commands.h | 6 ++++--
drivers/net/wireless/intel/iwlwifi/fw/api/tx.h | 6 ++++--
drivers/scsi/aic94xx/aic94xx_sds.c | 6 ++++--
fs/hpfs/hpfs.h | 8 ++++----
include/linux/filter.h | 6 ++++--
include/scsi/sas.h | 12 ++++++++----
include/uapi/rdma/rdma_user_rxe.h | 4 ++--
include/uapi/sound/asoc.h | 4 ++--
13 files changed, 52 insertions(+), 31 deletions(-)
diff --git a/drivers/crypto/chelsio/chcr_crypto.h b/drivers/crypto/chelsio/chcr_crypto.h
index e89f9e0094b4..c7816c83e324 100644
--- a/drivers/crypto/chelsio/chcr_crypto.h
+++ b/drivers/crypto/chelsio/chcr_crypto.h
@@ -222,8 +222,10 @@ struct chcr_authenc_ctx {
};
struct __aead_ctx {
- struct chcr_gcm_ctx gcm[0];
- struct chcr_authenc_ctx authenc[];
+ union {
+ DECLARE_FLEX_ARRAY(struct chcr_gcm_ctx, gcm);
+ DECLARE_FLEX_ARRAY(struct chcr_authenc_ctx, authenc);
+ };
};
struct chcr_aead_ctx {
@@ -245,9 +247,11 @@ struct hmac_ctx {
};
struct __crypto_ctx {
- struct hmac_ctx hmacctx[0];
- struct ablk_ctx ablkctx[0];
- struct chcr_aead_ctx aeadctx[];
+ union {
+ DECLARE_FLEX_ARRAY(struct hmac_ctx, hmacctx);
+ DECLARE_FLEX_ARRAY(struct ablk_ctx, ablkctx);
+ DECLARE_FLEX_ARRAY(struct chcr_aead_ctx, aeadctx);
+ };
};
struct chcr_context {
diff --git a/drivers/net/can/usb/etas_es58x/es581_4.h b/drivers/net/can/usb/etas_es58x/es581_4.h
index 4bc60a6df697..667ecb77168c 100644
--- a/drivers/net/can/usb/etas_es58x/es581_4.h
+++ b/drivers/net/can/usb/etas_es58x/es581_4.h
@@ -192,7 +192,7 @@ struct es581_4_urb_cmd {
struct es581_4_rx_cmd_ret rx_cmd_ret;
__le64 timestamp;
u8 rx_cmd_ret_u8;
- u8 raw_msg[0];
+ DECLARE_FLEX_ARRAY(u8, raw_msg);
} __packed;
__le16 reserved_for_crc16_do_not_use;
diff --git a/drivers/net/can/usb/etas_es58x/es58x_fd.h b/drivers/net/can/usb/etas_es58x/es58x_fd.h
index ee18a87e40c0..e33003f96e5e 100644
--- a/drivers/net/can/usb/etas_es58x/es58x_fd.h
+++ b/drivers/net/can/usb/etas_es58x/es58x_fd.h
@@ -228,7 +228,7 @@ struct es58x_fd_urb_cmd {
struct es58x_fd_tx_ack_msg tx_ack_msg;
__le64 timestamp;
__le32 rx_cmd_ret_le32;
- u8 raw_msg[0];
+ DECLARE_FLEX_ARRAY(u8, raw_msg);
} __packed;
__le16 reserved_for_crc16_do_not_use;
diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
index ec689e3ce48a..a6de08d3bf4a 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1674,8 +1674,11 @@ struct htt_tx_fetch_ind {
__le32 token;
__le16 num_resp_ids;
__le16 num_records;
- __le32 resp_ids[0]; /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
- struct htt_tx_fetch_record records[];
+ union {
+ /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
+ DECLARE_FLEX_ARRAY(__le32, resp_ids);
+ DECLARE_FLEX_ARRAY(struct htt_tx_fetch_record, records);
+ };
} __packed;
static inline void *
diff --git a/drivers/net/wireless/intel/iwlegacy/commands.h b/drivers/net/wireless/intel/iwlegacy/commands.h
index 89c6671b32bc..4a97310f8fee 100644
--- a/drivers/net/wireless/intel/iwlegacy/commands.h
+++ b/drivers/net/wireless/intel/iwlegacy/commands.h
@@ -1408,8 +1408,10 @@ struct il3945_tx_cmd {
* MAC header goes here, followed by 2 bytes padding if MAC header
* length is 26 or 30 bytes, followed by payload data
*/
- u8 payload[0];
- struct ieee80211_hdr hdr[];
+ union {
+ DECLARE_FLEX_ARRAY(u8, payload);
+ DECLARE_FLEX_ARRAY(struct ieee80211_hdr, hdr);
+ };
} __packed;
/*
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h
index 235c7a2e3483..75a4b8e26232 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h
@@ -1251,8 +1251,10 @@ struct iwl_tx_cmd {
* MAC header goes here, followed by 2 bytes padding if MAC header
* length is 26 or 30 bytes, followed by payload data
*/
- u8 payload[0];
- struct ieee80211_hdr hdr[];
+ union {
+ DECLARE_FLEX_ARRAY(u8, payload);
+ DECLARE_FLEX_ARRAY(struct ieee80211_hdr, hdr);
+ };
} __packed;
/*
diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h b/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
index 24e4a82a55da..66c5487e857e 100644
--- a/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
@@ -713,8 +713,10 @@ struct iwl_mvm_compressed_ba_notif {
__le32 tx_rate;
__le16 tfd_cnt;
__le16 ra_tid_cnt;
- struct iwl_mvm_compressed_ba_ratid ra_tid[0];
- struct iwl_mvm_compressed_ba_tfd tfd[];
+ union {
+ DECLARE_FLEX_ARRAY(struct iwl_mvm_compressed_ba_ratid, ra_tid);
+ DECLARE_FLEX_ARRAY(struct iwl_mvm_compressed_ba_tfd, tfd);
+ };
} __packed; /* COMPRESSED_BA_RES_API_S_VER_4 */
/**
diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c b/drivers/scsi/aic94xx/aic94xx_sds.c
index 46815e65f7a4..5def83c88f13 100644
--- a/drivers/scsi/aic94xx/aic94xx_sds.c
+++ b/drivers/scsi/aic94xx/aic94xx_sds.c
@@ -517,8 +517,10 @@ struct asd_ms_conn_map {
u8 num_nodes;
u8 usage_model_id;
u32 _resvd;
- struct asd_ms_conn_desc conn_desc[0];
- struct asd_ms_node_desc node_desc[];
+ union {
+ DECLARE_FLEX_ARRAY(struct asd_ms_conn_desc, conn_desc);
+ DECLARE_FLEX_ARRAY(struct asd_ms_node_desc, node_desc);
+ };
} __attribute__ ((packed));
struct asd_ctrla_phy_entry {
diff --git a/fs/hpfs/hpfs.h b/fs/hpfs/hpfs.h
index d92c4af3e1b4..281dec8f636b 100644
--- a/fs/hpfs/hpfs.h
+++ b/fs/hpfs/hpfs.h
@@ -409,10 +409,10 @@ struct bplus_header
__le16 first_free; /* offset from start of header to
first free node in array */
union {
- struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
- subtree pointers */
- struct bplus_leaf_node external[0]; /* (external) 3-word entries giving
- sector runs */
+ /* (internal) 2-word entries giving subtree pointers */
+ DECLARE_FLEX_ARRAY(struct bplus_internal_node, internal);
+ /* (external) 3-word entries giving sector runs */
+ DECLARE_FLEX_ARRAY(struct bplus_leaf_node, external);
} u;
};
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 472f97074da0..5ca52bfa5868 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -572,8 +572,10 @@ struct bpf_prog {
struct bpf_prog_aux *aux; /* Auxiliary fields */
struct sock_fprog_kern *orig_prog; /* Original BPF program */
/* Instructions for interpreter */
- struct sock_filter insns[0];
- struct bpf_insn insnsi[];
+ union {
+ DECLARE_FLEX_ARRAY(struct sock_filter, insns);
+ DECLARE_FLEX_ARRAY(struct bpf_insn, insnsi);
+ };
};
struct sk_filter {
diff --git a/include/scsi/sas.h b/include/scsi/sas.h
index 4726c1bbec65..64154c1fed02 100644
--- a/include/scsi/sas.h
+++ b/include/scsi/sas.h
@@ -323,8 +323,10 @@ struct ssp_response_iu {
__be32 sense_data_len;
__be32 response_data_len;
- u8 resp_data[0];
- u8 sense_data[];
+ union {
+ DECLARE_FLEX_ARRAY(u8, resp_data);
+ DECLARE_FLEX_ARRAY(u8, sense_data);
+ };
} __attribute__ ((packed));
struct ssp_command_iu {
@@ -554,8 +556,10 @@ struct ssp_response_iu {
__be32 sense_data_len;
__be32 response_data_len;
- u8 resp_data[0];
- u8 sense_data[];
+ union {
+ DECLARE_FLEX_ARRAY(u8, resp_data);
+ DECLARE_FLEX_ARRAY(u8, sense_data);
+ };
} __attribute__ ((packed));
struct ssp_command_iu {
diff --git a/include/uapi/rdma/rdma_user_rxe.h b/include/uapi/rdma/rdma_user_rxe.h
index e283c2220aba..040752c99ec9 100644
--- a/include/uapi/rdma/rdma_user_rxe.h
+++ b/include/uapi/rdma/rdma_user_rxe.h
@@ -141,8 +141,8 @@ struct rxe_dma_info {
__u32 sge_offset;
__u32 reserved;
union {
- __u8 inline_data[0];
- struct rxe_sge sge[0];
+ __DECLARE_FLEX_ARRAY(u8, inline_data);
+ __DECLARE_FLEX_ARRAY(struct rxe_sge, sge);
};
};
diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h
index da61398b1f8f..053949287ce8 100644
--- a/include/uapi/sound/asoc.h
+++ b/include/uapi/sound/asoc.h
@@ -240,8 +240,8 @@ struct snd_soc_tplg_vendor_array {
struct snd_soc_tplg_private {
__le32 size; /* in bytes of private data */
union {
- char data[0];
- struct snd_soc_tplg_vendor_array array[0];
+ __DECLARE_FLEX_ARRAY(char, data);
+ __DECLARE_FLEX_ARRAY(struct snd_soc_tplg_vendor_array, array);
};
} __attribute__((packed));
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-26 5:04 ` [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions Kees Cook
@ 2021-08-26 6:24 ` Marc Kleine-Budde
2021-08-27 16:08 ` Kees Cook
2021-08-27 16:17 ` Kees Cook
2021-08-26 7:36 ` Vincent MAILHOL
1 sibling, 2 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2021-08-26 6:24 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Gustavo A. R. Silva, Arnd Bergmann, Ayush Sawal,
Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu, David S. Miller,
Kalle Valo, Jakub Kicinski, Stanislaw Gruszka, Luca Coelho,
James E.J. Bottomley, Martin K. Petersen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Johannes Berg,
Mordechay Goodstein, Lee Jones, Wolfgang Grandegger,
Arunachalam Santhanam, Vincent Mailhol, Mikulas Patocka,
linux-crypto, ath10k, linux-wireless, netdev, linux-scsi,
linux-can, bpf, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
[-- Attachment #1: Type: text/plain, Size: 3950 bytes --]
On 25.08.2021 22:04:55, Kees Cook wrote:
> In support of enabling -Warray-bounds and -Wzero-length-bounds and
> correctly handling run-time memcpy() bounds checking, replace all
> open-coded flexible arrays (i.e. 0-element arrays) in unions with the
> flex_array() helper macro.
>
> This fixes warnings such as:
>
> fs/hpfs/anode.c: In function 'hpfs_add_sector_to_btree':
> fs/hpfs/anode.c:209:27: warning: array subscript 0 is outside the bounds of an interior zero-length array 'struct bplus_internal_node[0]' [-Wzero-length-bounds]
> 209 | anode->btree.u.internal[0].down = cpu_to_le32(a);
> | ~~~~~~~~~~~~~~~~~~~~~~~^~~
> In file included from fs/hpfs/hpfs_fn.h:26,
> from fs/hpfs/anode.c:10:
> fs/hpfs/hpfs.h:412:32: note: while referencing 'internal'
> 412 | struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
> | ^~~~~~~~
>
> drivers/net/can/usb/etas_es58x/es58x_fd.c: In function 'es58x_fd_tx_can_msg':
> drivers/net/can/usb/etas_es58x/es58x_fd.c:360:35: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned char[]'} [-Wzero-length-bounds]
> 360 | tx_can_msg = (typeof(tx_can_msg))&es58x_fd_urb_cmd->raw_msg[msg_len];
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/net/can/usb/etas_es58x/es58x_core.h:22,
> from drivers/net/can/usb/etas_es58x/es58x_fd.c:17:
> drivers/net/can/usb/etas_es58x/es58x_fd.h:231:6: note: while referencing 'raw_msg'
> 231 | u8 raw_msg[0];
> | ^~~~~~~
>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Ayush Sawal <ayush.sawal@chelsio.com>
> Cc: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
> Cc: Rohit Maheshwari <rohitm@chelsio.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Stanislaw Gruszka <stf_xl@wp.pl>
> Cc: Luca Coelho <luciano.coelho@intel.com>
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Song Liu <songliubraving@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: KP Singh <kpsingh@kernel.org>
> Cc: Johannes Berg <johannes.berg@intel.com>
> Cc: Mordechay Goodstein <mordechay.goodstein@intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Wolfgang Grandegger <wg@grandegger.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Arunachalam Santhanam <arunachalam.santhanam@in.bosch.com>
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
> Cc: linux-crypto@vger.kernel.org
> Cc: ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-can@vger.kernel.org
> Cc: bpf@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/net/can/usb/etas_es58x/es581_4.h | 2 +-
> drivers/net/can/usb/etas_es58x/es58x_fd.h | 2 +-
For the can drivers:
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
BTW: Is there opportunity for conversion, too?
| drivers/net/can/peak_canfd/peak_pciefd_main.c:146:32: warning: array of flexible structures
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-26 5:04 ` [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions Kees Cook
2021-08-26 6:24 ` Marc Kleine-Budde
@ 2021-08-26 7:36 ` Vincent MAILHOL
2021-08-26 15:39 ` Kees Cook
1 sibling, 1 reply; 8+ messages in thread
From: Vincent MAILHOL @ 2021-08-26 7:36 UTC (permalink / raw)
To: Kees Cook
Cc: open list, Gustavo A. R. Silva, Arnd Bergmann, Ayush Sawal,
Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu, David S. Miller,
Kalle Valo, Jakub Kicinski, Stanislaw Gruszka, Luca Coelho,
James E.J. Bottomley, Martin K. Petersen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Johannes Berg,
Mordechay Goodstein, Lee Jones, Wolfgang Grandegger,
Marc Kleine-Budde, Arunachalam Santhanam, Mikulas Patocka,
linux-crypto, ath10k, linux-wireless, netdev, linux-scsi,
linux-can, bpf, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
On Thu. 26 Aug 2021 at 14:04, Kees Cook <keescook@chromium.org> wrote:
> In support of enabling -Warray-bounds and -Wzero-length-bounds and
> correctly handling run-time memcpy() bounds checking, replace all
> open-coded flexible arrays (i.e. 0-element arrays) in unions with the
> flex_array() helper macro.
Nitpick: the commit description says flex_array() but the code is
actually DECLARE_FLEX_ARRAY() or __DECLARE_FLEX_ARRAY().
> This fixes warnings such as:
>
> fs/hpfs/anode.c: In function 'hpfs_add_sector_to_btree':
> fs/hpfs/anode.c:209:27: warning: array subscript 0 is outside the bounds of an interior zero-length array 'struct bplus_internal_node[0]' [-Wzero-length-bounds]
> 209 | anode->btree.u.internal[0].down = cpu_to_le32(a);
> | ~~~~~~~~~~~~~~~~~~~~~~~^~~
> In file included from fs/hpfs/hpfs_fn.h:26,
> from fs/hpfs/anode.c:10:
> fs/hpfs/hpfs.h:412:32: note: while referencing 'internal'
> 412 | struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
> | ^~~~~~~~
>
> drivers/net/can/usb/etas_es58x/es58x_fd.c: In function 'es58x_fd_tx_can_msg':
> drivers/net/can/usb/etas_es58x/es58x_fd.c:360:35: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned char[]'} [-Wzero-length-bounds]
> 360 | tx_can_msg = (typeof(tx_can_msg))&es58x_fd_urb_cmd->raw_msg[msg_len];
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/net/can/usb/etas_es58x/es58x_core.h:22,
> from drivers/net/can/usb/etas_es58x/es58x_fd.c:17:
> drivers/net/can/usb/etas_es58x/es58x_fd.h:231:6: note: while referencing 'raw_msg'
> 231 | u8 raw_msg[0];
> | ^~~~~~~
>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Ayush Sawal <ayush.sawal@chelsio.com>
> Cc: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
> Cc: Rohit Maheshwari <rohitm@chelsio.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Stanislaw Gruszka <stf_xl@wp.pl>
> Cc: Luca Coelho <luciano.coelho@intel.com>
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Song Liu <songliubraving@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Cc: KP Singh <kpsingh@kernel.org>
> Cc: Johannes Berg <johannes.berg@intel.com>
> Cc: Mordechay Goodstein <mordechay.goodstein@intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Wolfgang Grandegger <wg@grandegger.com>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Arunachalam Santhanam <arunachalam.santhanam@in.bosch.com>
> Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
> Cc: linux-crypto@vger.kernel.org
> Cc: ath10k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-scsi@vger.kernel.org
> Cc: linux-can@vger.kernel.org
> Cc: bpf@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/crypto/chelsio/chcr_crypto.h | 14 +++++++++-----
> drivers/net/can/usb/etas_es58x/es581_4.h | 2 +-
> drivers/net/can/usb/etas_es58x/es58x_fd.h | 2 +-
> drivers/net/wireless/ath/ath10k/htt.h | 7 +++++--
> drivers/net/wireless/intel/iwlegacy/commands.h | 6 ++++--
> drivers/net/wireless/intel/iwlwifi/dvm/commands.h | 6 ++++--
> drivers/net/wireless/intel/iwlwifi/fw/api/tx.h | 6 ++++--
> drivers/scsi/aic94xx/aic94xx_sds.c | 6 ++++--
> fs/hpfs/hpfs.h | 8 ++++----
> include/linux/filter.h | 6 ++++--
> include/scsi/sas.h | 12 ++++++++----
> include/uapi/rdma/rdma_user_rxe.h | 4 ++--
> include/uapi/sound/asoc.h | 4 ++--
> 13 files changed, 52 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/crypto/chelsio/chcr_crypto.h b/drivers/crypto/chelsio/chcr_crypto.h
> index e89f9e0094b4..c7816c83e324 100644
> --- a/drivers/crypto/chelsio/chcr_crypto.h
> +++ b/drivers/crypto/chelsio/chcr_crypto.h
> @@ -222,8 +222,10 @@ struct chcr_authenc_ctx {
> };
>
> struct __aead_ctx {
> - struct chcr_gcm_ctx gcm[0];
> - struct chcr_authenc_ctx authenc[];
> + union {
> + DECLARE_FLEX_ARRAY(struct chcr_gcm_ctx, gcm);
> + DECLARE_FLEX_ARRAY(struct chcr_authenc_ctx, authenc);
> + };
> };
>
> struct chcr_aead_ctx {
> @@ -245,9 +247,11 @@ struct hmac_ctx {
> };
>
> struct __crypto_ctx {
> - struct hmac_ctx hmacctx[0];
> - struct ablk_ctx ablkctx[0];
> - struct chcr_aead_ctx aeadctx[];
> + union {
> + DECLARE_FLEX_ARRAY(struct hmac_ctx, hmacctx);
> + DECLARE_FLEX_ARRAY(struct ablk_ctx, ablkctx);
> + DECLARE_FLEX_ARRAY(struct chcr_aead_ctx, aeadctx);
> + };
> };
>
> struct chcr_context {
> diff --git a/drivers/net/can/usb/etas_es58x/es581_4.h b/drivers/net/can/usb/etas_es58x/es581_4.h
> index 4bc60a6df697..667ecb77168c 100644
> --- a/drivers/net/can/usb/etas_es58x/es581_4.h
> +++ b/drivers/net/can/usb/etas_es58x/es581_4.h
> @@ -192,7 +192,7 @@ struct es581_4_urb_cmd {
> struct es581_4_rx_cmd_ret rx_cmd_ret;
> __le64 timestamp;
> u8 rx_cmd_ret_u8;
> - u8 raw_msg[0];
> + DECLARE_FLEX_ARRAY(u8, raw_msg);
> } __packed;
>
> __le16 reserved_for_crc16_do_not_use;
> diff --git a/drivers/net/can/usb/etas_es58x/es58x_fd.h b/drivers/net/can/usb/etas_es58x/es58x_fd.h
> index ee18a87e40c0..e33003f96e5e 100644
> --- a/drivers/net/can/usb/etas_es58x/es58x_fd.h
> +++ b/drivers/net/can/usb/etas_es58x/es58x_fd.h
> @@ -228,7 +228,7 @@ struct es58x_fd_urb_cmd {
> struct es58x_fd_tx_ack_msg tx_ack_msg;
> __le64 timestamp;
> __le32 rx_cmd_ret_le32;
> - u8 raw_msg[0];
> + DECLARE_FLEX_ARRAY(u8, raw_msg);
> } __packed;
>
> __le16 reserved_for_crc16_do_not_use;
> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
> index ec689e3ce48a..a6de08d3bf4a 100644
> --- a/drivers/net/wireless/ath/ath10k/htt.h
> +++ b/drivers/net/wireless/ath/ath10k/htt.h
> @@ -1674,8 +1674,11 @@ struct htt_tx_fetch_ind {
> __le32 token;
> __le16 num_resp_ids;
> __le16 num_records;
> - __le32 resp_ids[0]; /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
> - struct htt_tx_fetch_record records[];
> + union {
> + /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
> + DECLARE_FLEX_ARRAY(__le32, resp_ids);
> + DECLARE_FLEX_ARRAY(struct htt_tx_fetch_record, records);
> + };
> } __packed;
>
> static inline void *
> diff --git a/drivers/net/wireless/intel/iwlegacy/commands.h b/drivers/net/wireless/intel/iwlegacy/commands.h
> index 89c6671b32bc..4a97310f8fee 100644
> --- a/drivers/net/wireless/intel/iwlegacy/commands.h
> +++ b/drivers/net/wireless/intel/iwlegacy/commands.h
> @@ -1408,8 +1408,10 @@ struct il3945_tx_cmd {
> * MAC header goes here, followed by 2 bytes padding if MAC header
> * length is 26 or 30 bytes, followed by payload data
> */
> - u8 payload[0];
> - struct ieee80211_hdr hdr[];
> + union {
> + DECLARE_FLEX_ARRAY(u8, payload);
> + DECLARE_FLEX_ARRAY(struct ieee80211_hdr, hdr);
> + };
> } __packed;
>
> /*
> diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h
> index 235c7a2e3483..75a4b8e26232 100644
> --- a/drivers/net/wireless/intel/iwlwifi/dvm/commands.h
> +++ b/drivers/net/wireless/intel/iwlwifi/dvm/commands.h
> @@ -1251,8 +1251,10 @@ struct iwl_tx_cmd {
> * MAC header goes here, followed by 2 bytes padding if MAC header
> * length is 26 or 30 bytes, followed by payload data
> */
> - u8 payload[0];
> - struct ieee80211_hdr hdr[];
> + union {
> + DECLARE_FLEX_ARRAY(u8, payload);
> + DECLARE_FLEX_ARRAY(struct ieee80211_hdr, hdr);
> + };
> } __packed;
>
> /*
> diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h b/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
> index 24e4a82a55da..66c5487e857e 100644
> --- a/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
> +++ b/drivers/net/wireless/intel/iwlwifi/fw/api/tx.h
> @@ -713,8 +713,10 @@ struct iwl_mvm_compressed_ba_notif {
> __le32 tx_rate;
> __le16 tfd_cnt;
> __le16 ra_tid_cnt;
> - struct iwl_mvm_compressed_ba_ratid ra_tid[0];
> - struct iwl_mvm_compressed_ba_tfd tfd[];
> + union {
> + DECLARE_FLEX_ARRAY(struct iwl_mvm_compressed_ba_ratid, ra_tid);
> + DECLARE_FLEX_ARRAY(struct iwl_mvm_compressed_ba_tfd, tfd);
> + };
> } __packed; /* COMPRESSED_BA_RES_API_S_VER_4 */
>
> /**
> diff --git a/drivers/scsi/aic94xx/aic94xx_sds.c b/drivers/scsi/aic94xx/aic94xx_sds.c
> index 46815e65f7a4..5def83c88f13 100644
> --- a/drivers/scsi/aic94xx/aic94xx_sds.c
> +++ b/drivers/scsi/aic94xx/aic94xx_sds.c
> @@ -517,8 +517,10 @@ struct asd_ms_conn_map {
> u8 num_nodes;
> u8 usage_model_id;
> u32 _resvd;
> - struct asd_ms_conn_desc conn_desc[0];
> - struct asd_ms_node_desc node_desc[];
> + union {
> + DECLARE_FLEX_ARRAY(struct asd_ms_conn_desc, conn_desc);
> + DECLARE_FLEX_ARRAY(struct asd_ms_node_desc, node_desc);
> + };
> } __attribute__ ((packed));
>
> struct asd_ctrla_phy_entry {
> diff --git a/fs/hpfs/hpfs.h b/fs/hpfs/hpfs.h
> index d92c4af3e1b4..281dec8f636b 100644
> --- a/fs/hpfs/hpfs.h
> +++ b/fs/hpfs/hpfs.h
> @@ -409,10 +409,10 @@ struct bplus_header
> __le16 first_free; /* offset from start of header to
> first free node in array */
> union {
> - struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
> - subtree pointers */
> - struct bplus_leaf_node external[0]; /* (external) 3-word entries giving
> - sector runs */
> + /* (internal) 2-word entries giving subtree pointers */
> + DECLARE_FLEX_ARRAY(struct bplus_internal_node, internal);
> + /* (external) 3-word entries giving sector runs */
> + DECLARE_FLEX_ARRAY(struct bplus_leaf_node, external);
> } u;
> };
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 472f97074da0..5ca52bfa5868 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -572,8 +572,10 @@ struct bpf_prog {
> struct bpf_prog_aux *aux; /* Auxiliary fields */
> struct sock_fprog_kern *orig_prog; /* Original BPF program */
> /* Instructions for interpreter */
> - struct sock_filter insns[0];
> - struct bpf_insn insnsi[];
> + union {
> + DECLARE_FLEX_ARRAY(struct sock_filter, insns);
> + DECLARE_FLEX_ARRAY(struct bpf_insn, insnsi);
> + };
> };
>
> struct sk_filter {
> diff --git a/include/scsi/sas.h b/include/scsi/sas.h
> index 4726c1bbec65..64154c1fed02 100644
> --- a/include/scsi/sas.h
> +++ b/include/scsi/sas.h
> @@ -323,8 +323,10 @@ struct ssp_response_iu {
> __be32 sense_data_len;
> __be32 response_data_len;
>
> - u8 resp_data[0];
> - u8 sense_data[];
> + union {
> + DECLARE_FLEX_ARRAY(u8, resp_data);
> + DECLARE_FLEX_ARRAY(u8, sense_data);
> + };
> } __attribute__ ((packed));
>
> struct ssp_command_iu {
> @@ -554,8 +556,10 @@ struct ssp_response_iu {
> __be32 sense_data_len;
> __be32 response_data_len;
>
> - u8 resp_data[0];
> - u8 sense_data[];
> + union {
> + DECLARE_FLEX_ARRAY(u8, resp_data);
> + DECLARE_FLEX_ARRAY(u8, sense_data);
> + };
> } __attribute__ ((packed));
>
> struct ssp_command_iu {
> diff --git a/include/uapi/rdma/rdma_user_rxe.h b/include/uapi/rdma/rdma_user_rxe.h
> index e283c2220aba..040752c99ec9 100644
> --- a/include/uapi/rdma/rdma_user_rxe.h
> +++ b/include/uapi/rdma/rdma_user_rxe.h
> @@ -141,8 +141,8 @@ struct rxe_dma_info {
> __u32 sge_offset;
> __u32 reserved;
> union {
> - __u8 inline_data[0];
> - struct rxe_sge sge[0];
> + __DECLARE_FLEX_ARRAY(u8, inline_data);
> + __DECLARE_FLEX_ARRAY(struct rxe_sge, sge);
> };
> };
>
> diff --git a/include/uapi/sound/asoc.h b/include/uapi/sound/asoc.h
> index da61398b1f8f..053949287ce8 100644
> --- a/include/uapi/sound/asoc.h
> +++ b/include/uapi/sound/asoc.h
> @@ -240,8 +240,8 @@ struct snd_soc_tplg_vendor_array {
> struct snd_soc_tplg_private {
> __le32 size; /* in bytes of private data */
> union {
> - char data[0];
> - struct snd_soc_tplg_vendor_array array[0];
> + __DECLARE_FLEX_ARRAY(char, data);
> + __DECLARE_FLEX_ARRAY(struct snd_soc_tplg_vendor_array, array);
> };
> } __attribute__((packed));
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-26 7:36 ` Vincent MAILHOL
@ 2021-08-26 15:39 ` Kees Cook
0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2021-08-26 15:39 UTC (permalink / raw)
To: Vincent MAILHOL
Cc: open list, Gustavo A. R. Silva, Arnd Bergmann, Ayush Sawal,
Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu, David S. Miller,
Kalle Valo, Jakub Kicinski, Stanislaw Gruszka, Luca Coelho,
James E.J. Bottomley, Martin K. Petersen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Johannes Berg,
Mordechay Goodstein, Lee Jones, Wolfgang Grandegger,
Marc Kleine-Budde, Arunachalam Santhanam, Mikulas Patocka,
linux-crypto, ath10k, linux-wireless, netdev, linux-scsi,
linux-can, bpf, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
On August 26, 2021 12:36:27 AM PDT, Vincent MAILHOL <mailhol.vincent@wanadoo.fr> wrote:
>On Thu. 26 Aug 2021 at 14:04, Kees Cook <keescook@chromium.org> wrote:
>> In support of enabling -Warray-bounds and -Wzero-length-bounds and
>> correctly handling run-time memcpy() bounds checking, replace all
>> open-coded flexible arrays (i.e. 0-element arrays) in unions with the
>> flex_array() helper macro.
>
>Nitpick: the commit description says flex_array() but the code is
>actually DECLARE_FLEX_ARRAY() or __DECLARE_FLEX_ARRAY().
Ah yes, thanks! I had renamed the macro but missed these references. Now fixed!
-Kees
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-26 6:24 ` Marc Kleine-Budde
@ 2021-08-27 16:08 ` Kees Cook
2021-08-27 17:08 ` Marc Kleine-Budde
2021-08-27 16:17 ` Kees Cook
1 sibling, 1 reply; 8+ messages in thread
From: Kees Cook @ 2021-08-27 16:08 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-kernel, Gustavo A. R. Silva, Arnd Bergmann, Ayush Sawal,
Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu, David S. Miller,
Kalle Valo, Jakub Kicinski, Stanislaw Gruszka, Luca Coelho,
James E.J. Bottomley, Martin K. Petersen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Johannes Berg,
Mordechay Goodstein, Lee Jones, Wolfgang Grandegger,
Arunachalam Santhanam, Vincent Mailhol, Mikulas Patocka,
linux-crypto, ath10k, linux-wireless, netdev, linux-scsi,
linux-can, bpf, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
On Thu, Aug 26, 2021 at 08:24:52AM +0200, Marc Kleine-Budde wrote:
> On 25.08.2021 22:04:55, Kees Cook wrote:
> > In support of enabling -Warray-bounds and -Wzero-length-bounds and
> > correctly handling run-time memcpy() bounds checking, replace all
> > open-coded flexible arrays (i.e. 0-element arrays) in unions with the
> > flex_array() helper macro.
> >
> > This fixes warnings such as:
> >
> > fs/hpfs/anode.c: In function 'hpfs_add_sector_to_btree':
> > fs/hpfs/anode.c:209:27: warning: array subscript 0 is outside the bounds of an interior zero-length array 'struct bplus_internal_node[0]' [-Wzero-length-bounds]
> > 209 | anode->btree.u.internal[0].down = cpu_to_le32(a);
> > | ~~~~~~~~~~~~~~~~~~~~~~~^~~
> > In file included from fs/hpfs/hpfs_fn.h:26,
> > from fs/hpfs/anode.c:10:
> > fs/hpfs/hpfs.h:412:32: note: while referencing 'internal'
> > 412 | struct bplus_internal_node internal[0]; /* (internal) 2-word entries giving
> > | ^~~~~~~~
> >
> > drivers/net/can/usb/etas_es58x/es58x_fd.c: In function 'es58x_fd_tx_can_msg':
> > drivers/net/can/usb/etas_es58x/es58x_fd.c:360:35: warning: array subscript 65535 is outside the bounds of an interior zero-length array 'u8[0]' {aka 'unsigned char[]'} [-Wzero-length-bounds]
> > 360 | tx_can_msg = (typeof(tx_can_msg))&es58x_fd_urb_cmd->raw_msg[msg_len];
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > In file included from drivers/net/can/usb/etas_es58x/es58x_core.h:22,
> > from drivers/net/can/usb/etas_es58x/es58x_fd.c:17:
> > drivers/net/can/usb/etas_es58x/es58x_fd.h:231:6: note: while referencing 'raw_msg'
> > 231 | u8 raw_msg[0];
> > | ^~~~~~~
> >
> > Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Ayush Sawal <ayush.sawal@chelsio.com>
> > Cc: Vinay Kumar Yadav <vinay.yadav@chelsio.com>
> > Cc: Rohit Maheshwari <rohitm@chelsio.com>
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Stanislaw Gruszka <stf_xl@wp.pl>
> > Cc: Luca Coelho <luciano.coelho@intel.com>
> > Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> > Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> > Cc: Alexei Starovoitov <ast@kernel.org>
> > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > Cc: Andrii Nakryiko <andrii@kernel.org>
> > Cc: Martin KaFai Lau <kafai@fb.com>
> > Cc: Song Liu <songliubraving@fb.com>
> > Cc: Yonghong Song <yhs@fb.com>
> > Cc: John Fastabend <john.fastabend@gmail.com>
> > Cc: KP Singh <kpsingh@kernel.org>
> > Cc: Johannes Berg <johannes.berg@intel.com>
> > Cc: Mordechay Goodstein <mordechay.goodstein@intel.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Wolfgang Grandegger <wg@grandegger.com>
> > Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> > Cc: Arunachalam Santhanam <arunachalam.santhanam@in.bosch.com>
> > Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > Cc: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
> > Cc: linux-crypto@vger.kernel.org
> > Cc: ath10k@lists.infradead.org
> > Cc: linux-wireless@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Cc: linux-scsi@vger.kernel.org
> > Cc: linux-can@vger.kernel.org
> > Cc: bpf@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > drivers/net/can/usb/etas_es58x/es581_4.h | 2 +-
> > drivers/net/can/usb/etas_es58x/es58x_fd.h | 2 +-
>
> For the can drivers:
>
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Thanks!
> BTW: Is there opportunity for conversion, too?
>
> | drivers/net/can/peak_canfd/peak_pciefd_main.c:146:32: warning: array of flexible structures
Oh, hrmpf. This isn't a sane use of flex arrays:
struct __packed pucan_rx_msg {
...
__le32 can_id;
u8 d[];
};
struct pciefd_rx_dma {
__le32 irq_status;
__le32 sys_time_low;
__le32 sys_time_high;
struct pucan_rx_msg msg[];
} __packed __aligned(4);
I think that needs to be handled separately. How are you building to get
that warning, by the way? I haven't seen that in my builds...
--
Kees Cook
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-26 6:24 ` Marc Kleine-Budde
2021-08-27 16:08 ` Kees Cook
@ 2021-08-27 16:17 ` Kees Cook
2021-08-28 7:31 ` Vincent MAILHOL
1 sibling, 1 reply; 8+ messages in thread
From: Kees Cook @ 2021-08-27 16:17 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-kernel, Gustavo A. R. Silva, Arnd Bergmann, Ayush Sawal,
Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu, David S. Miller,
Kalle Valo, Jakub Kicinski, Stanislaw Gruszka, Luca Coelho,
James E.J. Bottomley, Martin K. Petersen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Johannes Berg,
Mordechay Goodstein, Lee Jones, Wolfgang Grandegger,
Arunachalam Santhanam, Vincent Mailhol, Mikulas Patocka,
linux-crypto, ath10k, linux-wireless, netdev, linux-scsi,
linux-can, bpf, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
On Thu, Aug 26, 2021 at 08:24:52AM +0200, Marc Kleine-Budde wrote:
> [...]
> BTW: Is there opportunity for conversion, too?
>
> | drivers/net/can/peak_canfd/peak_pciefd_main.c:146:32: warning: array of flexible structures
Untested potential solution:
diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c b/drivers/net/can/peak_canfd/peak_pciefd_main.c
index 1df3c4b54f03..efa2b5a52bd7 100644
--- a/drivers/net/can/peak_canfd/peak_pciefd_main.c
+++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c
@@ -143,7 +143,11 @@ struct pciefd_rx_dma {
__le32 irq_status;
__le32 sys_time_low;
__le32 sys_time_high;
- struct pucan_rx_msg msg[];
+ /*
+ * with "msg" being pciefd_irq_rx_cnt(priv->irq_status)-many
+ * variable-sized struct pucan_rx_msg following.
+ */
+ __le32 msg[];
} __packed __aligned(4);
/* Tx Link record */
@@ -327,7 +331,7 @@ static irqreturn_t pciefd_irq_handler(int irq, void *arg)
/* handle rx messages (if any) */
peak_canfd_handle_msgs_list(&priv->ucan,
- rx_dma->msg,
+ (struct pucan_rx_msg *)rx_dma->msg,
pciefd_irq_rx_cnt(priv->irq_status));
/* handle tx link interrupt (if any) */
It's not great, but it's also not strictly a flex array, in the sense
that since struct pucan_rx_msg is a variable size, the compiler cannot
reason about the size of struct pciefd_rx_dma based only on the
irq_status encoding...
--
Kees Cook
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-27 16:08 ` Kees Cook
@ 2021-08-27 17:08 ` Marc Kleine-Budde
0 siblings, 0 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2021-08-27 17:08 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Gustavo A. R. Silva, Arnd Bergmann, Ayush Sawal,
Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu, David S. Miller,
Kalle Valo, Jakub Kicinski, Stanislaw Gruszka, Luca Coelho,
James E.J. Bottomley, Martin K. Petersen, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Johannes Berg,
Mordechay Goodstein, Lee Jones, Wolfgang Grandegger,
Arunachalam Santhanam, Vincent Mailhol, Mikulas Patocka,
linux-crypto, ath10k, linux-wireless, netdev, linux-scsi,
linux-can, bpf, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]
On 27.08.2021 09:08:19, Kees Cook wrote:
> > BTW: Is there opportunity for conversion, too?
> >
> > | drivers/net/can/peak_canfd/peak_pciefd_main.c:146:32: warning: array of flexible structures
>
> Oh, hrmpf. This isn't a sane use of flex arrays:
>
> struct __packed pucan_rx_msg {
> ...
> __le32 can_id;
> u8 d[];
> };
>
> struct pciefd_rx_dma {
> __le32 irq_status;
> __le32 sys_time_low;
> __le32 sys_time_high;
> struct pucan_rx_msg msg[];
> } __packed __aligned(4);
>
> I think that needs to be handled separately. How are you building to get
> that warning, by the way? I haven't seen that in my builds...
If compiling with C=1, sparse will complain about this.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-27 16:17 ` Kees Cook
@ 2021-08-28 7:31 ` Vincent MAILHOL
0 siblings, 0 replies; 8+ messages in thread
From: Vincent MAILHOL @ 2021-08-28 7:31 UTC (permalink / raw)
To: Kees Cook
Cc: Marc Kleine-Budde, open list, Gustavo A. R. Silva, Arnd Bergmann,
Ayush Sawal, Vinay Kumar Yadav, Rohit Maheshwari, Herbert Xu,
David S. Miller, Kalle Valo, Jakub Kicinski, Stanislaw Gruszka,
Luca Coelho, James E.J. Bottomley, Martin K. Petersen,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Johannes Berg, Mordechay Goodstein, Lee Jones,
Wolfgang Grandegger, Arunachalam Santhanam, Mikulas Patocka,
linux-crypto, ath10k, linux-wireless, netdev, linux-scsi,
linux-can, bpf, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
Le sam. 28 août 2021 à 01:17, Kees Cook <keescook@chromium.org> a écrit :
>
> On Thu, Aug 26, 2021 at 08:24:52AM +0200, Marc Kleine-Budde wrote:
> > [...]
> > BTW: Is there opportunity for conversion, too?
> >
> > | drivers/net/can/peak_canfd/peak_pciefd_main.c:146:32: warning: array of flexible structures
>
> Untested potential solution:
>
> diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c b/drivers/net/can/peak_canfd/peak_pciefd_main.c
> index 1df3c4b54f03..efa2b5a52bd7 100644
> --- a/drivers/net/can/peak_canfd/peak_pciefd_main.c
> +++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c
> @@ -143,7 +143,11 @@ struct pciefd_rx_dma {
> __le32 irq_status;
> __le32 sys_time_low;
> __le32 sys_time_high;
> - struct pucan_rx_msg msg[];
> + /*
> + * with "msg" being pciefd_irq_rx_cnt(priv->irq_status)-many
> + * variable-sized struct pucan_rx_msg following.
> + */
> + __le32 msg[];
Isn't u8 msg[] preferable here?
> } __packed __aligned(4);
>
> /* Tx Link record */
> @@ -327,7 +331,7 @@ static irqreturn_t pciefd_irq_handler(int irq, void *arg)
>
> /* handle rx messages (if any) */
> peak_canfd_handle_msgs_list(&priv->ucan,
> - rx_dma->msg,
> + (struct pucan_rx_msg *)rx_dma->msg,
> pciefd_irq_rx_cnt(priv->irq_status));
>
> /* handle tx link interrupt (if any) */
>
>
> It's not great, but it's also not strictly a flex array, in the sense
> that since struct pucan_rx_msg is a variable size, the compiler cannot
> reason about the size of struct pciefd_rx_dma based only on the
> irq_status encoding...
In the same spirit, it is a bit cleaner to change the prototype of
handle_msgs_list().
Like that:
diff --git a/drivers/net/can/peak_canfd/peak_canfd.c b/drivers/net/can/peak_canf
d/peak_canfd.c
index d08718e98e11..81a9faa6193f 100644
--- a/drivers/net/can/peak_canfd/peak_canfd.c
+++ b/drivers/net/can/peak_canfd/peak_canfd.c
@@ -484,9 +484,8 @@ int peak_canfd_handle_msg(struct peak_canfd_priv *priv,
/* handle a list of rx_count messages from rx_msg memory address */
int peak_canfd_handle_msgs_list(struct peak_canfd_priv *priv,
- struct pucan_rx_msg *msg_list, int msg_count)
+ void *msg_ptr, int msg_count)
{
- void *msg_ptr = msg_list;
int i, msg_size = 0;
for (i = 0; i < msg_count; i++) {
diff --git a/drivers/net/can/peak_canfd/peak_canfd_user.h b/drivers/net/can/peak
_canfd/peak_canfd_user.h
index a72719dc3b74..ef91f92e70c3 100644
--- a/drivers/net/can/peak_canfd/peak_canfd_user.h
+++ b/drivers/net/can/peak_canfd/peak_canfd_user.h
@@ -42,5 +42,5 @@ struct net_device *alloc_peak_canfd_dev(int
sizeof_priv, int index,
int peak_canfd_handle_msg(struct peak_canfd_priv *priv,
struct pucan_rx_msg *msg);
int peak_canfd_handle_msgs_list(struct peak_canfd_priv *priv,
- struct pucan_rx_msg *rx_msg, int rx_count);
+ void *msg_ptr, int rx_count);
#endif
diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c
b/drivers/net/can/peak_canfd/peak_pciefd_main.c
index 1df3c4b54f03..c1de1e3dc4bc 100644
--- a/drivers/net/can/peak_canfd/peak_pciefd_main.c
+++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c
@@ -143,7 +143,11 @@ struct pciefd_rx_dma {
__le32 irq_status;
__le32 sys_time_low;
__le32 sys_time_high;
- struct pucan_rx_msg msg[];
+ /*
+ * with "msg" being pciefd_irq_rx_cnt(priv->irq_status)-many
+ * variable-sized struct pucan_rx_msg following.
+ */
+ u8 msg[];
} __packed __aligned(4);
/* Tx Link record */
Another solution would be to declare a maximum length for struct
pucan_rx_msg::d. Because these are CAN FD messages, I suppose
that maximum length would be CANFD_MAX_DLEN. struct canfd_frame
from the UAPI uses the same pattern.
N.B. This solution is not exclusive from the above one (actually,
I think that using both would be the best solution).
diff --git a/include/linux/can/dev/peak_canfd.h
b/include/linux/can/dev/peak_canfd.h
index f38772fd0c07..a048359db430 100644
--- a/include/linux/can/dev/peak_canfd.h
+++ b/include/linux/can/dev/peak_canfd.h
@@ -189,7 +189,7 @@ struct __packed pucan_rx_msg {
u8 client;
__le16 flags;
__le32 can_id;
- u8 d[];
+ u8 d[CANFD_MAX_DLEN];
};
/* uCAN error types */
I only tested for compilation.
Yours sincerely,
Vincent
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-28 7:31 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210826050458.1540622-1-keescook@chromium.org>
2021-08-26 5:04 ` [PATCH v2 2/5] treewide: Replace open-coded flex arrays in unions Kees Cook
2021-08-26 6:24 ` Marc Kleine-Budde
2021-08-27 16:08 ` Kees Cook
2021-08-27 17:08 ` Marc Kleine-Budde
2021-08-27 16:17 ` Kees Cook
2021-08-28 7:31 ` Vincent MAILHOL
2021-08-26 7:36 ` Vincent MAILHOL
2021-08-26 15:39 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).