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
