* [PATCH v3 1/5] stddef: Introduce DECLARE_FLEX_ARRAY() helper
2021-08-27 16:30 [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Kees Cook
@ 2021-08-27 16:30 ` Kees Cook
2021-08-27 16:30 ` [PATCH v3 2/5] treewide: Replace open-coded flex arrays in unions Kees Cook
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Kees Cook @ 2021-08-27 16:30 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Arnd Bergmann, Gustavo A. R. Silva, Rasmus Villemoes,
Keith Packard, Dan Williams, Daniel Vetter, clang-built-linux,
linux-hardening
There are many places where kernel code wants to have several different
typed trailing flexible arrays. This would normally be done with multiple
flexible arrays in a union, but since GCC and Clang don't (on the surface)
allow this, there have been many open-coded workarounds, usually involving
neighboring 0-element arrays at the end of a structure. For example,
instead of something like this:
struct thing {
...
union {
struct type1 foo[];
struct type2 bar[];
};
};
code works around the compiler with:
struct thing {
...
struct type1 foo[0];
struct type2 bar[];
};
Another case is when a flexible array is wanted as the single member
within a struct (which itself is usually in a union). For example, this
would be worked around as:
union many {
...
struct {
struct type3 baz[0];
};
};
These kinds of work-arounds cause problems with size checks against such
zero-element arrays (for example when building with -Warray-bounds and
-Wzero-length-bounds, and with the coming FORTIFY_SOURCE improvements),
so they must all be converted to "real" flexible arrays, avoiding warnings
like this:
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];
| ^~~~~~~
However, it _is_ entirely possible to have one or more flexible arrays
in a struct or union: it just has to be in another struct. And since it
cannot be alone in a struct, such a struct must have at least 1 other
named member -- but that member can be zero sized. Wrap all this nonsense
into the new DECLARE_FLEX_ARRAY() in support of having flexible arrays
in unions (or alone in a struct).
As with struct_group(), since this is needed in UAPI headers as well,
implement the core there, with a non-UAPI wrapper.
Additionally update kernel-doc to understand its existence.
https://github.com/KSPP/linux/issues/137
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/stddef.h | 13 +++++++++++++
include/uapi/linux/stddef.h | 16 ++++++++++++++++
scripts/kernel-doc | 2 ++
3 files changed, 31 insertions(+)
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index 8b103a53b000..ca507bd5f808 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -84,4 +84,17 @@ enum {
#define struct_group_tagged(TAG, NAME, MEMBERS...) \
__struct_group(TAG, NAME, /* no attrs */, MEMBERS)
+/**
+ * DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union
+ *
+ * @TYPE: The type of each flexible array element
+ * @NAME: The name of the flexible array member
+ *
+ * In order to have a flexible array member in a union or alone in a
+ * struct, it needs to be wrapped in an anonymous struct with at least 1
+ * named member, but that member can be empty.
+ */
+#define DECLARE_FLEX_ARRAY(TYPE, NAME) \
+ __DECLARE_FLEX_ARRAY(TYPE, NAME)
+
#endif
diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
index 610204f7c275..3021ea25a284 100644
--- a/include/uapi/linux/stddef.h
+++ b/include/uapi/linux/stddef.h
@@ -25,3 +25,19 @@
struct { MEMBERS } ATTRS; \
struct TAG { MEMBERS } ATTRS NAME; \
}
+
+/**
+ * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union
+ *
+ * @TYPE: The type of each flexible array element
+ * @NAME: The name of the flexible array member
+ *
+ * In order to have a flexible array member in a union or alone in a
+ * struct, it needs to be wrapped in an anonymous struct with at least 1
+ * named member, but that member can be empty.
+ */
+#define __DECLARE_FLEX_ARRAY(TYPE, NAME) \
+ struct { \
+ struct { } __empty_ ## NAME; \
+ TYPE NAME[]; \
+ }
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index d9715efbe0b7..65088b512d14 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -1263,6 +1263,8 @@ sub dump_struct($$) {
$members =~ s/DECLARE_KFIFO\s*\($args,\s*$args,\s*$args\)/$2 \*$1/gos;
# replace DECLARE_KFIFO_PTR
$members =~ s/DECLARE_KFIFO_PTR\s*\($args,\s*$args\)/$2 \*$1/gos;
+ # replace DECLARE_FLEX_ARRAY
+ $members =~ s/(?:__)?DECLARE_FLEX_ARRAY\s*\($args,\s*$args\)/$1 $2\[\]/gos;
my $declaration = $members;
# Split nested struct/union elements as newer ones
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-27 16:30 [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Kees Cook
2021-08-27 16:30 ` [PATCH v3 1/5] stddef: Introduce DECLARE_FLEX_ARRAY() helper Kees Cook
@ 2021-08-27 16:30 ` Kees Cook
2021-08-28 7:51 ` Vincent MAILHOL
2021-08-27 16:30 ` [PATCH v3 3/5] treewide: Replace 0-element memcpy() destinations with flexible arrays Kees Cook
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2021-08-27 16:30 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
DECLARE_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
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> # drivers/net/can/usb/etas_es58x/*
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..7f44d54bb0ab 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] 16+ messages in thread* Re: [PATCH v3 2/5] treewide: Replace open-coded flex arrays in unions
2021-08-27 16:30 ` [PATCH v3 2/5] treewide: Replace open-coded flex arrays in unions Kees Cook
@ 2021-08-28 7:51 ` Vincent MAILHOL
0 siblings, 0 replies; 16+ messages in thread
From: Vincent MAILHOL @ 2021-08-28 7:51 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 Tue. 28 Aug 2021 at 01:30, 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
> DECLARE_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
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> # drivers/net/can/usb/etas_es58x/*
> 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..7f44d54bb0ab 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));
Thanks for addressing all my remarks on the commit message.
FYI, I compared the assembly code before and after the patch:
both give the same output (as expected).
Acked-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/5] treewide: Replace 0-element memcpy() destinations with flexible arrays
2021-08-27 16:30 [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Kees Cook
2021-08-27 16:30 ` [PATCH v3 1/5] stddef: Introduce DECLARE_FLEX_ARRAY() helper Kees Cook
2021-08-27 16:30 ` [PATCH v3 2/5] treewide: Replace open-coded flex arrays in unions Kees Cook
@ 2021-08-27 16:30 ` Kees Cook
2021-08-27 16:30 ` [PATCH v3 4/5] Makefile: Enable -Warray-bounds Kees Cook
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Kees Cook @ 2021-08-27 16:30 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Gustavo A. R. Silva, Arnd Bergmann, Kalle Valo,
David S. Miller, Jakub Kicinski, Nilesh Javali, Manish Rangankar,
GR-QLogic-Storage-Upstream, James E.J. Bottomley,
Martin K. Petersen, Larry Finger, Phillip Potter,
Greg Kroah-Hartman, Florian Schilhabel, Johannes Berg,
Christophe JAILLET, Fabio Aiuto, Ross Schmidt, Marco Cesati,
ath10k, linux-wireless, netdev, linux-scsi, linux-staging,
Rasmus Villemoes, Keith Packard, Dan Williams, Daniel Vetter,
clang-built-linux, linux-hardening
The 0-element arrays that are used as memcpy() destinations are actually
flexible arrays. Adjust their structures accordingly so that memcpy()
can better reason able their destination size (i.e. they need to be seen
as "unknown" length rather than "zero").
In some cases, use of the DECLARE_FLEX_ARRAY() helper is needed when a
flexible array is alone in a struct.
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Nilesh Javali <njavali@marvell.com>
Cc: Manish Rangankar <mrangankar@marvell.com>
Cc: GR-QLogic-Storage-Upstream@marvell.com
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Phillip Potter <phil@philpotter.co.uk>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Florian Schilhabel <florian.c.schilhabel@googlemail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: Fabio Aiuto <fabioaiuto83@gmail.com>
Cc: Ross Schmidt <ross.schm.dev@gmail.com>
Cc: Marco Cesati <marcocesati@gmail.com>
Cc: ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-staging@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/wireless/ath/ath10k/bmi.h | 10 +++----
drivers/scsi/qla4xxx/ql4_def.h | 4 +--
drivers/staging/rtl8188eu/include/ieee80211.h | 6 ++--
drivers/staging/rtl8712/ieee80211.h | 4 +--
drivers/staging/rtl8723bs/include/ieee80211.h | 6 ++--
include/linux/ieee80211.h | 30 +++++++++----------
include/uapi/linux/dlm_device.h | 4 +--
7 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/bmi.h b/drivers/net/wireless/ath/ath10k/bmi.h
index f6fadcbdd86e..0685c0d2d4ea 100644
--- a/drivers/net/wireless/ath/ath10k/bmi.h
+++ b/drivers/net/wireless/ath/ath10k/bmi.h
@@ -109,7 +109,7 @@ struct bmi_cmd {
struct {
__le32 addr;
__le32 len;
- u8 payload[0];
+ u8 payload[];
} write_mem;
struct {
__le32 addr;
@@ -138,18 +138,18 @@ struct bmi_cmd {
} rompatch_uninstall;
struct {
__le32 count;
- __le32 patch_ids[0]; /* length of @count */
+ __le32 patch_ids[]; /* length of @count */
} rompatch_activate;
struct {
__le32 count;
- __le32 patch_ids[0]; /* length of @count */
+ __le32 patch_ids[]; /* length of @count */
} rompatch_deactivate;
struct {
__le32 addr;
} lz_start;
struct {
__le32 len; /* max BMI_MAX_DATA_SIZE */
- u8 payload[0]; /* length of @len */
+ u8 payload[]; /* length of @len */
} lz_data;
struct {
u8 name[BMI_NVRAM_SEG_NAME_SZ];
@@ -160,7 +160,7 @@ struct bmi_cmd {
union bmi_resp {
struct {
- u8 payload[0];
+ DECLARE_FLEX_ARRAY(u8, payload);
} read_mem;
struct {
__le32 result;
diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h
index 031569c496e5..69a590546bf9 100644
--- a/drivers/scsi/qla4xxx/ql4_def.h
+++ b/drivers/scsi/qla4xxx/ql4_def.h
@@ -366,13 +366,13 @@ struct qla4_work_evt {
struct {
enum iscsi_host_event_code code;
uint32_t data_size;
- uint8_t data[0];
+ uint8_t data[];
} aen;
struct {
uint32_t status;
uint32_t pid;
uint32_t data_size;
- uint8_t data[0];
+ uint8_t data[];
} ping;
} u;
};
diff --git a/drivers/staging/rtl8188eu/include/ieee80211.h b/drivers/staging/rtl8188eu/include/ieee80211.h
index da6245a77d5d..aa5c1a513495 100644
--- a/drivers/staging/rtl8188eu/include/ieee80211.h
+++ b/drivers/staging/rtl8188eu/include/ieee80211.h
@@ -199,7 +199,7 @@ struct ieee_param {
struct {
u32 len;
u8 reserved[32];
- u8 data[0];
+ u8 data[];
} wpa_ie;
struct {
int command;
@@ -212,7 +212,7 @@ struct ieee_param {
u8 idx;
u8 seq[8]; /* sequence counter (set: RX, get: TX) */
u16 key_len;
- u8 key[0];
+ u8 key[];
} crypt;
#ifdef CONFIG_88EU_AP_MODE
struct {
@@ -224,7 +224,7 @@ struct ieee_param {
} add_sta;
struct {
u8 reserved[2];/* for set max_num_sta */
- u8 buf[0];
+ u8 buf[];
} bcn_ie;
#endif
diff --git a/drivers/staging/rtl8712/ieee80211.h b/drivers/staging/rtl8712/ieee80211.h
index 61eff7c5746b..65ceaca9b51e 100644
--- a/drivers/staging/rtl8712/ieee80211.h
+++ b/drivers/staging/rtl8712/ieee80211.h
@@ -78,7 +78,7 @@ struct ieee_param {
struct {
u32 len;
u8 reserved[32];
- u8 data[0];
+ u8 data[];
} wpa_ie;
struct {
int command;
@@ -91,7 +91,7 @@ struct ieee_param {
u8 idx;
u8 seq[8]; /* sequence counter (set: RX, get: TX) */
u16 key_len;
- u8 key[0];
+ u8 key[];
} crypt;
} u;
};
diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h b/drivers/staging/rtl8723bs/include/ieee80211.h
index 378c21595e05..89c311cd20a6 100644
--- a/drivers/staging/rtl8723bs/include/ieee80211.h
+++ b/drivers/staging/rtl8723bs/include/ieee80211.h
@@ -180,7 +180,7 @@ struct ieee_param {
struct {
u32 len;
u8 reserved[32];
- u8 data[0];
+ u8 data[];
} wpa_ie;
struct{
int command;
@@ -193,7 +193,7 @@ struct ieee_param {
u8 idx;
u8 seq[8]; /* sequence counter (set: RX, get: TX) */
u16 key_len;
- u8 key[0];
+ u8 key[];
} crypt;
struct {
u16 aid;
@@ -204,7 +204,7 @@ struct ieee_param {
} add_sta;
struct {
u8 reserved[2];/* for set max_num_sta */
- u8 buf[0];
+ u8 buf[];
} bcn_ie;
} u;
};
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index a6730072d13a..445597c03cd1 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1101,7 +1101,7 @@ struct ieee80211_mgmt {
__le16 auth_transaction;
__le16 status_code;
/* possibly followed by Challenge text */
- u8 variable[0];
+ u8 variable[];
} __packed auth;
struct {
__le16 reason_code;
@@ -1110,26 +1110,26 @@ struct ieee80211_mgmt {
__le16 capab_info;
__le16 listen_interval;
/* followed by SSID and Supported rates */
- u8 variable[0];
+ u8 variable[];
} __packed assoc_req;
struct {
__le16 capab_info;
__le16 status_code;
__le16 aid;
/* followed by Supported rates */
- u8 variable[0];
+ u8 variable[];
} __packed assoc_resp, reassoc_resp;
struct {
__le16 capab_info;
__le16 status_code;
- u8 variable[0];
+ u8 variable[];
} __packed s1g_assoc_resp, s1g_reassoc_resp;
struct {
__le16 capab_info;
__le16 listen_interval;
u8 current_ap[ETH_ALEN];
/* followed by SSID and Supported rates */
- u8 variable[0];
+ u8 variable[];
} __packed reassoc_req;
struct {
__le16 reason_code;
@@ -1140,11 +1140,11 @@ struct ieee80211_mgmt {
__le16 capab_info;
/* followed by some of SSID, Supported rates,
* FH Params, DS Params, CF Params, IBSS Params, TIM */
- u8 variable[0];
+ u8 variable[];
} __packed beacon;
struct {
/* only variable items: SSID, Supported rates */
- u8 variable[0];
+ DECLARE_FLEX_ARRAY(u8, variable);
} __packed probe_req;
struct {
__le64 timestamp;
@@ -1152,7 +1152,7 @@ struct ieee80211_mgmt {
__le16 capab_info;
/* followed by some of SSID, Supported rates,
* FH Params, DS Params, CF Params, IBSS Params */
- u8 variable[0];
+ u8 variable[];
} __packed probe_resp;
struct {
u8 category;
@@ -1161,16 +1161,16 @@ struct ieee80211_mgmt {
u8 action_code;
u8 dialog_token;
u8 status_code;
- u8 variable[0];
+ u8 variable[];
} __packed wme_action;
struct{
u8 action_code;
- u8 variable[0];
+ u8 variable[];
} __packed chan_switch;
struct{
u8 action_code;
struct ieee80211_ext_chansw_ie data;
- u8 variable[0];
+ u8 variable[];
} __packed ext_chan_switch;
struct{
u8 action_code;
@@ -1186,7 +1186,7 @@ struct ieee80211_mgmt {
__le16 timeout;
__le16 start_seq_num;
/* followed by BA Extension */
- u8 variable[0];
+ u8 variable[];
} __packed addba_req;
struct{
u8 action_code;
@@ -1202,11 +1202,11 @@ struct ieee80211_mgmt {
} __packed delba;
struct {
u8 action_code;
- u8 variable[0];
+ u8 variable[];
} __packed self_prot;
struct{
u8 action_code;
- u8 variable[0];
+ u8 variable[];
} __packed mesh_action;
struct {
u8 action;
@@ -1250,7 +1250,7 @@ struct ieee80211_mgmt {
u8 toa[6];
__le16 tod_error;
__le16 toa_error;
- u8 variable[0];
+ u8 variable[];
} __packed ftm;
} u;
} __packed action;
diff --git a/include/uapi/linux/dlm_device.h b/include/uapi/linux/dlm_device.h
index f880d2831160..e83954c69fff 100644
--- a/include/uapi/linux/dlm_device.h
+++ b/include/uapi/linux/dlm_device.h
@@ -45,13 +45,13 @@ struct dlm_lock_params {
void __user *bastaddr;
struct dlm_lksb __user *lksb;
char lvb[DLM_USER_LVB_LEN];
- char name[0];
+ char name[];
};
struct dlm_lspace_params {
__u32 flags;
__u32 minor;
- char name[0];
+ char name[];
};
struct dlm_purge_params {
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 4/5] Makefile: Enable -Warray-bounds
2021-08-27 16:30 [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Kees Cook
` (2 preceding siblings ...)
2021-08-27 16:30 ` [PATCH v3 3/5] treewide: Replace 0-element memcpy() destinations with flexible arrays Kees Cook
@ 2021-08-27 16:30 ` Kees Cook
2021-08-31 4:31 ` Guenter Roeck
2021-08-27 16:30 ` [PATCH v3 5/5] Makefile: Enable -Wzero-length-bounds Kees Cook
2021-08-30 18:44 ` [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Nathan Chancellor
5 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2021-08-27 16:30 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Arnd Bergmann, Masahiro Yamada, linux-kbuild,
Gustavo A . R . Silva, Rasmus Villemoes, Keith Packard,
Dan Williams, Daniel Vetter, clang-built-linux, linux-hardening
With the recent fixes for flexible arrays and expanded FORTIFY_SOURCE
coverage, it is now possible to enable -Warray-bounds. Since both
GCC and Clang include -Warray-bounds in -Wall, we just need to stop
disabling it.
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Co-developed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/Makefile b/Makefile
index e4f5895badb5..8e7e73a642e2 100644
--- a/Makefile
+++ b/Makefile
@@ -995,7 +995,6 @@ KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
# We'll want to enable this eventually, but it's not going away for 5.7 at least
KBUILD_CFLAGS += $(call cc-disable-warning, zero-length-bounds)
-KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
# Another good warning that we'll want to enable eventually
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 4/5] Makefile: Enable -Warray-bounds
2021-08-27 16:30 ` [PATCH v3 4/5] Makefile: Enable -Warray-bounds Kees Cook
@ 2021-08-31 4:31 ` Guenter Roeck
2021-08-31 17:46 ` Kees Cook
0 siblings, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2021-08-31 4:31 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Arnd Bergmann, Masahiro Yamada, linux-kbuild,
Gustavo A . R . Silva, Rasmus Villemoes, Keith Packard,
Dan Williams, Daniel Vetter, clang-built-linux, linux-hardening
On Fri, Aug 27, 2021 at 09:30:14AM -0700, Kees Cook wrote:
> With the recent fixes for flexible arrays and expanded FORTIFY_SOURCE
> coverage, it is now possible to enable -Warray-bounds. Since both
> GCC and Clang include -Warray-bounds in -Wall, we just need to stop
> disabling it.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: linux-kbuild@vger.kernel.org
> Co-developed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index e4f5895badb5..8e7e73a642e2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -995,7 +995,6 @@ KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
>
> # We'll want to enable this eventually, but it's not going away for 5.7 at least
> KBUILD_CFLAGS += $(call cc-disable-warning, zero-length-bounds)
> -KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
> KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
>
This patch causes 'alpha' builds to fail when trying to build an image with
gcc 11.2.
In file included from include/linux/string.h:20,
from include/linux/bitmap.h:11,
from include/linux/cpumask.h:12,
from include/linux/smp.h:13,
from include/linux/lockdep.h:14,
from include/linux/spinlock.h:63,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from include/linux/pagemap.h:8,
from arch/alpha/mm/init.c:10:
In function '__memset',
inlined from '__bad_pagetable' at arch/alpha/mm/init.c:79:2:
arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
37 | return __builtin_memset(s, c, n);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
In function '__memset',
inlined from '__bad_page' at arch/alpha/mm/init.c:86:2:
arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
37 | return __builtin_memset(s, c, n);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
In function '__memset',
inlined from 'paging_init' at arch/alpha/mm/init.c:256:2:
arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
37 | return __builtin_memset(s, c, n);
Seen in next-20210830.
Guenter
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 4/5] Makefile: Enable -Warray-bounds
2021-08-31 4:31 ` Guenter Roeck
@ 2021-08-31 17:46 ` Kees Cook
2021-08-31 19:40 ` Guenter Roeck
0 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2021-08-31 17:46 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-kernel, Arnd Bergmann, Masahiro Yamada, linux-kbuild,
Gustavo A . R . Silva, Rasmus Villemoes, Keith Packard,
Dan Williams, Daniel Vetter, clang-built-linux, linux-hardening
On Mon, Aug 30, 2021 at 09:31:28PM -0700, Guenter Roeck wrote:
> On Fri, Aug 27, 2021 at 09:30:14AM -0700, Kees Cook wrote:
> > With the recent fixes for flexible arrays and expanded FORTIFY_SOURCE
> > coverage, it is now possible to enable -Warray-bounds. Since both
> > GCC and Clang include -Warray-bounds in -Wall, we just need to stop
> > disabling it.
> >
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Cc: linux-kbuild@vger.kernel.org
> > Co-developed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > Makefile | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index e4f5895badb5..8e7e73a642e2 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -995,7 +995,6 @@ KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
> >
> > # We'll want to enable this eventually, but it's not going away for 5.7 at least
> > KBUILD_CFLAGS += $(call cc-disable-warning, zero-length-bounds)
> > -KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
> > KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
> >
>
> This patch causes 'alpha' builds to fail when trying to build an image with
> gcc 11.2.
>
> In file included from include/linux/string.h:20,
> from include/linux/bitmap.h:11,
> from include/linux/cpumask.h:12,
> from include/linux/smp.h:13,
> from include/linux/lockdep.h:14,
> from include/linux/spinlock.h:63,
> from include/linux/mmzone.h:8,
> from include/linux/gfp.h:6,
> from include/linux/mm.h:10,
> from include/linux/pagemap.h:8,
> from arch/alpha/mm/init.c:10:
> In function '__memset',
> inlined from '__bad_pagetable' at arch/alpha/mm/init.c:79:2:
> arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
> 37 | return __builtin_memset(s, c, n);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> In function '__memset',
> inlined from '__bad_page' at arch/alpha/mm/init.c:86:2:
> arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
> 37 | return __builtin_memset(s, c, n);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> In function '__memset',
> inlined from 'paging_init' at arch/alpha/mm/init.c:256:2:
> arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
> 37 | return __builtin_memset(s, c, n);
>
> Seen in next-20210830.
Ah-ha, thanks for the report! I didn't see this in my builds -- what
config target did you use for this?
Looks like GCC isn't happy about an unsigned long->void * conversion
here...
include/asm/page.h:#define PAGE_OFFSET 0xfffffc0000000000UL
...
include/uapi/asm/setup.h:#define KERNEL_START_PHYS 0x1000000 /* required: Wildfire/Titan/Marvel */
include/uapi/asm/setup.h:#define EMPTY_PGT (PAGE_OFFSET+KERNEL_START_PHYS+0x04000)
...
mm/init.c: memset((void *) EMPTY_PGT, 0, PAGE_SIZE);
I'll try to figure out the right annotations to fix this...
--
Kees Cook
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/5] Makefile: Enable -Warray-bounds
2021-08-31 17:46 ` Kees Cook
@ 2021-08-31 19:40 ` Guenter Roeck
2021-08-31 20:18 ` Kees Cook
0 siblings, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2021-08-31 19:40 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Arnd Bergmann, Masahiro Yamada, linux-kbuild,
Gustavo A . R . Silva, Rasmus Villemoes, Keith Packard,
Dan Williams, Daniel Vetter, clang-built-linux, linux-hardening
Hi Kees,
On 8/31/21 10:46 AM, Kees Cook wrote:
> On Mon, Aug 30, 2021 at 09:31:28PM -0700, Guenter Roeck wrote:
>> On Fri, Aug 27, 2021 at 09:30:14AM -0700, Kees Cook wrote:
>>> With the recent fixes for flexible arrays and expanded FORTIFY_SOURCE
>>> coverage, it is now possible to enable -Warray-bounds. Since both
>>> GCC and Clang include -Warray-bounds in -Wall, we just need to stop
>>> disabling it.
>>>
>>> Cc: Arnd Bergmann <arnd@arndb.de>
>>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>>> Cc: linux-kbuild@vger.kernel.org
>>> Co-developed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>> Makefile | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index e4f5895badb5..8e7e73a642e2 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -995,7 +995,6 @@ KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
>>>
>>> # We'll want to enable this eventually, but it's not going away for 5.7 at least
>>> KBUILD_CFLAGS += $(call cc-disable-warning, zero-length-bounds)
>>> -KBUILD_CFLAGS += $(call cc-disable-warning, array-bounds)
>>> KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
>>>
>>
>> This patch causes 'alpha' builds to fail when trying to build an image with
>> gcc 11.2.
>>
>> In file included from include/linux/string.h:20,
>> from include/linux/bitmap.h:11,
>> from include/linux/cpumask.h:12,
>> from include/linux/smp.h:13,
>> from include/linux/lockdep.h:14,
>> from include/linux/spinlock.h:63,
>> from include/linux/mmzone.h:8,
>> from include/linux/gfp.h:6,
>> from include/linux/mm.h:10,
>> from include/linux/pagemap.h:8,
>> from arch/alpha/mm/init.c:10:
>> In function '__memset',
>> inlined from '__bad_pagetable' at arch/alpha/mm/init.c:79:2:
>> arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
>> 37 | return __builtin_memset(s, c, n);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>> In function '__memset',
>> inlined from '__bad_page' at arch/alpha/mm/init.c:86:2:
>> arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
>> 37 | return __builtin_memset(s, c, n);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~
>> In function '__memset',
>> inlined from 'paging_init' at arch/alpha/mm/init.c:256:2:
>> arch/alpha/include/asm/string.h:37:32: error: '__builtin_memset' offset [0, 8191] is out of the bounds [0, 0] [-Werror=array-bounds]
>> 37 | return __builtin_memset(s, c, n);
>>
>> Seen in next-20210830.
>
> Ah-ha, thanks for the report! I didn't see this in my builds -- what
> config target did you use for this?
>
The configuration doesn't matter; it fails for me for all configurations,
including defconfig, alllnoconfig, and allmodconfig.
Key is to use gcc 11.2. The image builds just fine with gcc 9.4 and 10.3.
Guenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/5] Makefile: Enable -Warray-bounds
2021-08-31 19:40 ` Guenter Roeck
@ 2021-08-31 20:18 ` Kees Cook
2021-08-31 20:49 ` Guenter Roeck
0 siblings, 1 reply; 16+ messages in thread
From: Kees Cook @ 2021-08-31 20:18 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-kernel, Arnd Bergmann, Masahiro Yamada, linux-kbuild,
Gustavo A . R . Silva, Rasmus Villemoes, Keith Packard,
Dan Williams, Daniel Vetter, clang-built-linux, linux-hardening
On Tue, Aug 31, 2021 at 12:40:53PM -0700, Guenter Roeck wrote:
> The configuration doesn't matter; it fails for me for all configurations,
> including defconfig, alllnoconfig, and allmodconfig.
> Key is to use gcc 11.2. The image builds just fine with gcc 9.4 and 10.3.
Ah! Yes, heh. That would be my problem; I've got 10.3 for my builders.
Thanks! I will give 11.2 a spin.
--
Kees Cook
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/5] Makefile: Enable -Warray-bounds
2021-08-31 20:18 ` Kees Cook
@ 2021-08-31 20:49 ` Guenter Roeck
0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2021-08-31 20:49 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Arnd Bergmann, Masahiro Yamada, linux-kbuild,
Gustavo A . R . Silva, Rasmus Villemoes, Keith Packard,
Dan Williams, Daniel Vetter, clang-built-linux, linux-hardening
On 8/31/21 1:18 PM, Kees Cook wrote:
> On Tue, Aug 31, 2021 at 12:40:53PM -0700, Guenter Roeck wrote:
>> The configuration doesn't matter; it fails for me for all configurations,
>> including defconfig, alllnoconfig, and allmodconfig.
>> Key is to use gcc 11.2. The image builds just fine with gcc 9.4 and 10.3.
>
> Ah! Yes, heh. That would be my problem; I've got 10.3 for my builders.
> Thanks! I will give 11.2 a spin.
>
In case you don't have gcc 11.2 available - I tested with 11.1 as well;
that also fails.
Guenter
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 5/5] Makefile: Enable -Wzero-length-bounds
2021-08-27 16:30 [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Kees Cook
` (3 preceding siblings ...)
2021-08-27 16:30 ` [PATCH v3 4/5] Makefile: Enable -Warray-bounds Kees Cook
@ 2021-08-27 16:30 ` Kees Cook
2021-08-30 18:44 ` [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Nathan Chancellor
5 siblings, 0 replies; 16+ messages in thread
From: Kees Cook @ 2021-08-27 16:30 UTC (permalink / raw)
To: linux-kernel
Cc: Kees Cook, Gustavo A. R. Silva, Arnd Bergmann, Masahiro Yamada,
linux-kbuild, Rasmus Villemoes, Keith Packard, Dan Williams,
Daniel Vetter, clang-built-linux, linux-hardening
With all known internal zero-length accesses fixed, it is possible to
enable -Wzero-length-bounds globally. Since this is included by default
in -Warray-bounds, we just need to stop disabling it.
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/Makefile b/Makefile
index 8e7e73a642e2..8e732e875e78 100644
--- a/Makefile
+++ b/Makefile
@@ -994,7 +994,6 @@ KBUILD_CFLAGS += -Wno-pointer-sign
KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation)
# We'll want to enable this eventually, but it's not going away for 5.7 at least
-KBUILD_CFLAGS += $(call cc-disable-warning, zero-length-bounds)
KBUILD_CFLAGS += $(call cc-disable-warning, stringop-overflow)
# Another good warning that we'll want to enable eventually
--
2.30.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds
2021-08-27 16:30 [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Kees Cook
` (4 preceding siblings ...)
2021-08-27 16:30 ` [PATCH v3 5/5] Makefile: Enable -Wzero-length-bounds Kees Cook
@ 2021-08-30 18:44 ` Nathan Chancellor
2021-08-30 20:12 ` Kees Cook
2021-08-30 20:16 ` Kees Cook
5 siblings, 2 replies; 16+ messages in thread
From: Nathan Chancellor @ 2021-08-30 18:44 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Arnd Bergmann, Gustavo A. R. Silva,
Rasmus Villemoes, Keith Packard, Dan Williams, Daniel Vetter,
clang-built-linux, linux-hardening, llvm
On Fri, Aug 27, 2021 at 09:30:10AM -0700, Kees Cook wrote:
> v3:
> - fix typo in treewide conversion (u8 should have been __u8)
> - improve changelog for DECLARE_FLEX_ARRAY patch
> - add acks/reviews
> v2: https://lore.kernel.org/lkml/20210826050458.1540622-1-keescook@chromium.org/
> v1: https://lore.kernel.org/lkml/20210818081118.1667663-1-keescook@chromium.org/
>
> Hi,
>
> In support of the improved buffer overflow detection for memcpy(),
> this enables -Warray-bounds and -Wzero-length-bounds globally. Mostly
> it involves some struct member tricks with the new DECLARE_FLEX_ARRAY()
> macro. Everything else is just replacing stacked 0-element arrays
> with actual unions in two related treewide patches. There is one set of
> special cases that were fixed separately[1] and are needed as well.
>
> I'm expecting to carry this series with the memcpy() series in my
> "overflow" tree. Reviews appreciated! :)
Hi Kees,
I ran this series through my local build tests and uncovered two
warnings in the same file that appear to be unhandled as of
next-20210830. This is from ARCH=powerpc pseries_defconfig with
clang-14, I did not try earlier versions of clang.
arch/powerpc/kernel/signal_32.c:780:2: error: array index 3 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds]
unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_32.c:85:29: note: expanded from macro 'unsafe_put_sigset_t'
#define unsafe_put_sigset_t unsafe_put_compat_sigset
^
./include/linux/compat.h:455:19: note: expanded from macro 'unsafe_put_compat_sigset'
unsafe_put_user(__s->sig[3] >> 32, &__c->sig[7], label); \
^ ~
./arch/powerpc/include/asm/uaccess.h:426:42: note: expanded from macro 'unsafe_put_user'
__put_user_size_goto((__typeof__(*(p)))(x), (p), sizeof(*(p)), e)
^
./arch/powerpc/include/asm/uaccess.h:114:30: note: expanded from macro '__put_user_size_goto'
case 1: __put_user_asm_goto(x, __pus_addr, label, "stb"); break; \
^
./arch/powerpc/include/asm/uaccess.h:89:10: note: expanded from macro '__put_user_asm_goto'
: "r" (x), "m"UPD_CONSTR (*addr) \
^
./include/linux/compiler_types.h:254:42: note: expanded from macro 'asm_volatile_goto'
#define asm_volatile_goto(x...) asm goto(x)
^
./arch/powerpc/include/uapi/asm/signal.h:18:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
arch/powerpc/kernel/signal_32.c:1044:3: error: array index 2 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds]
unsafe_put_sigset_t(&old_ctx->uc_sigmask, ¤t->blocked, failed);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_32.c:85:29: note: expanded from macro 'unsafe_put_sigset_t'
#define unsafe_put_sigset_t unsafe_put_compat_sigset
^
./include/linux/compat.h:459:19: note: expanded from macro 'unsafe_put_compat_sigset'
unsafe_put_user(__s->sig[2] >> 32, &__c->sig[5], label); \
^ ~
./arch/powerpc/include/asm/uaccess.h:426:42: note: expanded from macro 'unsafe_put_user'
__put_user_size_goto((__typeof__(*(p)))(x), (p), sizeof(*(p)), e)
^
./arch/powerpc/include/asm/uaccess.h:116:30: note: expanded from macro '__put_user_size_goto'
case 4: __put_user_asm_goto(x, __pus_addr, label, "stw"); break; \
^
./arch/powerpc/include/asm/uaccess.h:89:10: note: expanded from macro '__put_user_asm_goto'
: "r" (x), "m"UPD_CONSTR (*addr) \
^
./include/linux/compiler_types.h:254:42: note: expanded from macro 'asm_volatile_goto'
#define asm_volatile_goto(x...) asm goto(x)
^
./arch/powerpc/include/uapi/asm/signal.h:18:2: note: array 'sig' declared here
unsigned long sig[_NSIG_WORDS];
^
Cheers,
Nathan
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds
2021-08-30 18:44 ` [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Nathan Chancellor
@ 2021-08-30 20:12 ` Kees Cook
2021-08-30 20:16 ` Kees Cook
1 sibling, 0 replies; 16+ messages in thread
From: Kees Cook @ 2021-08-30 20:12 UTC (permalink / raw)
To: Nathan Chancellor
Cc: linux-kernel, Arnd Bergmann, Gustavo A. R. Silva,
Rasmus Villemoes, Keith Packard, Dan Williams, Daniel Vetter,
clang-built-linux, linux-hardening, llvm
On Mon, Aug 30, 2021 at 11:44:54AM -0700, Nathan Chancellor wrote:
> On Fri, Aug 27, 2021 at 09:30:10AM -0700, Kees Cook wrote:
> > v3:
> > - fix typo in treewide conversion (u8 should have been __u8)
> > - improve changelog for DECLARE_FLEX_ARRAY patch
> > - add acks/reviews
> > v2: https://lore.kernel.org/lkml/20210826050458.1540622-1-keescook@chromium.org/
> > v1: https://lore.kernel.org/lkml/20210818081118.1667663-1-keescook@chromium.org/
> >
> > Hi,
> >
> > In support of the improved buffer overflow detection for memcpy(),
> > this enables -Warray-bounds and -Wzero-length-bounds globally. Mostly
> > it involves some struct member tricks with the new DECLARE_FLEX_ARRAY()
> > macro. Everything else is just replacing stacked 0-element arrays
> > with actual unions in two related treewide patches. There is one set of
> > special cases that were fixed separately[1] and are needed as well.
> >
> > I'm expecting to carry this series with the memcpy() series in my
> > "overflow" tree. Reviews appreciated! :)
>
> Hi Kees,
>
> I ran this series through my local build tests and uncovered two
> warnings in the same file that appear to be unhandled as of
> next-20210830. This is from ARCH=powerpc pseries_defconfig with
> clang-14, I did not try earlier versions of clang.
Thanks for double-checking!
>
> arch/powerpc/kernel/signal_32.c:780:2: error: array index 3 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds]
> unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> [...]
> arch/powerpc/kernel/signal_32.c:1044:3: error: array index 2 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds]
> unsafe_put_sigset_t(&old_ctx->uc_sigmask, ¤t->blocked, failed);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This smells like some kind of casting issue. uc_sigmask has only a single
unsigned long element but unsafe_put_compat_sigset() seems to be doing
stuff with [3], etc. Is it expecting u8? I will keep looking...
--
Kees Cook
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds
2021-08-30 18:44 ` [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds Nathan Chancellor
2021-08-30 20:12 ` Kees Cook
@ 2021-08-30 20:16 ` Kees Cook
2021-08-30 22:34 ` Nathan Chancellor
1 sibling, 1 reply; 16+ messages in thread
From: Kees Cook @ 2021-08-30 20:16 UTC (permalink / raw)
To: Nathan Chancellor
Cc: linux-kernel, Arnd Bergmann, Gustavo A. R. Silva,
Rasmus Villemoes, Keith Packard, Dan Williams, Daniel Vetter,
clang-built-linux, linux-hardening, llvm
On Mon, Aug 30, 2021 at 11:44:54AM -0700, Nathan Chancellor wrote:
> arch/powerpc/kernel/signal_32.c:780:2: error: array index 3 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds]
> unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Or is this a Clang DCE failure?
#define unsafe_put_compat_sigset(compat, set, label) do { \
compat_sigset_t __user *__c = compat; \
const sigset_t *__s = set; \
\
switch (_NSIG_WORDS) { \
case 4: \
unsafe_put_user(__s->sig[3] >> 32, &__c->sig[7], label); \
unsafe_put_user(__s->sig[3], &__c->sig[6], label); \
fallthrough; \
case 3: \
unsafe_put_user(__s->sig[2] >> 32, &__c->sig[5], label); \
unsafe_put_user(__s->sig[2], &__c->sig[4], label); \
fallthrough; \
case 2: \
unsafe_put_user(__s->sig[1] >> 32, &__c->sig[3], label); \
unsafe_put_user(__s->sig[1], &__c->sig[2], label); \
fallthrough; \
case 1: \
unsafe_put_user(__s->sig[0] >> 32, &__c->sig[1], label); \
unsafe_put_user(__s->sig[0], &__c->sig[0], label); \
} \
} while (0)
if "set" has only 1 element, then _NSIG_WORDS must be 1. The warnings
are coming from cases 4 and 3. (But why not 2, which would also access
beyond the end?)
--
Kees Cook
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 0/5] Enable -Warray-bounds and -Wzero-length-bounds
2021-08-30 20:16 ` Kees Cook
@ 2021-08-30 22:34 ` Nathan Chancellor
0 siblings, 0 replies; 16+ messages in thread
From: Nathan Chancellor @ 2021-08-30 22:34 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Arnd Bergmann, Gustavo A. R. Silva,
Rasmus Villemoes, Keith Packard, Dan Williams, Daniel Vetter,
clang-built-linux, linux-hardening, llvm
On Mon, Aug 30, 2021 at 01:16:41PM -0700, Kees Cook wrote:
> On Mon, Aug 30, 2021 at 11:44:54AM -0700, Nathan Chancellor wrote:
> > arch/powerpc/kernel/signal_32.c:780:2: error: array index 3 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds]
> > unsafe_put_sigset_t(&frame->uc.uc_sigmask, oldset, failed);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Or is this a Clang DCE failure?
>
> #define unsafe_put_compat_sigset(compat, set, label) do { \
> compat_sigset_t __user *__c = compat; \
> const sigset_t *__s = set; \
> \
> switch (_NSIG_WORDS) { \
> case 4: \
> unsafe_put_user(__s->sig[3] >> 32, &__c->sig[7], label); \
> unsafe_put_user(__s->sig[3], &__c->sig[6], label); \
> fallthrough; \
> case 3: \
> unsafe_put_user(__s->sig[2] >> 32, &__c->sig[5], label); \
> unsafe_put_user(__s->sig[2], &__c->sig[4], label); \
> fallthrough; \
> case 2: \
> unsafe_put_user(__s->sig[1] >> 32, &__c->sig[3], label); \
> unsafe_put_user(__s->sig[1], &__c->sig[2], label); \
> fallthrough; \
> case 1: \
> unsafe_put_user(__s->sig[0] >> 32, &__c->sig[1], label); \
> unsafe_put_user(__s->sig[0], &__c->sig[0], label); \
> } \
> } while (0)
>
> if "set" has only 1 element, then _NSIG_WORDS must be 1. The warnings
> are coming from cases 4 and 3. (But why not 2, which would also access
> beyond the end?)
I trimmed the warnings down otherwise it would have been 400 lines long
:) it did warn for the 2 case.
Clang does not like the use of asm goto in unsafe_put_user on powerpc it
seems:
$ cat warray-bounds.c
#define NSIG_WORDS 1
typedef struct {
unsigned long sig[NSIG_WORDS];
} sigset_t;
int handle_rt_signal32_bad(sigset_t *);
int handle_rt_signal32_bad(sigset_t *oldset)
{
switch (NSIG_WORDS) {
case 4:
__asm__ goto("" : : "r"(oldset->sig[3] >> 32) : : failed);
__asm__ goto("" : : "r"(oldset->sig[3]) : : failed);
__attribute__((fallthrough));
case 3:
__asm__ goto("" : : "r"(oldset->sig[2] >> 32) : : failed);
__asm__ goto("" : : "r"(oldset->sig[2]) : : failed);
__attribute__((fallthrough));
case 2:
__asm__ goto("" : : "r"(oldset->sig[1] >> 32) : : failed);
__asm__ goto("" : : "r"(oldset->sig[1]) : : failed);
__attribute__((fallthrough));
case 1:
__asm__ goto("" : : "r"(oldset->sig[0] >> 32) : : failed);
__asm__ goto("" : : "r"(oldset->sig[0]) : : failed);
}
return 0;
failed:
return 1;
}
void normal_array_access(unsigned long);
int handle_rt_signal32_good(sigset_t *);
int handle_rt_signal32_good(sigset_t *oldset)
{
switch (NSIG_WORDS) {
case 4:
normal_array_access(oldset->sig[3] >> 32);
normal_array_access(oldset->sig[3]);
__attribute__((fallthrough));
case 3:
normal_array_access(oldset->sig[2] >> 32);
normal_array_access(oldset->sig[2]);
__attribute__((fallthrough));
case 2:
normal_array_access(oldset->sig[1] >> 32);
normal_array_access(oldset->sig[1]);
__attribute__((fallthrough));
case 1:
normal_array_access(oldset->sig[0] >> 32);
normal_array_access(oldset->sig[0]);
}
return 0;
}
$ clang -fsyntax-only -Weverything warray-bounds.c
warray-bounds.c:12:27: warning: array index 3 is past the end of the array (which contains 1 element) [-Warray-bounds]
__asm__ goto("" : : "r"(oldset->sig[3] >> 32) : : failed);
^ ~
warray-bounds.c:4:2: note: array 'sig' declared here
unsigned long sig[NSIG_WORDS];
^
warray-bounds.c:16:27: warning: array index 2 is past the end of the array (which contains 1 element) [-Warray-bounds]
__asm__ goto("" : : "r"(oldset->sig[2] >> 32) : : failed);
^ ~
warray-bounds.c:4:2: note: array 'sig' declared here
unsigned long sig[NSIG_WORDS];
^
warray-bounds.c:20:27: warning: array index 1 is past the end of the array (which contains 1 element) [-Warray-bounds]
__asm__ goto("" : : "r"(oldset->sig[1] >> 32) : : failed);
^ ~
warray-bounds.c:4:2: note: array 'sig' declared here
unsigned long sig[NSIG_WORDS];
^
3 warnings generated.
$ gcc -fsyntax-only -Wall -Wextra -Wpedantic warray-bounds.c
godbolt link: https://godbolt.org/z/8xYojs1WY
I've reported this on LLVM's bug tracker to see what the clang
developers can do with you on CC:
https://bugs.llvm.org/show_bug.cgi?id=51682
Cheers,
Nathan
^ permalink raw reply [flat|nested] 16+ messages in thread