[PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules

[PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules

[lirongqing]

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

Re: [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules

[sashiko-bot]

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

答复: [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules

[Li,Rongqing]

> 
> 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] 

Re: [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules

[Leon Romanovsky]

On Thu, 17 Sep 2026 19:17:22 +0800, lirongqing wrote:
> 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:
> 
> [...]

Applied, thanks!

[1/1] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules
      https://git.kernel.org/rdma/rdma/c/c1212a3c7ccd46

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>

Re: [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules

[Leon Romanovsky]

On Wed, Sep 23, 2026 at 08:41:48AM -0400, Leon Romanovsky wrote:
> 
> On Thu, 17 Sep 2026 19:17:22 +0800, lirongqing wrote:
> > 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:
> > 
> > [...]
> 
> Applied, thanks!

I dropped it, as it causes to flex array errors to return.

Thanks

> 
> [1/1] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules
>       https://git.kernel.org/rdma/rdma/c/c1212a3c7ccd46
> 
> Best regards,
> -- 
> Leon Romanovsky <leon@kernel.org>
> 
> 

Re: [PATCH] RDMA/mlx5: Restore flow_attr-first layout in leftovers flow rules

[Jason Gunthorpe]

On Wed, Sep 23, 2026 at 03:45:47PM +0300, Leon Romanovsky wrote:
> On Wed, Sep 23, 2026 at 08:41:48AM -0400, Leon Romanovsky wrote:
> > 
> > On Thu, 17 Sep 2026 19:17:22 +0800, lirongqing wrote:
> > > 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:
> > > 
> > > [...]
> > 
> > Applied, thanks!
> 
> I dropped it, as it causes to flex array errors to return.

Yeah, but the bug is real..

There is a macro for this case:

	static TRAILING_OVERLAP(struct ib_flow_attr, flow_attr, flows,
		struct ib_flow_spec_eth eth_flow;
	) leftovers_wc = { .flow_attr = { .num_of_specs = 1,

Jason