Messages in this thread | | | From | Paul Moore <> | Date | Thu, 2 Sep 2021 22:40:31 -0400 | Subject | Re: [syzbot] WARNING: refcount bug in qrtr_node_lookup |
| |
On Thu, Sep 2, 2021 at 9:58 AM Paul Moore <paul@paul-moore.com> wrote: > > On Thu, Sep 2, 2021 at 12:13 AM Hillf Danton <hdanton@sina.com> 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
| |