overflow: Change DEFINE_FLEX to take __counted_by member
Subsystems (5)
PATCH v2 1/1 overflow: Change DEFINE_FLEX to take __counted_by member
The norm should be flexible array structures with __counted_by annotations, so DEFINE_FLEX() is updated to expect that. Rename the non-annotated version to DEFINE_RAW_FLEX(), and update the few existing users.
---
Cc: Przemek Kitszel <[email protected]>
Cc: Jesse Brandeburg <[email protected]>
Cc: Tony Nguyen <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: "Gustavo A. R. Silva" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
v2: swap member/counter args
v1: https://lore.kernel.org/lkml/[email protected]/
--- drivers/net/ethernet/intel/ice/ice_common.c | 4 +-- drivers/net/ethernet/intel/ice/ice_ddp.c | 8 +++--- drivers/net/ethernet/intel/ice/ice_lag.c | 6 ++--- drivers/net/ethernet/intel/ice/ice_lib.c | 4 +-- drivers/net/ethernet/intel/ice/ice_sched.c | 4 +-- drivers/net/ethernet/intel/ice/ice_switch.c | 10 ++++---- drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +- include/linux/overflow.h | 27 +++++++++++++++++---- lib/overflow_kunit.c | 19 +++++++++++++++ 9 files changed, 60 insertions(+), 24 deletions(-)
drivers/net/ethernet/intel/ice/ice_common.c +2 -2
@@ -4700,7 +4700,7 @@ ice_dis_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u8 num_queues, enum ice_disq_rst_src rst_src, u16 vmvf_num, struct ice_sq_cd *cd) { - DEFINE_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1); + DEFINE_RAW_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1);u16 i, buf_size =__struct_size (qg_list);struct ice_q_ctx *q_ctx;int status = -ENOENT ;
@@ -4922,7 +4922,7 @@ int ice_dis_vsi_rdma_qset(struct ice_port_info *pi, u16 count, u32 *qset_teid, u16 *q_id) { - DEFINE_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1); + DEFINE_RAW_FLEX(struct ice_aqc_dis_txq_item, qg_list, q_id, 1);u16 qg_size =__struct_size (qg_list);struct ice_hw *hw;int status =0 ;
drivers/net/ethernet/intel/ice/ice_ddp.c +4 -4
@@ -1934,8 +1934,8 @@ static enum ice_ddp_state ice_init_pkg_info(struct ice_hw *hw,*/ static enum ice_ddp_state ice_get_pkg_info (struct ice_hw *hw) { - DEFINE_FLEX(struct ice_aqc_get_pkg_info_resp, pkg_info, pkg_info, - ICE_PKG_CNT); + DEFINE_RAW_FLEX(struct ice_aqc_get_pkg_info_resp, pkg_info, pkg_info, + ICE_PKG_CNT);u16 size =__struct_size (pkg_info);u32 i;
@@ -1986,8 +1986,8 @@ static enum ice_ddp_state ice_chk_pkg_compat(struct ice_hw *hw,struct ice_pkg_hdr *ospkg, struct ice_seg **seg) { -DEFINE_FLEX (struct ice_aqc_get_pkg_info_resp ,pkg ,pkg_info , -ICE_PKG_CNT ); +DEFINE_RAW_FLEX (struct ice_aqc_get_pkg_info_resp ,pkg ,pkg_info , +ICE_PKG_CNT );u16 size =__struct_size (pkg);enum ice_ddp_state state;u32 i;
drivers/net/ethernet/intel/ice/ice_lag.c +3 -3
@@ -470,7 +470,7 @@ static void ice_lag_move_vf_node_tc(struct ice_lag *lag, u8 oldport, u8 newport, u16 vsi_num, u8 tc) { - DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1); + DEFINE_RAW_FLEX(struct ice_aqc_move_elem, buf, teid, 1);struct device *dev =ice_pf_to_dev (lag->pf);u16 numq, valq, num_moved, qbuf_size;u16 buf_size =__struct_size (buf);
@@ -828,7 +828,7 @@ static void ice_lag_reclaim_vf_tc(struct ice_lag *lag, struct ice_hw *src_hw, u16 vsi_num, u8 tc) { - DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1); + DEFINE_RAW_FLEX(struct ice_aqc_move_elem, buf, teid, 1);struct device *dev =ice_pf_to_dev (lag->pf);u16 numq, valq, num_moved, qbuf_size;u16 buf_size =__struct_size (buf);
@@ -1852,7 +1852,7 @@ static void ice_lag_move_vf_nodes_tc_sync(struct ice_lag *lag, struct ice_hw *dest_hw, u16 vsi_num, u8 tc) { - DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1); + DEFINE_RAW_FLEX(struct ice_aqc_move_elem, buf, teid, 1);struct device *dev =ice_pf_to_dev (lag->pf);u16 numq, valq, num_moved, qbuf_size;u16 buf_size =__struct_size (buf);
drivers/net/ethernet/intel/ice/ice_lib.c +2 -2
@@ -1805,7 +1805,7 @@ int ice_vsi_cfg_single_rxq(struct ice_vsi *vsi, u16 q_idx)int ice_vsi_cfg_single_txq (struct ice_vsi *vsi,struct ice_tx_ring **tx_rings,u16 q_idx) { - DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1); + DEFINE_RAW_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);if (q_idx >= vsi->alloc_txq || !tx_rings || !tx_rings[q_idx])return -EINVAL ;
@@ -1854,7 +1854,7 @@ int ice_vsi_cfg_rxqs(struct ice_vsi *vsi)static int ice_vsi_cfg_txqs (struct ice_vsi *vsi,struct ice_tx_ring **rings,u16 count) { - DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1); + DEFINE_RAW_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);int err =0 ;u16 q_idx;
drivers/net/ethernet/intel/ice/ice_sched.c +2 -2
@@ -237,7 +237,7 @@ static int ice_sched_remove_elems(struct ice_hw *hw, struct ice_sched_node *parent, u32 node_teid) { - DEFINE_FLEX(struct ice_aqc_delete_elem, buf, teid, 1); + DEFINE_RAW_FLEX(struct ice_aqc_delete_elem, buf, teid, 1);u16 buf_size =__struct_size (buf);u16 num_groups_removed =0 ;int status;
@@ -2219,7 +2219,7 @@ int ice_sched_move_nodes(struct ice_port_info *pi, struct ice_sched_node *parent, u16 num_items, u32 *list) { - DEFINE_FLEX(struct ice_aqc_move_elem, buf, teid, 1); + DEFINE_RAW_FLEX(struct ice_aqc_move_elem, buf, teid, 1);u16 buf_len =__struct_size (buf);struct ice_sched_node *node;u16 i, grps_movd =0 ;
drivers/net/ethernet/intel/ice/ice_switch.c +5 -5
@@ -1812,7 +1812,7 @@ ice_aq_alloc_free_vsi_list(struct ice_hw *hw, u16 *vsi_list_id,enum ice_sw_lkup_type lkup_type, enum ice_adminq_opc opc) { - DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1); + DEFINE_RAW_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);u16 buf_len =__struct_size (sw_buf);struct ice_aqc_res_elem *vsi_ele;int status;
@@ -2081,7 +2081,7 @@ ice_aq_get_recipe_to_profile(struct ice_hw *hw, u32 profile_id, u8 *r_bitmap,*/ int ice_alloc_recipe (struct ice_hw *hw,u16 *rid) { - DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1); + DEFINE_RAW_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);u16 buf_len =__struct_size (sw_buf);int status;
@@ -4418,7 +4418,7 @@ int ice_alloc_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items, u16 *counter_id) { - DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1); + DEFINE_RAW_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);u16 buf_len =__struct_size (buf);int status;
@@ -4446,7 +4446,7 @@ int ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items, u16 counter_id) { - DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1); + DEFINE_RAW_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);u16 buf_len =__struct_size (buf);int status;
@@ -4476,7 +4476,7 @@ ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,*/ int ice_share_res (struct ice_hw *hw,u16 type,u8 shared,u16 res_id) { - DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1); + DEFINE_RAW_FLEX(struct ice_aqc_alloc_free_res_elem, buf, elem, 1);u16 buf_len =__struct_size (buf);u16 res_type;int status;
drivers/net/ethernet/intel/ice/ice_xsk.c +1 -1
@@ -217,7 +217,7 @@ static int ice_qp_dis(struct ice_vsi *vsi, u16 q_idx)*/ static int ice_qp_ena (struct ice_vsi *vsi,u16 q_idx) { - DEFINE_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1); + DEFINE_RAW_FLEX(struct ice_aqc_add_tx_qgrp, qg_buf, txqs, 1);u16 size =__struct_size (qg_buf);struct ice_q_vector *q_vector;struct ice_tx_ring *tx_ring;
include/linux/overflow.h +22 -5
@@ -396,9 +396,9 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @name: Name for a variable to define. * @member: Name of the array member. * @count: Number of elements in the array; must be compile-time const. -* @initializer: initializer expression (could be empty for no init). +* @initializer...: initializer expression (could be empty for no init). */ -#define _DEFINE_FLEX (type, name, member, count, initializer) \ +#define _DEFINE_FLEX(type, name, member, count, initializer...) \ _Static_assert(__builtin_constant_p(count), \ "onstack flex array members require compile-time const count"); \ union { \
@@ -408,8 +408,8 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) type *name = (type *)&name##_u/** -* DEFINE_FLEX() - Define an on-stack instance of structure with a trailing -* flexible array member. +* DEFINE_RAW_FLEX() - Define an on-stack instance of structure with a trailing +* flexible array member, when it does not have a __counted_by annotation. * * @type: structure type name, including "struct" keyword. * @name: Name for a variable to define.
@@ -420,7 +420,24 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* flexible array member. * Use __struct_size(@name) to get compile-time size of it afterwards. */ -#define DEFINE_FLEX (type, name, member, count) \ +#define DEFINE_RAW_FLEX (type, name, member, count) \ _DEFINE_FLEX(type, name, member, count, = {}) +/** +* DEFINE_FLEX() - Define an on-stack instance of structure with a trailing +* flexible array member. +* +* @TYPE: structure type name, including "struct" keyword. +* @NAME: Name for a variable to define. +* @MEMBER: Name of the array member. +* @COUNTER: Name of the __counted_by member. +* @COUNT: Number of elements in the array; must be compile-time const. +* +* Define a zeroed, on-stack, instance of @TYPE structure with a trailing +* flexible array member. +* Use __struct_size(@NAME) to get compile-time size of it afterwards. +*/ +#define DEFINE_FLEX (TYPE ,NAME ,MEMBER ,COUNTER ,COUNT ) \ + _DEFINE_FLEX(TYPE, NAME, MEMBER, COUNT, = { .obj.COUNTER = COUNT, }) +#endif /* __LINUX_OVERFLOW_H */
lib/overflow_kunit.c +19 -0
@@ -1172,6 +1172,24 @@ static void castable_to_type_test(struct kunit *test)#undef TEST_CASTABLE_TO_TYPE } +struct foo { +int a; +u32 counter; + s16 array[] __counted_by(counter); +}; + +static void DEFINE_FLEX_test (struct kunit *test) +{ + DEFINE_RAW_FLEX(struct foo, two, array, 2); + DEFINE_FLEX(struct foo, eight, array, counter, 8); + DEFINE_FLEX(struct foo, empty, array, counter, 0); + +KUNIT_EXPECT_EQ (test,__struct_size (two), +sizeof (struct foo ) +sizeof (s16) +sizeof (s16)); +KUNIT_EXPECT_EQ (test,__struct_size (eight),24 ); +KUNIT_EXPECT_EQ (test,__struct_size (empty),sizeof (struct foo )); +} +static struct kunit_case overflow_test_cases[] = {KUNIT_CASE (u8_u8__u8_overflow_test),KUNIT_CASE (s8_s8__s8_overflow_test),
@@ -1194,6 +1212,7 @@ static struct kunit_case overflow_test_cases[] = {KUNIT_CASE (overflows_type_test),KUNIT_CASE (same_type_test),KUNIT_CASE (castable_to_type_test), + KUNIT_CASE(DEFINE_FLEX_test), {} };
Signature · 1 line
2.34.1
Quoted 6 lines · Kees Cook
The norm should be flexible array structures with
__counted_byannotations, soDEFINE_FLEX()is updated to expect that. Rename the non-annotated version toDEFINE_RAW_FLEX(), and update the few existing users.Signed-off-by Kees Cook <[email protected]>
LGTM:
Thanks!
Signature · 1 line
Gustavo
Quoted 20 lines · Kees Cook
The norm should be flexible array structures with
__counted_byannotations, soDEFINE_FLEX()is updated to expect that. Rename the non-annotated version toDEFINE_RAW_FLEX(), and update the few existing users.Signed-off-by Kees Cook <[email protected]>---
Cc: Przemek Kitszel <[email protected]>
Cc: Jesse Brandeburg <[email protected]>
Cc: Tony Nguyen <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: "Gustavo A. R. Silva" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
v2: swap member/counter args
v1: https://lore.kernel.org/lkml/[email protected]/
Quoted 12 lines
--- drivers/net/ethernet/intel/ice/ice_common.c | 4 +-- drivers/net/ethernet/intel/ice/ice_ddp.c | 8 +++--- drivers/net/ethernet/intel/ice/ice_lag.c | 6 ++--- drivers/net/ethernet/intel/ice/ice_lib.c | 4 +-- drivers/net/ethernet/intel/ice/ice_sched.c | 4 +-- drivers/net/ethernet/intel/ice/ice_switch.c | 10 ++++---- drivers/net/ethernet/intel/ice/ice_xsk.c | 2 +- include/linux/overflow.h | 27 +++++++++++++++++---- lib/overflow_kunit.c | 19 +++++++++++++++ 9 files changed, 60 insertions(+), 24 deletions(-)
Simon Horman
reviewerQuoted 6 lines · Kees Cook
The norm should be flexible array structures with
__counted_byannotations, soDEFINE_FLEX()is updated to expect that. Rename the non-annotated version toDEFINE_RAW_FLEX(), and update the few existing users.Signed-off-by Kees Cook <[email protected]>
Hi Kees,
I'm unclear what this is based on, as it doesn't appear to apply cleanly to net-next or the dev-queue branch of the iwl-next tree. But I manually applied it to the latter and ran some checks.
...
drivers/net/ethernet/intel/ice/ice_switch.c
Quoted 4 lines
drivers/net/ethernet/intel/ice/ice_switch.c +0
-0
Given what is currently in the dev-queue branch of the iwl-next tree, the following hunk also seems to be required for ice_switch.c.
@@ -5378,7 +5378,7 @@ ice_get_compat_fv_bitmap(struct ice_hw *hw, struct ice_adv_rule_info *rinfo,
*/
static int ice_subscribe_recipe(struct ice_hw *hw, u16 rid)
{
- DEFINE_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);
+ DEFINE_RAW_FLEX(struct ice_aqc_alloc_free_res_elem, sw_buf, elem, 1);
u16 buf_len = __struct_size(sw_buf);
u16 res_type;
int status;
...
include/linux/overflow.h static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) · line 399
Quoted 10 lines
include/linux/overflow.h +1
-1@@ -396,9 +396,9 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @name: Name for a variable to define. * @member: Name of the array member. * @count: Number of elements in the array; must be compile-time const. -* @initializer: initializer expression (could be empty for no init). +* @initializer...: initializer expression (could be empty for no init).
Curiously kernel-doc --none seems happier without the line above changed.
Quoted 6 lines
include/linux/overflow.h +1
-1*/ -#define _DEFINE_FLEX (type, name, member, count, initializer) \ +#define _DEFINE_FLEX(type, name, member, count, initializer...) \ _Static_assert(__builtin_constant_p(count), \ "onstack flex array members require compile-time const count"); \ union { \
...
Quoted 6 lines · Simon Horman
Quoted 5 lines · Kees Cook
The norm should be flexible array structures with
__counted_byannotations, soDEFINE_FLEX()is updated to expect that. Rename the non-annotated version toDEFINE_RAW_FLEX(), and update the few existing users.
Quoted 7 lines
Quoted 1 line
Signed-off-by Kees Cook <[email protected]>Hi Kees,
I'm unclear what this is based on, as it doesn't appear to apply cleanly to net-next or the dev-queue branch of the iwl-next tree. But I manually applied it to the latter and ran some checks.
It was based on v6.8-rc2, but it no longer applies cleanly to iwl-next:
https://lore.kernel.org/linux-next/[email protected]/
Is this something iwl-next can take for the v6.9 merge window? I can send a rebased patch if that helps?
Quoted 8 lines
include/linux/overflow.h static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) · line 399
Quoted 6 lines
include/linux/overflow.h +1
-1@@ -396,9 +396,9 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @name: Name for a variable to define. * @member: Name of the array member. * @count: Number of elements in the array; must be compile-time const. -* @initializer: initializer expression (could be empty for no init). +* @initializer...: initializer expression (could be empty for no init). Curiously kernel-doc --none seems happier without the line above changed.
I've fixed this up too:
https://lore.kernel.org/linux-next/202403071124.36DC2B617A@keescook/
Signature · 1 line
Kees Cook
Simon Horman
reviewerQuoted 15 lines · Kees Cook
Quoted 6 lines · Simon Horman
Quoted 5 lines · Kees Cook
The norm should be flexible array structures with
__counted_byannotations, soDEFINE_FLEX()is updated to expect that. Rename the non-annotated version toDEFINE_RAW_FLEX(), and update the few existing users.Quoted 7 lines
Quoted 1 line
Signed-off-by Kees Cook <[email protected]>Hi Kees,
I'm unclear what this is based on, as it doesn't appear to apply cleanly to net-next or the dev-queue branch of the iwl-next tree. But I manually applied it to the latter and ran some checks.
Quoted 5 lines
It was based on v6.8-rc2, but it no longer applies cleanly to iwl-next:
https://lore.kernel.org/linux-next/[email protected]/Is this something iwl-next can take for the v6.9 merge window? I can send a rebased patch if that helps?
Thanks Kees,
I think that would help in the sense that from my POV it would be more in fitting with the usual workflow for netdev patches.
But if the iwl maintainers think otherwise then I have no objections.
Quoted 16 lines
Quoted 9 lines
include/linux/overflow.h static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) · line 399
Quoted 7 lines
include/linux/overflow.h +1
-1@@ -396,9 +396,9 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @name: Name for a variable to define. * @member: Name of the array member. * @count: Number of elements in the array; must be compile-time const. -* @initializer: initializer expression (could be empty for no init). +* @initializer...: initializer expression (could be empty for no init). Curiously kernel-doc --none seems happier without the line above changed.
I've fixed this up too:
https://lore.kernel.org/linux-next/202403071124.36DC2B617A@keescook/Signature · 1 line
Kees Cook
Tony Nguyen
maintainerQuoted 22 lines · Simon Horman
Quoted 15 lines · Kees Cook
Quoted 6 lines · Simon Horman
Quoted 5 lines · Kees Cook
The norm should be flexible array structures with
__counted_byannotations, soDEFINE_FLEX()is updated to expect that. Rename the non-annotated version toDEFINE_RAW_FLEX(), and update the few existing users.Quoted 7 lines
Quoted 1 line
Signed-off-by Kees Cook <[email protected]>Hi Kees,
I'm unclear what this is based on, as it doesn't appear to apply cleanly to net-next or the dev-queue branch of the iwl-next tree. But I manually applied it to the latter and ran some checks.
Quoted 5 lines
It was based on v6.8-rc2, but it no longer applies cleanly to iwl-next:
https://lore.kernel.org/linux-next/[email protected]/Is this something iwl-next can take for the v6.9 merge window? I can send a rebased patch if that helps?
Quoted 6 lines
Thanks Kees,
I think that would help in the sense that from my POV it would be more in fitting with the usual workflow for netdev patches.
But if the iwl maintainers think otherwise then I have no objections.
I can take this through iwl-next. A rebase would be great and if you mark it for iwl-next ('PATCH iwl-next') so that everyone is clear on target tree. Just to note since net-next is now closed, it would be going to 6.10.
Thanks,
Tony
Quoted 16 lines
Quoted 16 lines
Quoted 9 lines
include/linux/overflow.h static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) · line 399
Quoted 7 lines
include/linux/overflow.h +1
-1@@ -396,9 +396,9 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)* @name: Name for a variable to define. * @member: Name of the array member. * @count: Number of elements in the array; must be compile-time const. -* @initializer: initializer expression (could be empty for no init). +* @initializer...: initializer expression (could be empty for no init). Curiously kernel-doc --none seems happier without the line above changed.
I've fixed this up too:
https://lore.kernel.org/linux-next/202403071124.36DC2B617A@keescook/Signature · 1 line
Kees Cook