2024-03-06 23:53:37

by Kees Cook

[permalink] [raw]
Subject: [PATCH v2] 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.

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]/
---
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(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 10c32cd80fff..ce50a322daa9 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -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;
diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c b/drivers/net/ethernet/intel/ice/ice_ddp.c
index 8b7504a9df31..03efb2521ca7 100644
--- a/drivers/net/ethernet/intel/ice/ice_ddp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
@@ -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;
diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c b/drivers/net/ethernet/intel/ice/ice_lag.c
index 2a25323105e5..da38edac1c42 100644
--- a/drivers/net/ethernet/intel/ice/ice_lag.c
+++ b/drivers/net/ethernet/intel/ice/ice_lag.c
@@ -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);
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 9be724291ef8..6819e95cec32 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -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;

diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c b/drivers/net/ethernet/intel/ice/ice_sched.c
index d174a4eeb899..a1525992d14b 100644
--- a/drivers/net/ethernet/intel/ice/ice_sched.c
+++ b/drivers/net/ethernet/intel/ice/ice_sched.c
@@ -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;
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index f84bab80ca42..d4baae8c3b72 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -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;
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 8b81a1677045..92ffa8de5171 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -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;
diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index aa691f2119b0..677b03c4c84f 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -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 */
diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c
index 65e8a72a83bf..4ef31b0bb74d 100644
--- a/lib/overflow_kunit.c
+++ b/lib/overflow_kunit.c
@@ -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),
{}
};

--
2.34.1



2024-03-07 00:41:19

by Gustavo A. R. Silva

[permalink] [raw]
Subject: Re: [PATCH v2] overflow: Change DEFINE_FLEX to take __counted_by member



On 3/6/24 17:51, Kees Cook wrote:
> 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.
>
> Signed-off-by: Kees Cook <[email protected]>

LGTM:

Reviewed-by: Gustavo A. R. Silva <[email protected]>

Thanks!
--
Gustavo

2024-03-07 07:32:31

by Przemek Kitszel

[permalink] [raw]
Subject: Re: [PATCH v2] overflow: Change DEFINE_FLEX to take __counted_by member

On 3/7/24 00:51, Kees Cook wrote:
> 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.
>
> 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]/
> ---
> 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(-)
>

Reviewed-by: Przemek Kitszel <[email protected]>

2024-03-08 20:20:33

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2] overflow: Change DEFINE_FLEX to take __counted_by member

On Wed, Mar 06, 2024 at 03:51:36PM -0800, Kees Cook wrote:
> 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.
>
> 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.

..

> diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
> index f84bab80ca42..d4baae8c3b72 100644
> --- a/drivers/net/ethernet/intel/ice/ice_switch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_switch.c

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;

..

> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index aa691f2119b0..677b03c4c84f 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -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.

> */
> -#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 { \

..

2024-03-09 20:32:57

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2] overflow: Change DEFINE_FLEX to take __counted_by member

On Fri, Mar 08, 2024 at 08:20:18PM +0000, Simon Horman wrote:
> On Wed, Mar 06, 2024 at 03:51:36PM -0800, Kees Cook wrote:
> > 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.
> >
> > 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?

> > @@ -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/

--
Kees Cook

2024-03-11 09:28:31

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v2] overflow: Change DEFINE_FLEX to take __counted_by member

On Sat, Mar 09, 2024 at 12:32:45PM -0800, Kees Cook wrote:
> On Fri, Mar 08, 2024 at 08:20:18PM +0000, Simon Horman wrote:
> > On Wed, Mar 06, 2024 at 03:51:36PM -0800, Kees Cook wrote:
> > > 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.
> > >
> > > 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?

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.

>
> > > @@ -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/
>
> --
> Kees Cook
>

2024-03-11 18:53:27

by Tony Nguyen

[permalink] [raw]
Subject: Re: [PATCH v2] overflow: Change DEFINE_FLEX to take __counted_by member



On 3/11/2024 2:28 AM, Simon Horman wrote:
> On Sat, Mar 09, 2024 at 12:32:45PM -0800, Kees Cook wrote:
>> On Fri, Mar 08, 2024 at 08:20:18PM +0000, Simon Horman wrote:
>>> On Wed, Mar 06, 2024 at 03:51:36PM -0800, Kees Cook wrote:
>>>> 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.
>>>>
>>>> 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?
>
> 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

>>
>>>> @@ -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/
>>
>> --
>> Kees Cook
>>