Port-arm archive

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

Unaligned access in kernel on ARMv6+ (Re: CVS commit: src/sys/dev/usb)

(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:


This results in kernel panic due to alignment fault on earmv[67]hf:


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..


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
% cat test.c
#include <stdio.h>
#include <string.h>

        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

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

% 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?

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
 ## (3) libkern and compat

Home | Main Index | Thread Index | Old Index