netdev - Re: [PATCH iproute2] tc_util: Fix parsing action control with space and slash

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] [day] [month] [year] [list]

Date:   Thu, 10 Feb 2022 17:21:40 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Roi Dayan <roid@...dia.com>
Cc:     <netdev@...r.kernel.org>, Maor Dickman <maord@...dia.com>
Subject: Re: [PATCH iproute2] tc_util: Fix parsing action control with space
 and slash

On Thu, 3 Feb 2022 14:20:46 +0200
Roi Dayan <roid@...dia.com> wrote:

> For action police there is an conform-exceed action control
> which can be for example "jump 2 / pipe".
> The current parsing loop is doing one more iteration than necessary
> and results in ok var being 3.
> 
> Example filter:
> 
> tc filter add dev enp8s0f0_0 ingress protocol ip prio 2 flower \
>     verbose action police rate 100mbit burst 12m \
>     conform-exceed jump 1 / pipe mirred egress redirect dev enp8s0f0_1 action drop
> 
> Before this change the command will fail.
> Trying to add another "pipe" before mirred as a workaround for the stopping the loop
> in ok var 3 resulting in result2 not being saved and wrong filter.
> 
> ... conform-exceed jump 1 / pipe pipe mirred ...
> 
> Example dump of the action part:
> ... action order 1:  police 0x1 rate 100Mbit burst 12Mb mtu 2Kb action jump 1 overhead 0b  ...
> 
> Fix the behavior by removing redundant case 2 handling, either argc is over or breaking.
> 
> Example dump of the action part with the fix:
> ... action order 1:  police 0x1 rate 100Mbit burst 12Mb mtu 2Kb action jump 1/pipe overhead 0b ...
> 
> Signed-off-by: Roi Dayan <roid@...dia.com>
> Reviewed-by: Maor Dickman <maord@...dia.com>
> ---
>  tc/tc_util.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index 48065897cee7..b82dbd5dc75d 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
> @@ -476,7 +476,6 @@ static int parse_action_control_slash_spaces(int *argc_p, char ***argv_p,
>  			NEXT_ARG();
>  			/* fall-through */
>  		case 0: /* fall-through */
> -		case 2:
>  			ret = parse_action_control(&argc, &argv,
>  						   result_p, allow_num);
>  			if (ret)

Applied, and removed the now unnecessary second fall-through comment.