* Re: [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules
2026-09-17 11:17 [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules lirongqing
@ 2026-09-17 11:25 ` sashiko-bot
2026-09-18 6:55 ` 答复: " Li,Rongqing
2026-09-23 12:41 ` Leon Romanovsky
2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-17 11:25 UTC (permalink / raw)
To: lirongqing; +Cc: linux-rdma
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rdma: embedding struct with flexible array member not at end
--
commit 8e283bb18af02522e075bf4c88abcbec6ff60c4a
Author: Li RongQing <lirongqing@baidu.com>
RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules
This commit restores the original ordering of members in the leftovers
flow rule structs to fix an out-of-bounds read during flow rule creation.
By moving flow_attr to the front, the layout matches the offset calculations
expected by the flow creation code.
> diff --git a/drivers/infiniband/hw/mlx5/fs.c b/drivers/infiniband/hw/mlx5/fs.c
> index b155baee0017c..e27826c3087bf 100644
> --- a/drivers/infiniband/hw/mlx5/fs.c
> +++ b/drivers/infiniband/hw/mlx5/fs.c
> @@ -1651,8 +1651,8 @@ static struct mlx5_ib_flow_handler *create_leftovers_rule(struct mlx5_ib_dev *de
> struct mlx5_ib_flow_handler *handler = NULL;
>
> static struct {
> - struct ib_flow_spec_eth eth_flow;
> struct ib_flow_attr flow_attr;
> + struct ib_flow_spec_eth eth_flow;
[Severity: Low]
Does embedding struct ib_flow_attr at the beginning of this anonymous
structure violate the C standard for flexible array members?
Looking at struct ib_flow_attr, it contains a C99 flexible array member
union ib_flow_spec flows[] at the end. The C standard specifies that a
structure containing a flexible array member should not be embedded within
another structure, particularly as a non-last member.
While this ordering fixes the runtime offset calculation expected by
create_flow_rule(), does placing it before eth_flow trigger modern compiler
warnings like -Wflex-array-member-not-at-end?
> } leftovers_wc = { .flow_attr = { .num_of_specs = 1,
> .size = sizeof(leftovers_wc) },
> .eth_flow = {
> @@ -1662,8 +1662,8 @@ static struct mlx5_ib_flow_handler *create_leftovers_rule(struct mlx5_ib_dev *de
> .val = { .dst_mac = { 0x1 } } } };
>
> static struct {
> - struct ib_flow_spec_eth eth_flow;
> struct ib_flow_attr flow_attr;
> + struct ib_flow_spec_eth eth_flow;
[Severity: Low]
Similarly, does this second anonymous struct declaration also embed
struct ib_flow_attr and its flexible array member improperly?
> } leftovers_uc = { .flow_attr = { .num_of_specs = 1,
> .size = sizeof(leftovers_uc) },
> .eth_flow = {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260917111722.1723-1-lirongqing@baidu.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread* 答复: [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules
2026-09-17 11:17 [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules lirongqing
2026-09-17 11:25 ` sashiko-bot
@ 2026-09-18 6:55 ` Li,Rongqing
2026-09-23 12:41 ` Leon Romanovsky
2 siblings, 0 replies; 6+ messages in thread
From: Li,Rongqing @ 2026-09-18 6:55 UTC (permalink / raw)
To: Leon Romanovsky, Jason Gunthorpe, Zhu Yanjun,
linux-rdma@vger.kernel.org
>
> From: Li RongQing <lirongqing@baidu.com>
>
> Commit e91fb8b9d0ed ("RDMA/mlx5: Avoid flexible array warning") split the
> leftovers_specs[] array into two separate structs to silence the "array of flexible
> structures" sparse warning, but in doing so reversed the member order: eth_flow
> was placed before flow_attr.
>
> _create_flow_rule() locates the flow specs with:
>
> ib_flow = (void *)flow_attr + sizeof(*flow_attr);
>
> which expects the spec data to immediately follow the ib_flow_attr.
> With the reversed order, flow_attr is the last member, so ib_flow points past the
> end of the struct and the num_of_specs = 1 loop reads out-of-bounds memory
> (whatever static data follows in the .data section), producing incorrect flow
> rules or a crash.
>
> Restore flow_attr as the first member so the eth_flow spec again sits at the offset
> _create_flow_rule() expects. The two structs are no longer arrays, so the "array
> of flexible structures" warning does not re-occur.
>
> Fixes: e91fb8b9d0ed ("RDMA/mlx5: Avoid flexible array warning")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> drivers/infiniband/hw/mlx5/fs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/fs.c b/drivers/infiniband/hw/mlx5/fs.c
> index 98cdc5c..034ab55 100644
> --- a/drivers/infiniband/hw/mlx5/fs.c
> +++ b/drivers/infiniband/hw/mlx5/fs.c
> @@ -1651,8 +1651,8 @@ static struct mlx5_ib_flow_handler
> *create_leftovers_rule(struct mlx5_ib_dev *de
> struct mlx5_ib_flow_handler *handler = NULL;
>
> static struct {
> - struct ib_flow_spec_eth eth_flow;
> struct ib_flow_attr flow_attr;
> + struct ib_flow_spec_eth eth_flow;
> } leftovers_wc = { .flow_attr = { .num_of_specs = 1,
> .size = sizeof(leftovers_wc) },
> .eth_flow = {
> @@ -1662,8 +1662,8 @@ static struct mlx5_ib_flow_handler
> *create_leftovers_rule(struct mlx5_ib_dev *de
> .val = { .dst_mac = { 0x1 } } } };
>
> static struct {
> - struct ib_flow_spec_eth eth_flow;
> struct ib_flow_attr flow_attr;
> + struct ib_flow_spec_eth eth_flow;
> } leftovers_uc = { .flow_attr = { .num_of_specs = 1,
> .size = sizeof(leftovers_uc) },
> .eth_flow = {
> --
> 2.9.4
This patch should be like below, I will send v2:
diff --git a/drivers/infiniband/hw/mlx5/fs.c b/drivers/infiniband/hw/mlx5/fs.c
index 98cdc5c..e63fdb4 100644
--- a/drivers/infiniband/hw/mlx5/fs.c
+++ b/drivers/infiniband/hw/mlx5/fs.c
@@ -17,6 +17,7 @@
#include <linux/mlx5/fs.h>
#include <linux/mlx5/fs_helpers.h>
#include <linux/mlx5/eswitch.h>
+#include <linux/overflow.h>
#include <net/inet_ecn.h>
#include "mlx5_ib.h"
#include "counters.h"
@@ -1647,36 +1648,32 @@ static struct mlx5_ib_flow_handler *create_leftovers_rule(struct mlx5_ib_dev *de
struct ib_flow_attr *flow_attr,
struct mlx5_flow_destination *dst)
{
+ DEFINE_RAW_FLEX(struct ib_flow_attr, flow_attr_wc, flows, 1);
+ DEFINE_RAW_FLEX(struct ib_flow_attr, flow_attr_uc, flows, 1);
+
struct mlx5_ib_flow_handler *handler_ucast = NULL;
struct mlx5_ib_flow_handler *handler = NULL;
- static struct {
- struct ib_flow_spec_eth eth_flow;
- struct ib_flow_attr flow_attr;
- } leftovers_wc = { .flow_attr = { .num_of_specs = 1,
- .size = sizeof(leftovers_wc) },
- .eth_flow = {
- .type = IB_FLOW_SPEC_ETH,
- .size = sizeof(struct ib_flow_spec_eth),
- .mask = { .dst_mac = { 0x1 } },
- .val = { .dst_mac = { 0x1 } } } };
-
- static struct {
- struct ib_flow_spec_eth eth_flow;
- struct ib_flow_attr flow_attr;
- } leftovers_uc = { .flow_attr = { .num_of_specs = 1,
- .size = sizeof(leftovers_uc) },
- .eth_flow = {
- .type = IB_FLOW_SPEC_ETH,
- .size = sizeof(struct ib_flow_spec_eth),
- .mask = { .dst_mac = { 0x1 } },
- .val = { .dst_mac = {} } } };
-
- handler = create_flow_rule(dev, ft_prio, &leftovers_wc.flow_attr, dst);
+ flow_attr_wc->num_of_specs = 1;
+ flow_attr_wc->size = sizeof(struct ib_flow_attr) +
+ sizeof(struct ib_flow_spec_eth);
+ flow_attr_wc->flows[0].eth.type = IB_FLOW_SPEC_ETH;
+ flow_attr_wc->flows[0].eth.size = sizeof(struct ib_flow_spec_eth);
+ flow_attr_wc->flows[0].eth.mask.dst_mac[0] = 0x1;
+ flow_attr_wc->flows[0].eth.val.dst_mac[0] = 0x1;
+
+ flow_attr_uc->num_of_specs = 1;
+ flow_attr_uc->size = sizeof(struct ib_flow_attr) +
+ sizeof(struct ib_flow_spec_eth);
+ flow_attr_uc->flows[0].eth.type = IB_FLOW_SPEC_ETH;
+ flow_attr_uc->flows[0].eth.size = sizeof(struct ib_flow_spec_eth);
+ flow_attr_uc->flows[0].eth.mask.dst_mac[0] = 0x1;
+
+ handler = create_flow_rule(dev, ft_prio, flow_attr_wc, dst);
if (!IS_ERR(handler) &&
flow_attr->type == IB_FLOW_ATTR_ALL_DEFAULT) {
handler_ucast = create_flow_rule(dev, ft_prio,
- &leftovers_uc.flow_attr, dst);
+ flow_attr_uc, dst);
if (IS_ERR(handler_ucast)) {
mlx5_del_flow_rules(handler->rule);
ft_prio->refcount--;
[Li,Rongqing]
^ permalink raw reply related [flat|nested] 6+ messages in thread