On 04/07/2018 07:40 PM, David Miller wrote:
> From: Eric Dumazet <eduma...@google.com>
> Date: Sat, 7 Apr 2018 13:42:35 -0700
>
>> It seems syzbot got new features enabled, and fired some interesting
>> reports. Oh well.
>
> Series applied, however in patch #7 the condition syzbot detects
> cannot happen.
>
> In all code paths that lead to __mkroute_output() with res->type
> uninitialized, __mkroute_output() will reassign the local variable
> 'type' before reading it.
Well, we have :
u16 type = res->type;
...
if (ipv4_is_lbcast(fl4->daddr))
type = RTN_BROADCAST;
else if (ipv4_is_multicast(fl4->daddr))
type = RTN_MULTICAST;
else if (ipv4_is_zeronet(fl4->daddr))
return ERR_PTR(-EINVAL);
...
if (type == RTN_BROADCAST) { /* This is where KMSAN complained */
So it looks like type could have been random at this point.
>
> Furthermore, by doing a full structure initialization lots of
> unrelated things will be initialized now as well.
fib_result is 40 bytes on 64bit arches.
>
> We explicitly are only setting up the "inputs" of the fib_result
> object before we call fib_lookup(). The prefixlen and other members
> have no business being initialized there.
>
Yep
We might put all inputs at the beginning of the structure,
and output at the end. then replace sizeof() by offsetof(),
but this looks a bit convoluted and maybe risky.
