ARM: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL

Commit Message

Florian Fainelli Sept. 28, 2022, 5:47 p.m. UTC

From: Seung-Woo Kim <[e-mail]>

To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
from arm confiuration. Basic kernel bootup test is passed on arm with
CONFIG_UBSAN_SANITIZE_ALL enabled.

Signed-off-by: Seung-Woo Kim <[e-mail]>
[florian: rebased against v6.0-rc7]
Signed-off-by: Florian Fainelli <[e-mail]>
---
 arch/arm/Kconfig                  | 1 +
 arch/arm/boot/compressed/Makefile | 1 +
 arch/arm/vdso/Makefile            | 1 +
 3 files changed, 3 insertions(+)

Comments

Kees Cook Sept. 28, 2022, 6:01 p.m. UTC | #1

On Wed, Sep 28, 2022 at 10:47:39AM -0700, Florian Fainelli wrote:
> From: Seung-Woo Kim <[e-mail]>
> 
> To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
> from arm confiuration. Basic kernel bootup test is passed on arm with
> CONFIG_UBSAN_SANITIZE_ALL enabled.
> 
> Signed-off-by: Seung-Woo Kim <[e-mail]>
> [florian: rebased against v6.0-rc7]
> Signed-off-by: Florian Fainelli <[e-mail]>

Ah-ha, thanks for testing this. What devices did you check this on? I
know boot-up on arm32 can be very device-specific.

Which UBSAN configs did you try?

Thanks!

-Kees

Florian Fainelli Sept. 28, 2022, 11:06 p.m. UTC | #2

On 9/28/22 11:01, Kees Cook wrote:
> On Wed, Sep 28, 2022 at 10:47:39AM -0700, Florian Fainelli wrote:
>> From: Seung-Woo Kim <[e-mail]>
>>
>> To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
>> from arm confiuration. Basic kernel bootup test is passed on arm with
>> CONFIG_UBSAN_SANITIZE_ALL enabled.
>>
>> Signed-off-by: Seung-Woo Kim <[e-mail]>
>> [florian: rebased against v6.0-rc7]
>> Signed-off-by: Florian Fainelli <[e-mail]>
> 
> Ah-ha, thanks for testing this. What devices did you check this on? I
> know boot-up on arm32 can be very device-specific.

This was tested on an ARCH_BRCMSTB system which is using an ARMv8 CPU 
booted in AArch32 mode, so virtually equivalent to armv7l. A raspberry 
Pi 4B is also happily booting with it.

> 
> Which UBSAN configs did you try?

All CONFIG_UBSAN_* work with the exception of CONFIG_UBSAN_ALIGNMENT on 
my ARCH_BRCMSTB system, however it works fine on the Raspberry Pi 4B.
Florian

William Zhang Sept. 29, 2022, 12:33 a.m. UTC | #3

On 09/28/2022 04:06 PM, Florian Fainelli wrote:
> On 9/28/22 11:01, Kees Cook wrote:
>> On Wed, Sep 28, 2022 at 10:47:39AM -0700, Florian Fainelli wrote:
>>> From: Seung-Woo Kim <[e-mail]>
>>>
>>> To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
>>> from arm confiuration. Basic kernel bootup test is passed on arm with
>>> CONFIG_UBSAN_SANITIZE_ALL enabled.
>>>
>>> Signed-off-by: Seung-Woo Kim <[e-mail]>
>>> [florian: rebased against v6.0-rc7]
>>> Signed-off-by: Florian Fainelli <[e-mail]>
>>
>> Ah-ha, thanks for testing this. What devices did you check this on? I
>> know boot-up on arm32 can be very device-specific.
> 
> This was tested on an ARCH_BRCMSTB system which is using an ARMv8 CPU 
> booted in AArch32 mode, so virtually equivalent to armv7l. A raspberry 
> Pi 4B is also happily booting with it.
> 
>>
>> Which UBSAN configs did you try?
> 
> All CONFIG_UBSAN_* work with the exception of CONFIG_UBSAN_ALIGNMENT on 
> my ARCH_BRCMSTB system, however it works fine on the Raspberry Pi 4B.
> Florian

