netdev - Re: [PATCH iproute2-next 2/2] dcb: add new subcommand for apptrust

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]

Date:   Fri, 25 Nov 2022 14:06:36 +0100
From:   Petr Machata <petrm@...dia.com>
To:     <Daniel.Machon@...rochip.com>
CC:     <petrm@...dia.com>, <netdev@...r.kernel.org>, <dsahern@...nel.org>,
        <stephen@...workplumber.org>, <maxime.chevallier@...tlin.com>,
        <vladimir.oltean@....com>, <UNGLinuxDriver@...rochip.com>
Subject: Re: [PATCH iproute2-next 2/2] dcb: add new subcommand for apptrust


<Daniel.Machon@...rochip.com> writes:

>> > +static int dcb_apptrust_parse_selector_list(int *argcp, char ***argvp,
>> > +                                         struct dcb_apptrust_table *table)
>> > +{
>> > +     char **argv = *argvp;
>> > +     int argc = *argcp;
>> > +     __u8 selector;
>> > +     int ret;
>> > +
>> > +     NEXT_ARG_FWD();
>> > +
>> > +     /* No trusted selectors ? */
>> > +     if (argc == 0)
>> > +             goto out;
>> > +
>> > +     while (argc > 0) {
>> > +             selector = parse_one_of("order", *argv, selector_names,
>> > +                                     ARRAY_SIZE(selector_names), &ret);
>> > +             if (ret < 0)
>> > +                     return -EINVAL;
>> 
>> I think this should legitimately conclude the parsing, because it could
>> be one of the higher-level keywords. Currently there's only one,
>> "order", but nonetheless. I think it should goto out, and be plonked by
>> the caller with "what is X?". Similar to how the first argument that
>> doesn't parse as e.g. DSCP:PRIO bails out and is attempted as a keyword
>> higher up, and either parsed, or plonked with "what is X".
>
> I dont quite follow you on this one. We are parsing the selector list
> here. Any offending selector is printed, as well as the entire list of
> valid ones. How could it be one of the higher-level keywords? Am I
> missing something here? :-)

Imagine there's more to specify than order. Say, per-selector rewrite
enablement or something. Then the command-line to specify both at the
same time could look like this:

# dcb apptrust set dev eth0 order dscp pcp rewrite dscp:on pcp:off

I think that currently the "rewrite" keyword will trigger the -EINVAL
return above and the whole command line will be rejected.

Right now it's all the same, because there's only one thing to
configure, but it would be cleaner to handle this case as if there could
be more things to configure.