[U-Boot] [PATCH 1/8] UBSAN: run-time undefined behavior sanity checker

[Tom Rini]

On Mon, Aug 20, 2018 at 02:00:25AM +0200, Eugeniu Rosca wrote:

> Import Undefined Behavior SANitizer from Linux Kernel v4.18, as
> implemented by Andrey Ryabinin <aryabinin at virtuozzo.com>.
> 
> Roughly, the UBSAN development history in Linux kernel looks like:
> 
> $ git log --format="%h (\"%s\")" --no-merges -- "*ubsan*"
> v4.18     3ca17b1f3628 ("lib/ubsan: remove null-pointer checks")
> v4.17-rc1 317506009216 ("lib/test_ubsan.c: make test_ubsan_misaligned_access() static")
> v4.17-rc1 854686f4edf4 ("lib: add testing module for UBSAN")
> v4.16-rc1 bac7a1fff792 ("lib/ubsan: remove returns-nonnull-attribute checks")
> v4.16-rc1 42440c1f9911 ("lib/ubsan: add type mismatch handler for new GCC/Clang")
> v4.16-rc1 b8fe1120b4ba ("lib/ubsan.c: s/missaligned/misaligned/")
> v4.14-rc8 b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license")
> v4.10-rc1 0462554707d6 ("Kconfig: lib/Kconfig.ubsan fix reference to ubsan documentation")
>  v4.9-rc5 a76bcf557ef4 ("Kbuild: enable -Wmaybe-uninitialized warning for "make W=1"")
>  v4.9-rc1 725c4d22bbc4 ("ubsan: allow to disable the null sanitizer")
>  v4.9-rc1 1ead009cd622 ("docs: sphinxify ubsan.txt and move it to dev-tools")
>  v4.8-rc1 901d805c33fc ("UBSAN: fix typo in format string")
>  v4.8-rc1 6e8d666e9253 ("Disable "maybe-uninitialized" warning globally")
>  v4.6-rc1 dde5cf39d4d2 ("ubsan: fix tree-wide -Wmaybe-uninitialized false positives")
>  v4.5-rc4 7707535ab95e ("ubsan: cosmetic fix to Kconfig text")
>  v4.5-rc1 c6d308534aef ("UBSAN: run-time undefined behavior sanity checker")
> 
> What's not interesting for U-Boot is:
>  - 317506009216 ("lib/test_ubsan.c: make test_ubsan_misaligned_access() static")
>  - 854686f4edf4 ("lib: add testing module for UBSAN")
>    since they add a module-only test functionality.
>  - any Documentation commits.
> 
> Since dump_stack() evaluates to NOOP in U-Boot, the UBSAN report
> retains only the header from the original kernel report.
> 
> As example, below is a UB found in U-Boot thanks to UBSAN:
> 
> ====================================================================
> UBSAN: Undefined behaviour in drivers/net/phy/phy.c:728:19
> left shift of 1 by 31 places cannot be represented in type 'int'
> ====================================================================
> 
> For comparison, below is a full-fledged kernel UBSAN report, based on
> Linux kernel commit 0dfc0c792d69 ("iommu/vt-d: fix shift-out-of-bounds
> in bug checking"):
> 
> ================================================================================
> UBSAN: Undefined behaviour in drivers/iommu/dmar.c:1348:3
> shift exponent 64 is too large for 32-bit type 'int'
> CPU: 2 PID: 0 Comm: swapper/2 Tainted: G     U            4.17.0-rc1+ #89
> Hardware name: Dell Inc. OptiPlex 7040/0Y7WYT, BIOS 1.2.8 01/26/2016
> Call Trace:
>  <IRQ>
>  dump_stack+0x90/0xfb
>  ubsan_epilogue+0x9/0x40
>  __ubsan_handle_shift_out_of_bounds+0x10e/0x170
>  ? qi_flush_dev_iotlb+0x124/0x180
> ------[snip]-----
>  apic_timer_interrupt+0xf/0x20
>  </IRQ>
> RIP: 0010:poll_idle+0x60/0xe7
> RSP: 0018:ffffb1b201943e30 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
> RAX: 0000000080200000 RBX: 000000000000008e RCX: 000000000000001f
> RDX: 0000000000000000 RSI: 000000002819aa06 RDI: 0000000000000000
> RBP: ffff9e93c6b33280 R08: 00000010f717d567 R09: 000000000010d205
> R10: ffffb1b201943df8 R11: 0000000000000001 R12: 00000000e01b169d
> R13: 0000000000000000 R14: ffffffffb12aa400 R15: 0000000000000000
>  cpuidle_enter_state+0xb4/0x470
>  do_idle+0x222/0x310
>  cpu_startup_entry+0x78/0x90
>  start_secondary+0x205/0x2e0
>  secondary_startup_64+0xa5/0xb0
> ================================================================================
> 
> To enable UBSAN, two prerequisites must be met from Kconfig perspective:
>  - ARCH has to select CONFIG_ARCH_HAS_UBSAN_SANITIZE_ALL
>  - defconfig has to enable CONFIG_UBSAN
> 
> This commit selects CONFIG_ARCH_HAS_UBSAN_SANITIZE_ALL for SANDBOX and
> ARM64 (r8a7795_salvator-x_defconfig is the only tested ARM64 platform).
> No defconfig changes are expected, since UBSAN is a development (not
> production) option. With CONFIG_UBSAN disabled, no functional change
> is expected from this commit.
> 
> The size increase of sanbox U-Boot (gcc 8.1.0):
> $ size u-boot.sandbox.*
>    text	   data	    bss	    dec	    hex	filename
> 1234958	  80048	 291472	1606478	 18834e	u-boot.sandbox.default
> 1422710	 272240	 291472	1986422	 1e4f76	u-boot.sandbox.ubsan
> +187752 +192192       0 +379944
> 
> The size increase of H3 Salvator-X U-Boot (aarch64-linux-gnu-gcc 7.2.1):
> $ size u-boot.r8a7795-salvator-x.*
>    text	   data	    bss	    dec	    hex	filename
>  589954	  23504	 263984	 877442	  d6382	u-boot.r8a7795-salvator-x.default
>  810968	 103304	 263984	1178256	 11fa90	u-boot.r8a7795-salvator-x.ubsan
> +221014  +79800       0 +300814

Can we re-work this so that there isn't a size increase unless UBSAN is
enabled?  I ask since I think for a v2 we should be able to say more
broadly that just about everyone can enable this, but only out of the
box sandbox should.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180819/e538e2ff/attachment.sig>