On 08.07.2018 11:24, Martin Husemann wrote:
> On Sun, Jul 08, 2018 at 10:49:53AM +0200, Jaromír Dole?ek wrote:
>>> Module Name: src
>>> Committed By: kamil
>>> Date: Sat Jul 7 23:05:50 UTC 2018
>>>
>>> Modified Files:
>>> src/sys/arch/x86/x86: mpbios.c
>>>
>>> Log Message:
>>> Remove unaligned access to mpbios_page[]
>>>
>>> Replace unaligned pointer dereference with a more portable construct that
>>> is free from Undefined Behavior semantics.
>>>
>>> sys/arch/x86/x86/mpbios.c:308:11, load of misaligned address 0xffff800031c7a413 for type 'const __uint16_t' which requires 2 byte alignment
>
>
> Can we please do NOT do such stupid changes?
>
> This is a bogus error message, please restore the original code!
>
> Martin
>
On the request by Martin, I'm moving this discussion to tech-kern.
Kernel Undefined Behavior Sanitizer detects various portability issues
including alignment.
Misaligned pointer is explicitly documented as undefined behavior in the
C standard (C11 6.3.2.3 p7). (In C++ it's basically the same.)
Depending on the CPU, misaligned access can be either supported (with or
without performance hit) or unsupported (CPU exception, reinterpret
instruction to perform a different operation etc). For some CPUs it's
possible to define a dedicated exception handler to align the data
access on the performance cost.
There is a NetBSD header symbol that marks certain architectures as
capable to handle miaslignment: __NO_STRICT_ALIGNMENT. It's defined
right now for m68k, vax, amd64 and i386.
In future __NO_STRICT_ALIGNMENT could be defined for aarch64, at least
for the use of acpica (which still contains a fallback for Itanium
without misaligned access, but not actively maintained).
Linux uses a different approach and ships get_unaligned() and
set_unaligned() macros and implements it on per ABI and CPU-basis.
Almost all of the remaining issues detected by Kernel UBsan come from
unaligned memory access:
Report from a boot in qemu:
http://netbsd.org/~kamil/kubsan/0006-boot.txt
Report from a boot on a real hardware (with drmkms disabled):
http://netbsd.org/~kamil/kubsan/0007-boot-real-hardware.txt
The plan with Kernel Sanitizers is to mark the issues detected with them
as fatal, with a warning/fatal switch option for UBSan. Marking UBSan
issues as fatal makes this sanitizer useful for fuzzing and regular
checks for regressions (ATF execution), because it will detect the
issues immediately or just detect at all.
However in order to make Kernel UBSan more useful there is need to
address the reports, some of them might be cautious.
Handling cautious KUBSan reports can be treated similarly to handling
compiler warnings. We are already marking a lot of variables as
initialized, signed/unsigned, long/short or even properly intended. Some
of them are overcautious. UBSan detects a similar set of problems during
the runtime execution.
Regarding the unaligned access, there is an option to:
- mark certain kernel functions with no-sanitize flags,
- address the reported issues if possible.
I've introduced the change to mpbios.c as it was small, selfcontained
and without the need to decorate the whole function.
Various device drivers (vge(4), vr(4), sip(4), wdc(4) etc) contain
fixups for misalignment access under ifdef. Other ones like bwfm(4) [1]
contain fixups inlined and were detecting when a driver was in the
process of porting from x86 to a port with strict alignment.
Such issues as bwfm(4) could be detected with the sanitizer aid more easily.
[1]
commit 44bb30584312c2fa6bc0661178986c4965b67c0c
Author: jmcneill <jmcneill%NetBSD.org@localhost>
Date: Fri Oct 20 23:38:21 2017 +0000
Fix an alignment problem with scan results within an escan event
Attachment:
signature.asc
Description: OpenPGP digital signature