netdev - Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app object

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: <87pmf9xrrd.fsf@nvidia.com>
Date:   Mon, 3 Oct 2022 09:52:59 +0200
From:   Petr Machata <petrm@...dia.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     Petr Machata <petrm@...dia.com>,
        Daniel Machon <daniel.machon@...rochip.com>,
        <netdev@...r.kernel.org>, <davem@...emloft.net>,
        <maxime.chevallier@...tlin.com>, <thomas.petazzoni@...tlin.com>,
        <edumazet@...gle.com>, <pabeni@...hat.com>,
        <lars.povlsen@...rochip.com>, <Steen.Hegelund@...rochip.com>,
        <UNGLinuxDriver@...rochip.com>, <joe@...ches.com>,
        <linux@...linux.org.uk>, <horatiu.vultur@...rochip.com>,
        <Julia.Lawall@...ia.fr>, <vladimir.oltean@....com>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH net-next v2 1/6] net: dcb: add new pcp selector to app
 object


Jakub Kicinski <kuba@...nel.org> writes:

> On Fri, 30 Sep 2022 14:20:50 +0200 Petr Machata wrote:
>> > @@ -1495,7 +1536,7 @@ static int dcbnl_ieee_set(struct net_device *netdev, struct nlmsghdr *nlh,
>> >  		nla_for_each_nested(attr, ieee[DCB_ATTR_IEEE_APP_TABLE], rem) {
>> >  			struct dcb_app *app_data;
>> >
>> > -			if (nla_type(attr) != DCB_ATTR_IEEE_APP)
>> > +			if (!dcbnl_app_attr_type_validate(nla_type(attr)))  
>> 
>> Oh no! It wasn't validating the DCB_ATTR_IEEE_APP_TABLE nest against a
>> policy! Instead it was just skipping whatever is not DCB_ATTR_IEEE_APP.
>> 
>> So userspace was permitted to shove random crap down here, and it would
>> just quietly be ignored. We can't start reinterpreting some of that crap
>> as information. We also can't start bouncing it.
>
> Are you saying that we can't start interpreting new attr types?
>
> "Traditionally" netlink ignored new attr types so from that perspective
> starting to interpret new types is pretty "run of the mill" for netlink.
> IOW *_deprecated() parsing routines do not use NL_VALIDATE_MAXTYPE.
>
> That does put netlink in a bit of a special category when it comes to
> input validation, but really putting in a random but valid attr is much
> harder than not initializing a struct member. Is there user space which
> does that?
>
> Sorry if I'm misinterpreting the situation.

I assumed the policy is much more strict with changes like this. If you
think it's OK, I'm fine with it as well.

The userspace (lldpad in particular) is doing the opposite thing BTW:
assuming everything in the nest is a DCB_ATTR_IEEE_APP. When we start
emitting the new attribute, it will get confused.