Re: [syzbot] WARNING: refcount bug in qrtr_node_lookup

[Paul Moore]

On Thu, Sep 2, 2021 at 9:58 AM Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
>
> On Thu, Sep 2, 2021 at 12:13 AM Hillf Danton <hdanton@xxxxxxxx> wrote:
> > On Wed, 01 Sep 2021 19:32:06 -0700
> > >
> > > syzbot has tested the proposed patch but the reproducer is still triggering an issue:
> > > UBSAN: object-size-mismatch in send4
> > >
> > > ================================================================================
> > > UBSAN: object-size-mismatch in ./include/net/flow.h:197:33
> > > member access within address 000000001597b753 with insufficient space
> > > for an object of type 'struct flowi'
> > > CPU: 1 PID: 231 Comm: kworker/u4:4 Not tainted 5.14.0-syzkaller #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > > Workqueue: wg-kex-wg0 wg_packet_handshake_send_worker
> > > Call Trace:
> > >  __dump_stack lib/dump_stack.c:88 [inline]
> > >  dump_stack_lvl+0x15e/0x1d3 lib/dump_stack.c:105
> > >  ubsan_epilogue lib/ubsan.c:148 [inline]
> > >  handle_object_size_mismatch lib/ubsan.c:229 [inline]
> > >  ubsan_type_mismatch_common+0x1de/0x390 lib/ubsan.c:242
> > >  __ubsan_handle_type_mismatch_v1+0x41/0x50 lib/ubsan.c:271
> > >  flowi4_to_flowi_common include/net/flow.h:197 [inline]
> >
> > This was added in 3df98d79215a ("lsm,selinux: pass flowi_common instead of
> > flowi to the LSM hooks"), could you take a look at the UBSAN report, Paul?
>
> Sure, although due to some flooding here at home it might take a day
> (two?) before I have any real comments on this.

I'm looking quickly at this tonight after a long day so it's possible
I'm missing something, but in the original report if you look one step
before the backtrace above you see the caller is send4 in the
wireguard code, which starts off by creating it's own flowi4 variable
on the stack and then eventually passing it down to
flowi4_to_flowi_common() for use as the second parameter to
security_sk_classify_flow().  Because the wireguard code only
allocates a flowi4 and not a full flowi struct it seems like this
would explain the size mismatch warning (flowi is larger than flowi4
due to the union containing flowi6 as well as flowi4).

Off the top of my head it isn't clear to me if it is considered "safe"
to allocate just a flowi4 in this way, you may need to allocate a full
flowi; if none of the netdev folks reply we could always check the
other flowi4 users in the kernel.

Depending on the above, the fix may be either to adjust send4() to use
a full flowi, or to adjust flowi4_to_flowi_common() to use the
flowi_common struct at the top of the flowi4 struct instead of first
looking for the flowi struct and going from there.

-- 
paul moore
www.paul-moore.com