tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: panic: UBSan: Undefined Behavior in /syzkaller/managers/netbsd-kubsan/kernel/sys/netinet6/scope6.c:LINE, member access w

On 20.09.2019 16:22, Dmitry Vyukov wrote:
> On Fri, Sep 20, 2019 at 2:58 PM Kamil Rytarowski <> wrote:
>> On 20.09.2019 08:14, Dmitry Vyukov wrote:
>>> On Fri, Sep 20, 2019 at 3:39 AM Kamil Rytarowski <> wrote:
>>>> On 20.09.2019 00:24, syzbot wrote:
>>>>> Hello,
>>>>> syzbot found the following crash on:
>>>>> HEAD commit:    338a6d82 Use an explicit run-time assertion where
>>>>> compile-..
>>>>> git tree:       netbsd
>>>>> console output:
>>>>> kernel config:
>>>>> dashboard link:
>>>>> Unfortunately, I don't have any reproducer for this crash yet.
>>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>>> Reported-by:
>>>>> [  47.4625386] panic: UBSan: Undefined Behavior in
>>>>> /syzkaller/managers/netbsd-kubsan/kernel/sys/netinet6/scope6.c:480:6,
>>>>> member access within misaligned address 0xffff9457bc441286 for type
>>>>> 'struct in6_addr' which requires 4 byte alignment
>>>> How to address these reports? There are numbers of them.
>>>> The problem is that struct in6_addr is stored in __packed structs such
>>>> as (ip6_hdr) in the kernel. I don't know any other way to fix it than
>>>> marking in6_addr as __packed, but that changes ABI and breaks userland
>>>> qemu for some reason.
>>>> FreeBSD/OpenBSD have the same structs and attributes.
>>>> Illumos and Linux don't use __packed for ip6_hdr, but it is probably too
>>>> late to change our struct layout?
>>>> Also I don't have enough confidence of the ipv6 algorithms.
>>>> One variation (valid ?) is to put back __packed under ifdef _KERNEL:
>>>> Alternatively mark [probably too] many ipv6 related functions with
>>>> __noubsan.
>>>> This is probably the last UBSan issue before getting syzbot to fuzz the
>>>> kernel.
>>> FWIW one of official way to fix this in C/C++ is to declare a properly
>>> aligned type (int) and copyout the data into it with memcpy. For x86
>>> compiler should understand that and eliminate the copy.
>> memcpy/memcmp is too invasive here as we need to change macros shared
>> with userland.
>> Is the ifdefed __packed patch correct?
> This makes struct alignment 1, right?
> Can't it break some ABIs? What if the struct is included in some other
> struct and alignment 1 changes overall layout. Then usespace will pass
> padded, but kernel will assume unpadded layout. Is it possible?
> If we are out of good ideas, it can make sense to add some __noubsan's
> for very frequent and/or hard to address otherwise warnings. This will
> unstack finding the long tail of potentially more interesting
> warnings. And then these warnings can be addressed, and in parallel
> the __noubsan's can be fixed in better ways (if we come up with better
> ways, but now this won't be urgent because we put down the fires).

For the time being, I have marked it with __noubsan.

I think I know how to fix it. There is need to disable

This will allow to use alternative code-paths, like here:

My patch:

This needs proper testing whether it really helps (hopefully yes).

My GCC patch (still building):

The LSAN change is related to Clang/LLVM:

We need LSAN for our userland applications for suppressing uninteresting

I will try to upstream them both in one go. If the GCC patch will be
merged, I will commit patch 0152 and 0153 to src/.

Attachment: signature.asc
Description: OpenPGP digital signature

Home | Main Index | Thread Index | Old Index