On 12/08/2017 02:24 AM, Andrew Morton wrote:
> On Thu, 7 Dec 2017 16:31:23 +0300 Andrey Ryabinin <[email protected]>
> wrote:
>
>> On 12/07/2017 03:49 AM, Andrew Morton wrote:
>>> (correcting Andrey's email address)
>>>
>>>
>>> From: Ding Tianhong <[email protected]>
>>> Subject: lib/ubsan.c: don't handle misaligned address when kernel supports
>>> unaligned access
>>>
>>> ubsan reports a warning like:
>>>
>>> UBSAN: Undefined behaviour in ../include/linux/etherdevice.h:386:9
>>> load of misaligned address ffffffc069ba0482 for type 'long unsigned int'
>>> which requires 8 byte alignment
>>> CPU: 0 PID: 901 Comm: sshd Not tainted 4.xx+ #1
>>> Hardware name: linux,dummy-virt (DT)
>>> Call trace:
>>> [<ffffffc000093600>] dump_backtrace+0x0/0x348
>>> [<ffffffc000093968>] show_stack+0x20/0x30
>>> [<ffffffc001651664>] dump_stack+0x144/0x1b4
>>> [<ffffffc0016519b0>] ubsan_epilogue+0x18/0x74
>>> [<ffffffc001651bac>] __ubsan_handle_type_mismatch+0x1a0/0x25c
>>> [<ffffffc00125d8a0>] dev_gro_receive+0x17d8/0x1830
>>> [<ffffffc00125d928>] napi_gro_receive+0x30/0x158
>>> [<ffffffc000f4f93c>] virtnet_receive+0xad4/0x1fa8
>>>
>>> The reason is that when enabling the CONFIG_UBSAN_ALIGNMENT, ubsan will
>>> report the unaligned access even if the system supports it
>>> (CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y). This produces a lot of noise
>>> in the log and causes confusion.
>>>
>>
>> NACK. This doesn't make sense. If you don't want to see misaligned access
>> reports
>> you simply shouldn't enable CONFIG_UBSAN_ALIGNMENT.
>
> So should UBSAN Kconfig disable CONFIG_UBSAN_ALIGNMENT when
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y?
>
CONFIG_UBSAN_ALIGNMENT is already disabled by default for
HAVE_EFFICIENT_UNALIGNED_ACCESS=y because it's noisy,
but we still allow users to enable it if they want to.
I don't think we should completely forbid enabling it for
HAVE_EFFICIENT_UNALIGNED_ACCESS=y.
Unaligned access is still a bug in non-arch code and outside of sections like
#ifdef HAVE_EFFICIENT_UNALIGNED_ACCESS .. #endif .
As for UBSAN noise inside #ifdef HAVE_EFFICIENT_UNALIGNED_ACCESS .. #endif
sections, it should be possible to suppress it
with something like this:
typedef __attribute__((aligned(1))) int unaligned_int;
....
int x = *(unaligned_int*)unalinged_addr;
This shouldn't affect generated code (on arches that support unaligned loads)
and suppresses UBSAN warnings.
It's might be a right thing todo. Even if arch supports unaligned access, it's
still undefined behaviour according to the C standard.
And one day, GCC might start doing optimizations based on this, e.g.:
u64 *ptr;
...
x = *ptr;
...
if (ptr & 7) // Compiler can assume that this statement is always
false, because 'ptr' was deferenced, so it must be aligned
do_something();