I also tested on a BCM63138 board (ARM A9) under ARCH_BCMBCA using the 
multi_v7_defconfig with all the UBSAN configs enabled except 
UBSAN_ALIGNMENT and board boots up fine. Turning on UBSAN_ALIGNMENT 
results in flood of false positive misaligned-access warnings. This is 
fine as ARM supports unaligned access.

It did catch an out-of-band bug in mach-sunxi smp code.  I will submit a 
separate patch to fix that bug.

Tested-by: William Zhang <[e-mail]>

Kees Cook Sept. 29, 2022, 8:10 a.m. UTC | #4

On Wed, Sep 28, 2022 at 05:33:14PM -0700, William Zhang wrote:
> 
> 
> On 09/28/2022 04:06 PM, Florian Fainelli wrote:
> > On 9/28/22 11:01, Kees Cook wrote:
> > > On Wed, Sep 28, 2022 at 10:47:39AM -0700, Florian Fainelli wrote:
> > > > From: Seung-Woo Kim <[e-mail]>
> > > > 
> > > > To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
> > > > from arm confiuration. Basic kernel bootup test is passed on arm with
> > > > CONFIG_UBSAN_SANITIZE_ALL enabled.
> > > > 
> > > > Signed-off-by: Seung-Woo Kim <[e-mail]>
> > > > [florian: rebased against v6.0-rc7]
> > > > Signed-off-by: Florian Fainelli <[e-mail]>
> > > 
> > > Ah-ha, thanks for testing this. What devices did you check this on? I
> > > know boot-up on arm32 can be very device-specific.
> > 
> > This was tested on an ARCH_BRCMSTB system which is using an ARMv8 CPU
> > booted in AArch32 mode, so virtually equivalent to armv7l. A raspberry
> > Pi 4B is also happily booting with it.
> > 
> > > 
> > > Which UBSAN configs did you try?
> > 
> > All CONFIG_UBSAN_* work with the exception of CONFIG_UBSAN_ALIGNMENT on
> > my ARCH_BRCMSTB system, however it works fine on the Raspberry Pi 4B.
> > Florian
> 
> I also tested on a BCM63138 board (ARM A9) under ARCH_BCMBCA using the
> multi_v7_defconfig with all the UBSAN configs enabled except UBSAN_ALIGNMENT
> and board boots up fine. Turning on UBSAN_ALIGNMENT results in flood of
> false positive misaligned-access warnings. This is fine as ARM supports
> unaligned access.
> 
> It did catch an out-of-band bug in mach-sunxi smp code.  I will submit a
> separate patch to fix that bug.

Yay! :) Move coverage is great. :)

> 
> Tested-by: William Zhang <[e-mail]>

Thanks!

Florian Fainelli Sept. 30, 2022, 9:34 p.m. UTC | #5

On 9/29/22 01:10, Kees Cook wrote:
> On Wed, Sep 28, 2022 at 05:33:14PM -0700, William Zhang wrote:
>>
>>
>> On 09/28/2022 04:06 PM, Florian Fainelli wrote:
>>> On 9/28/22 11:01, Kees Cook wrote:
>>>> On Wed, Sep 28, 2022 at 10:47:39AM -0700, Florian Fainelli wrote:
>>>>> From: Seung-Woo Kim <[e-mail]>
>>>>>
>>>>> To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
>>>>> from arm confiuration. Basic kernel bootup test is passed on arm with
>>>>> CONFIG_UBSAN_SANITIZE_ALL enabled.
>>>>>
>>>>> Signed-off-by: Seung-Woo Kim <[e-mail]>
>>>>> [florian: rebased against v6.0-rc7]
>>>>> Signed-off-by: Florian Fainelli <[e-mail]>
>>>>
>>>> Ah-ha, thanks for testing this. What devices did you check this on? I
>>>> know boot-up on arm32 can be very device-specific.
>>>
>>> This was tested on an ARCH_BRCMSTB system which is using an ARMv8 CPU
>>> booted in AArch32 mode, so virtually equivalent to armv7l. A raspberry
>>> Pi 4B is also happily booting with it.
>>>
>>>>
>>>> Which UBSAN configs did you try?
>>>
>>> All CONFIG_UBSAN_* work with the exception of CONFIG_UBSAN_ALIGNMENT on
>>> my ARCH_BRCMSTB system, however it works fine on the Raspberry Pi 4B.
>>> Florian
>>
>> I also tested on a BCM63138 board (ARM A9) under ARCH_BCMBCA using the
>> multi_v7_defconfig with all the UBSAN configs enabled except UBSAN_ALIGNMENT
>> and board boots up fine. Turning on UBSAN_ALIGNMENT results in flood of
>> false positive misaligned-access warnings. This is fine as ARM supports
>> unaligned access.
>>
>> It did catch an out-of-band bug in mach-sunxi smp code.  I will submit a
>> separate patch to fix that bug.
> 
> Yay! :) Move coverage is great. :)
> 
>>
>> Tested-by: William Zhang <[e-mail]>

