netdev - RE: [PATCH net v2] net/sched: cls_api: Fix flow action initialization

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DM5PR1301MB21724323009FF1C33518E4C5E78E9@DM5PR1301MB2172.namprd13.prod.outlook.com>
Date:   Wed, 20 Jul 2022 09:19:01 +0000
From:   Baowen Zheng <baowen.zheng@...igine.com>
To:     Oz Shlomo <ozsh@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Simon Horman <simon.horman@...igine.com>,
        "David S. Miller" <davem@...emloft.net>,
        Roi Dayan <roid@...dia.com>
Subject: RE: [PATCH net v2] net/sched: cls_api: Fix flow action initialization

>Subject: [PATCH net v2] net/sched: cls_api: Fix flow action initialization
>
>The cited commit refactored the flow action initialization sequence to use an
>interface method when translating tc action instances to flow offload objects.
>The refactored version skips the initialization of the generic flow action
>attributes for tc actions, such as pedit, that allocate more than one offload
>entry. This can cause potential issues for drivers mapping flow action ids.
>
>Populate the generic flow action fields for all the flow action entries.
>
>Fixes: c54e1d920f04 ("flow_offload: add ops to tc_action_ops for flow action
>setup")
>Signed-off-by: Oz Shlomo <ozsh@...dia.com>
>Reviewed-by: Roi Dayan <roid@...dia.com>
>
>----
>v1 -> v2:
> - coalese the generic flow action fields initialization to a single loop
>---
> net/sched/cls_api.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index
>c7a240232b8d..790d6809be81 100644
>--- a/net/sched/cls_api.c
>+++ b/net/sched/cls_api.c
>@@ -3534,7 +3534,7 @@ int tc_setup_action(struct flow_action *flow_action,
> 		    struct tc_action *actions[],
> 		    struct netlink_ext_ack *extack)
> {
>-	int i, j, index, err = 0;
>+	int i, j, k, index, err = 0;
> 	struct tc_action *act;
>
> 	BUILD_BUG_ON(TCA_ACT_HW_STATS_ANY !=
>FLOW_ACTION_HW_STATS_ANY); @@ -3554,14 +3554,18 @@ int
>tc_setup_action(struct flow_action *flow_action,
> 		if (err)
> 			goto err_out_locked;
>
>-		entry->hw_stats = tc_act_hw_stats(act->hw_stats);
>-		entry->hw_index = act->tcfa_index;
> 		index = 0;
> 		err = tc_setup_offload_act(act, entry, &index, extack);
>-		if (!err)
>-			j += index;
>-		else
>+		if (err)
> 			goto err_out_locked;
>+
>+		for (k = 0; k < index ; k++) {
>+			entry[k].hw_stats = tc_act_hw_stats(act->hw_stats);
>+			entry[k].hw_index = act->tcfa_index;
>+		}
>+
>+		j += index;
>+
> 		spin_unlock_bh(&act->tcfa_lock);
> 	}
>
>--
>1.8.3.1
-----------------------------------------------------
Thanks, this change looks good to me now.

Reviewed-by: Baowen Zheng <baowen.zheng@...igine.com>