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.


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.

Reply via email to