I've patched machine/types.h for amd64 and i368. __NO_STRICT_ALIGNMENT will be now disabled for UBSan. I presume that we need to update with NetBSD/src to HEAD and rebuild ./build.sh tools from scratch. There is needed to pick this commit: https://github.com/NetBSD/src/commit/e7ec8fee829fd9dde56df8aed97de3f91ebab711 It will specify __SANITIZE_UNDEFINED__ for kUBSan. This change should handle all IPv6 alignment reports (unless there are real bugs somewhere). On 20.09.2019 19:03, Kamil Rytarowski wrote: > On 20.09.2019 16:22, Dmitry Vyukov wrote: >> On Fri, Sep 20, 2019 at 2:58 PM Kamil Rytarowski <n54%gmx.com@localhost> wrote: >>> >>> On 20.09.2019 08:14, Dmitry Vyukov wrote: >>>> On Fri, Sep 20, 2019 at 3:39 AM Kamil Rytarowski <n54%gmx.com@localhost> 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: https://syzkaller.appspot.com/x/log.txt?x=1176d8e9600000 >>>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=824b23e1f4b6c76b >>>>>> dashboard link: >>>>>> https://syzkaller.appspot.com/bug?extid=b53a9bcf030288081e65 >>>>>> >>>>>> 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: syzbot+b53a9bcf030288081e65%syzkaller.appspotmail.com@localhost >>>>>> >>>>>> [ 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: >>>>> >>>>> http://netbsd.org/~kamil/patch-00151-ip6addr.txt >>>>> >>>>> 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? >>> >>> http://netbsd.org/~kamil/patch-00151-ip6addr.txt >> >> >> 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 > __NO_STRICT_ALIGNMENT under UBSan. > > http://cdn.netbsd.org/pub/NetBSD/misc/joerg/GENERIC/src/src/sys/arch/amd64/include/types.h.html#_M/__NO_STRICT_ALIGNMENT > > This will allow to use alternative code-paths, like here: > > http://cdn.netbsd.org/pub/NetBSD/misc/joerg/GENERIC/src/src/sys/netinet6/ip6_input.c.html#315 > > My patch: > > http://netbsd.org/~kamil/patch-00152-__NO_STRICT_ALIGNMENT-disable-under-ubsan.txt > > This needs proper testing whether it really helps (hopefully yes). > > > My GCC patch (still building): > > http://netbsd.org/~kamil/patch-00153-gcc-lsan-ubsan-ifdef.txt > > The LSAN change is related to Clang/LLVM: > > https://reviews.llvm.org/D67719 > > We need LSAN for our userland applications for suppressing uninteresting > leaks. > > 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