netdev - Re: [PATCH net-next v2 3/7] ethtool: trim policy tables

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 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:   Thu, 08 Oct 2020 11:13:49 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, kernel-team@...com, jiri@...nulli.us,
        andrew@...n.ch, mkubecek@...e.cz
Subject: Re: [PATCH net-next v2 3/7] ethtool: trim policy tables

On Thu, 2020-10-08 at 11:12 +0200, Eric Dumazet wrote:
> 
> On 10/6/20 12:07 AM, Jakub Kicinski wrote:
> > Since ethtool uses strict attribute validation there's no need
> > to initialize all attributes in policy tables. 0 is NLA_UNSPEC
> > which is going to be rejected. Remove the NLA_REJECTs.
> > 
> > Similarly attributes above maxattrs are rejected, so there's
> > no need to always size the policy tables to ETHTOOL_A_..._MAX.
> > 
> 
> This implies that all policy tables must be 'complete'.
> 
> strset_stringsets_policy[] for example is :
> 
> static const struct nla_policy strset_stringsets_policy[] = {
>     [ETHTOOL_A_STRINGSETS_STRINGSET]    = { .type = NLA_NESTED },
> };
> 
> So when later strset_parse_request() does :
> 
> req_info->counts_only = tb[ETHTOOL_A_STRSET_COUNTS_ONLY];
> 
> We have an out-of-bound access since ETHTOOL_A_STRSET_COUNTS_ONLY > ETHTOOL_A_STRINGSETS_STRINGSET

Yeah, Leon Romanovsky reported actually running into this yesterday, and
I sent a fix :-)

> Not sure what was the expected type for this attribute, the kernel
> only looks at its presence, not its value.

It was NLA_FLAG, but never actually in the policy, so you could never
even successfully use it ...

Here was the fix
https://lore.kernel.org/netdev/20201007125348.a74389e18168.Ieab7a871e27b9698826e75dc9e825e4ddbc852b1@changeid/

johannes