netdev - Re: [PATCH iproute2-next v3 2/2] tc: implement support for terse dump

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: <b8138715-8fd7-cbef-d220-76bdb8c52ba5@mojatatu.com>
Date:   Mon, 19 Oct 2020 09:48:41 -0400
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Vlad Buslov <vlad@...lov.dev>
Cc:     Vlad Buslov <vladbu@...dia.com>, dsahern@...il.com,
        stephen@...workplumber.org, netdev@...r.kernel.org,
        davem@...emloft.net, xiyou.wangcong@...il.com, jiri@...nulli.us,
        ivecera@...hat.com, Vlad Buslov <vladbu@...lanox.com>
Subject: Re: [PATCH iproute2-next v3 2/2] tc: implement support for terse dump

On 2020-10-18 8:16 a.m., Vlad Buslov wrote:
> On Sat 17 Oct 2020 at 14:20, Jamal Hadi Salim <jhs@...atatu.com> wrote:
>> On 2020-10-16 12:42 p.m., Vlad Buslov wrote:
>>

>> Either one sounds appealing - the refactoring feels simpler
>> as opposed to a->terse_print().
> 
> With such refactoring we action type will be printed before some basic
> validation which can lead to outputting the type together with error
> message. Consider tunnel key action output callback as example:
> 
> static int print_tunnel_key(struct action_util *au, FILE *f, struct rtattr *arg)
> {
> 	struct rtattr *tb[TCA_TUNNEL_KEY_MAX + 1];
> 	struct tc_tunnel_key *parm;
> 
> 	if (!arg)
> 		return 0;
> 
> 	parse_rtattr_nested(tb, TCA_TUNNEL_KEY_MAX, arg);
> 
> 	if (!tb[TCA_TUNNEL_KEY_PARMS]) {
> 		fprintf(stderr, "Missing tunnel_key parameters\n");
> 		return -1;
> 	}
> 	parm = RTA_DATA(tb[TCA_TUNNEL_KEY_PARMS]);
> 
> 	print_string(PRINT_ANY, "kind", "%s ", "tunnel_key");
> 
> If print "kind" call is moved before checking the arg it will always be
> printed, even when immediately followed by "Missing tunnel_key
> parameters\n" string. Is this a concern?
> 

That could be a good thing, no? you get to see the action name with the
error. Its really not a big deal if you decide to do a->terse_print()
instead.

>>
>> BTW: the action index, unless i missed something, is not transported
>> from the kernel for terse option. It is an important parameter
>> when actions are shared by filters (since they will have the same
>> index).
>> Am i missing something?
> 
> Yes, tc_action_ops->dump(), which outputs action index among other data,
> is not called at all by terse dump.

I am suggesting it is an important detail that is currently missing.
Alternatively since you have the cookies in there - it is feasible that
someone who creates the action could "encode" the index in the cookie.
But that makes it a "proprietary" choice of whoever is creating
the filter/action.

cheers,
jamal