kUBSan detected a number of unaligned accesses in USB code:
http://netbsd.org/~kamil/kubsan/0007-boot-real-hardware.txt
On 06.01.2019 09:46, Rin Okuyama wrote:
> (CC added to port-arm%NetBSD.org@localhost)
>
> Let me summarize the problem briefly. In axe(4), there is a code where
> memcpy() is carried out from 2-byte aligned buffer to 4-byte structure:
>
> https://nxr.netbsd.org/xref/src/sys/dev/usb/if_axe.c#1284
>
> This results in kernel panic due to alignment fault on earmv[67]hf:
>
> https://twitter.com/furandon_pig/status/1071771151418908672
>
> In short, this is because -munaligned-access is enabled on ARMv6+ by
> default for GCC. As the unaligned memory access is forbidden in the
> supervisor mode unlike in the user mode, we need to explicitly specify
> -mno-unaligned-access for kernel on ARMv6+.
>
> On 2019/01/06 12:59, matthew green wrote:
>> Christos Zoulas writes:
>>> In article <20190106003905.60969FB17%cvs.NetBSD.org@localhost>,
>>> Rin Okuyama <source-changes-d%NetBSD.org@localhost> wrote:
>>>> -=-=-=-=-=-
>>>>
>>>> Module Name: src
>>>> Committed By: rin
>>>> Date: Sun Jan 6 00:39:05 UTC 2019
>>>>
>>>> Modified Files:
>>>> src/sys/dev/usb: if_axe.c
>>>>
>>>> Log Message:
>>>> Fix kernel panic on arm reported by @furandon_pig on Twitter.
>>>>
>>>> Hardware header is 2-byte aligned in RX buffer, not 4-byte.
>>>> For some architectures, __builtin_memcpy() of GCC 6 attempts to
>>>> copy 4-byte header at once, which results in alignment error.
>>>
>>> This is really ugly..
>>>
>>> https://stackoverflow.com/questions/24883410/armcc-problems-with-memcpy-alignment-exceptions
>>>
>>>
>>> Perhaps there is a better solution? Can't memcpy be smarter?
>>
>> hmmm, what happens if struct axe_sframe_hdr is not marked
>> "packed"? this feels like a compiler bug, but perhaps it
>> is assuming it can write 4 bytes to the structure when it
>> is only 2 byte aligned.
>>
>> is there a small test case that reproduces the problem?
>> preferably in user land?
>
> On 2019/01/06 15:25, maya%netbsd.org@localhost wrote:
>> Are we building ARM with -mstrict-alignment?
>
> I tried to reproduce the problem on userland. objdump(1) shows an
> unaligned load is generated. However, alignment fault does not occur:
>
> % uname -p
> earmv7hf
> % cat test.c
> #include <stdio.h>
> #include <string.h>
>
> int
> main()
> {
> char buf[sizeof(int) + 1];
> int i;
>
> fread(buf, 1, sizeof(buf), stdin);
> memcpy(&i, &buf[1], sizeof(i));
> printf("0x%x\n", i);
> return 0;
> }
> % cc -g -O2 test.c && cc test.o
> % objdump test.o
> ...
> 28: e51b1013 ldr r1, [fp, #-19] ; 0xffffffed
> ...
> % ./a.out
> 01234
> 0x34333231
>
> This is because unaligned access is permitted for the user mode on
> ARMv6+. For GCC, -munaligned-access is enabled by default on ARMv6+.
> However, the unaligned access is forbidden in the supervisor mode.
> So, we need to explicitly specify -mno-unaligned-access for kernel
> on ARMv6+.
>
> By reverting if_axe.c r1.94 and applying the attached patch, axe(4)
> works fine on earmv7hf. We can see that the instruction is changed
> from word-wise load to byte-wise load by specifying
> -mno-unaligned-access:
>
> % armv7--netbsdelf-eabihf-objdump -d if_axe.o
> (before) 364: e4983004 ldr r3, [r8], #4
> --->
> (after) 364: e5d60000 ldrb r0, [r6]
>
> I guess other codes can be miscompiled if -mno-unaligned-access is
> not specified. Can I commit the patch?
>
> Thanks,
> rin
> ----
> Index: sys/arch/arm/conf/Makefile.arm
> ===================================================================
> RCS file: /home/netbsd/src/sys/arch/arm/conf/Makefile.arm,v
> retrieving revision 1.49
> diff -p -u -r1.49 Makefile.arm
> --- sys/arch/arm/conf/Makefile.arm 22 Sep 2018 12:24:01 -0000 1.49
> +++ sys/arch/arm/conf/Makefile.arm 6 Jan 2019 08:14:56 -0000
> @@ -53,6 +53,13 @@ CPPFLAGS.cpufunc_asm_armv6.S+= -mcpu=arm
> CPPFLAGS.cpufunc_asm_arm11.S+= -mcpu=arm1136j-s
> CPPFLAGS.cpufunc_asm_xscale.S+= -mcpu=xscale
>
> +# For GCC, -munaligned-access is enabled by default for ARMv6+.
> +# But the unaligned access is forbidden in the supervisor mode.
> +.if (!empty(MACHINE_ARCH:Mearmv6*) || !empty(MACHINE_ARCH:Mearmv7*)) \
> + && ${ACTIVE_CC} == "gcc"
> +CFLAGS+= -mno-unaligned-access
> +.endif
> +
> ##
> ## (3) libkern and compat
> ##
Attachment:
signature.asc
Description: OpenPGP digital signature