Submitted to the patch tracker:

https://www.armlinux.org.uk/developer/patches/viewpatch.php?id=9253/1

Linus Walleij Oct. 4, 2022, 8:51 a.m. UTC | #6

On Wed, Sep 28, 2022 at 7:47 PM Florian Fainelli <[e-mail]> wrote:

> From: Seung-Woo Kim <[e-mail]>
>
> To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
> from arm confiuration. Basic kernel bootup test is passed on arm with
> CONFIG_UBSAN_SANITIZE_ALL enabled.
>
> Signed-off-by: Seung-Woo Kim <[e-mail]>
> [florian: rebased against v6.0-rc7]
> Signed-off-by: Florian Fainelli <[e-mail]>

Pretty cool that it "just works", I was thinking to myself that I
should try this one
day. Sorry I was late to look at the patch.

Have you also looked at KCSAN?

Yours,
Linus Walleij

Florian Fainelli Oct. 12, 2022, 7:46 p.m. UTC | #7

On 10/4/22 01:51, Linus Walleij wrote:
> On Wed, Sep 28, 2022 at 7:47 PM Florian Fainelli <[e-mail]> wrote:
> 
>> From: Seung-Woo Kim <[e-mail]>
>>
>> To enable UBSAN on ARM, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL
>> from arm confiuration. Basic kernel bootup test is passed on arm with
>> CONFIG_UBSAN_SANITIZE_ALL enabled.
>>
>> Signed-off-by: Seung-Woo Kim <[e-mail]>
>> [florian: rebased against v6.0-rc7]
>> Signed-off-by: Florian Fainelli <[e-mail]>
> 
> Pretty cool that it "just works", I was thinking to myself that I
> should try this one
> day. Sorry I was late to look at the patch.
> 
> Have you also looked at KCSAN?

Not yet, that would be next. Most often, at least for ARCH_BRCMSTB the 
same hardware and set of drivers run on ARM64 which is typically much 
more feature full, so we get coverage on *SAN features that way.
--
Florian

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 87badeae3181..c90aa58eab7f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -27,6 +27,7 @@  config ARM
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG if CPU_V7 || CPU_V7M || CPU_V6K
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select ARCH_KEEP_MEMBLOCK
+	select ARCH_HAS_UBSAN_SANITIZE_ALL
 	select ARCH_MIGHT_HAVE_PC_PARPORT
 	select ARCH_NO_SG_CHAIN if !ARM_HAS_SG_CHAIN
 	select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 41bcbb460fac..2cc2af13779e 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -27,6 +27,7 @@  KASAN_SANITIZE		:= n
 
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT		:= n
+UBSAN_SANITIZE		:= n
 
 #
 # Architecture dependencies
diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile
index 8ca1c9f262a2..a7ec06ce3785 100644
--- a/arch/arm/vdso/Makefile
+++ b/arch/arm/vdso/Makefile
@@ -37,6 +37,7 @@  endif
 
 # Disable gcov profiling for VDSO code
 GCOV_PROFILE := n
+UBSAN_SANITIZE := n
 
 # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
 KCOV_INSTRUMENT := n