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

["Li,Rongqing" <lirongqing@baidu.com>]

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