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

[Johannes Berg <johannes@sipsolutions.net>]

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