Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1291824 > unrolled thread

[PATCH] powerpc: Enable UBSAN support

Started byDaniel Axtens <dja@axtens.net>
First post2015-12-15 04:50 +0100
Last post2015-12-18 12:50 +0100
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] powerpc: Enable UBSAN support Daniel Axtens <dja@axtens.net> - 2015-12-15 04:50 +0100
    Re: [PATCH] powerpc: Enable UBSAN support Michael Ellerman <mpe@ellerman.id.au> - 2015-12-15 05:40 +0100
      Re: [PATCH] powerpc: Enable UBSAN support Daniel Axtens <dja@axtens.net> - 2015-12-15 06:20 +0100
        Re: [PATCH] powerpc: Enable UBSAN support Daniel Axtens <dja@axtens.net> - 2015-12-15 07:50 +0100
    Re: [PATCH] powerpc: Enable UBSAN support Michael Ellerman <mpe@ellerman.id.au> - 2015-12-18 12:50 +0100

#1291824 — [PATCH] powerpc: Enable UBSAN support

FromDaniel Axtens <dja@axtens.net>
Date2015-12-15 04:50 +0100
Subject[PATCH] powerpc: Enable UBSAN support
Message-ID<qFOMF-733-9@gated-at.bofh.it>
This hooks up UBSAN support for PowerPC.

So far it's found some interesting cases where we don't properly sanitise
input to shifts, including one in our futex handling. It's also found an
out of bounds read in an array. Nothing critical, but worth fixing.

Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
CC: Andrey Ryabinin <aryabinin@virtuozzo.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---

RFC -> v1:
 - Update patch to use fixed spelling of SANITIZE.
 - Include tested by tag from Andrew - Thanks!

This applies on top of next with Andrey's patches:
 1) https://patchwork.kernel.org/patch/7761341/
 2) https://patchwork.kernel.org/patch/7761351/
 3) https://patchwork.kernel.org/patch/7761361/
 4) https://patchwork.kernel.org/patch/7785791/
 5) https://patchwork.kernel.org/patch/7819661/

-mm and therefore -next have these patches, and the RFC of this
patch.

This has now been tested on LE and BE 64bit, on pseries, bml and
PowerNV.
---
 arch/powerpc/Kconfig                | 1 +
 arch/powerpc/kernel/Makefile        | 8 +++++++-
 arch/powerpc/kernel/vdso32/Makefile | 1 +
 arch/powerpc/kernel/vdso64/Makefile | 1 +
 arch/powerpc/xmon/Makefile          | 1 +
 5 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 25283796a02e..171d4e4b015d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -156,6 +156,7 @@ config PPC
 	select EDAC_ATOMIC_SCRUB
 	select ARCH_HAS_DMA_SET_COHERENT_MASK
 	select HAVE_ARCH_SECCOMP_FILTER
+	select ARCH_HAS_UBSAN_SANITIZE_ALL
 
 config GENERIC_CSUM
 	def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index ba336930d448..794f22adf99d 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -136,12 +136,18 @@ endif
 obj-$(CONFIG_EPAPR_PARAVIRT)	+= epapr_paravirt.o epapr_hcalls.o
 obj-$(CONFIG_KVM_GUEST)		+= kvm.o kvm_emul.o
 
-# Disable GCOV in odd or sensitive code
+# Disable GCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
+UBSAN_SANITIZE_prom_init.o := n
 GCOV_PROFILE_ftrace.o := n
+UBSAN_SANITIZE_ftrace.o := n
 GCOV_PROFILE_machine_kexec_64.o := n
+UBSAN_SANITIZE_machine_kexec_64.o := n
 GCOV_PROFILE_machine_kexec_32.o := n
+UBSAN_SANITIZE_machine_kexec_32.o := n
 GCOV_PROFILE_kprobes.o := n
+UBSAN_SANITIZE_kprobes.o := n
+UBSAN_SANITIZE_vdso.o := n
 
 extra-$(CONFIG_PPC_FPU)		+= fpu.o
 extra-$(CONFIG_ALTIVEC)		+= vector.o
diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index 6abffb7a8cd9..cbabd143acae 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -15,6 +15,7 @@ targets := $(obj-vdso32) vdso32.so vdso32.so.dbg
 obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
 
 GCOV_PROFILE := n
+UBSAN_SANITIZE := n
 
 ccflags-y := -shared -fno-common -fno-builtin
 ccflags-y += -nostdlib -Wl,-soname=linux-vdso32.so.1 \
diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
index 8c8f2ae43935..c710802b8fb6 100644
--- a/arch/powerpc/kernel/vdso64/Makefile
+++ b/arch/powerpc/kernel/vdso64/Makefile
@@ -8,6 +8,7 @@ targets := $(obj-vdso64) vdso64.so vdso64.so.dbg
 obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
 
 GCOV_PROFILE := n
+UBSAN_SANITIZE := n
 
 ccflags-y := -shared -fno-common -fno-builtin
 ccflags-y += -nostdlib -Wl,-soname=linux-vdso64.so.1 \
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index 1278788d96e3..436062dbb6e2 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -3,6 +3,7 @@
 subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
 
 GCOV_PROFILE := n
+UBSAN_SANITIZE := n
 
 ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
 
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1291836

FromMichael Ellerman <mpe@ellerman.id.au>
Date2015-12-15 05:40 +0100
Message-ID<qFPz4-7z1-9@gated-at.bofh.it>
In reply to#1291824
Hi Daniel,

Great work thanks for getting this going.

On Tue, 2015-12-15 at 14:46 +1100, Daniel Axtens wrote:

> This hooks up UBSAN support for PowerPC.
> 
> So far it's found some interesting cases where we don't properly sanitise
> input to shifts, including one in our futex handling. It's also found an
> out of bounds read in an array. Nothing critical, but worth fixing.
> 
> Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> CC: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
> 
> RFC -> v1:
>  - Update patch to use fixed spelling of SANITIZE.
>  - Include tested by tag from Andrew - Thanks!
> 
> This applies on top of next with Andrey's patches:
>  1) https://patchwork.kernel.org/patch/7761341/
>  2) https://patchwork.kernel.org/patch/7761351/
>  3) https://patchwork.kernel.org/patch/7761361/
>  4) https://patchwork.kernel.org/patch/7785791/
>  5) https://patchwork.kernel.org/patch/7819661/
> 
> -mm and therefore -next have these patches, and the RFC of this
> patch.
> 
> This has now been tested on LE and BE 64bit, on pseries, bml and
> PowerNV.


Have you tried running with KVM?

I'm wondering if we should be excluding some of the KVM code that runs in real mode, eg:

  arch/powerpc/kvm/book3s_hv_rm_mmu.c
  arch/powerpc/kvm/book3s_hv_rm_xics.c

And maybe some other bits.

Also the early setup code, a/p/k/setup*.c might be dicey.

In all of the above it's probably OK unless you actually hit a warning at the
wrong point, so testing will probably not find problems. Although I guess we
could add some deliberatly incorrect code at certain points and check we
survive the warning.

Is there an easy way to spot the calls to UBSAN in the generated code?

cheers

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291846

FromDaniel Axtens <dja@axtens.net>
Date2015-12-15 06:20 +0100
Message-ID<qFQbL-83p-1@gated-at.bofh.it>
In reply to#1291836

[Multipart message — attachments visible in raw view] — view raw

Michael Ellerman <mpe@ellerman.id.au> writes:

> Hi Daniel,
>
> Great work thanks for getting this going.
>

> Have you tried running with KVM?
>
> I'm wondering if we should be excluding some of the KVM code that runs in real mode, eg:
>
>   arch/powerpc/kvm/book3s_hv_rm_mmu.c
>   arch/powerpc/kvm/book3s_hv_rm_xics.c
>
> And maybe some other bits.
>
> Also the early setup code, a/p/k/setup*.c might be dicey.

My philosphy was just to copy the GCOV excludes, although you're right
that perhaps we want to be more aggressive in excluding here given that
the UBSAN handlers print a bunch of stuff. I'm happy to respin with
further exclusions - they're really easy to add.

> In all of the above it's probably OK unless you actually hit a warning at the
> wrong point, so testing will probably not find problems. Although I guess we
> could add some deliberatly incorrect code at certain points and check we
> survive the warning.

Yep. I'll run a kvm guest on an instrumented kernel and let you know
what happens!

>
> Is there an easy way to spot the calls to UBSAN in the generated code?

Yes - because of the handler functions, they're *really* easy to spot.
Here's some assembly for GregorianDay():


c00000000002924c:       6d 26 7e 48     bl      c00000000080b8b8 <__ubsan_handle_mul_overflow+0x8>
c000000000029250:       00 00 00 60     nop
c000000000029254:       38 fe ff 4b     b       c00000000002908c <GregorianDay+0xdc>
c000000000029258:       8c ff 62 3c     addis   r3,r2,-116
c00000000002925c:       01 00 a0 38     li      r5,1
c000000000029260:       78 db 64 7f     mr      r4,r27
c000000000029264:       50 9c 63 38     addi    r3,r3,-25520
c000000000029268:       41 26 7e 48     bl      c00000000080b8a8 <__ubsan_handle_sub_overflow+0x8>
c00000000002926c:       00 00 00 60     nop
c000000000029270:       44 fe ff 4b     b       c0000000000290b4 <GregorianDay+0x104>
c000000000029274:       8c ff 62 3c     addis   r3,r2,-116
c000000000029278:       78 d3 45 7f     mr      r5,r26
c00000000002927c:       e0 9c 63 38     addi    r3,r3,-25376
c000000000029280:       19 26 7e 48     bl      c00000000080b898 <__ubsan_handle_add_overflow+0x8>
c000000000029284:       00 00 00 60     nop
c000000000029288:       94 fe ff 4b     b       c00000000002911c <GregorianDay+0x16c>
c00000000002928c:       8c ff 62 3c     addis   r3,r2,-116
c000000000029290:       78 f3 c4 7f     mr      r4,r30
c000000000029294:       68 9c 63 38     addi    r3,r3,-25496
c000000000029298:       b1 21 7e 48     bl      c00000000080b448 <__ubsan_handle_out_of_bounds+0x8>
c00000000002929c:       00 00 00 60     nop
c0000000000292a0:       20 fe ff 4b     b       c0000000000290c0 <GregorianDay+0x110>
c0000000000292a4:       14 4a 24 7d     add     r9,r4,r9
c0000000000292a8:       40 48 a4 7f     cmpld   cr7,r4,r9
c0000000000292ac:       2c fe fd 41     bgt+    cr7,c0000000000290d8 <GregorianDay+0x128>

You can see that there are lots of calls to __ubsan_handle_blah_blah
inserted.

Regards,
Daniel

>
> cheers
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1291889

FromDaniel Axtens <dja@axtens.net>
Date2015-12-15 07:50 +0100
Message-ID<qFRAR-pw-1@gated-at.bofh.it>
In reply to#1291846

[Multipart message — attachments visible in raw view] — view raw

>> Have you tried running with KVM?

OK, I ran a UBSAN kvm guest on a UBSAN host and I ran trinity inside the
guest. It all appears to work as usual.

I really don't have the knowledge to determine what we should exclude,
but I'm very very happy to respin with a greater list of excludes if you
think that's a good idea.

Regards,
Daniel


>>
>> I'm wondering if we should be excluding some of the KVM code that runs in real mode, eg:
>>
>>   arch/powerpc/kvm/book3s_hv_rm_mmu.c
>>   arch/powerpc/kvm/book3s_hv_rm_xics.c
>>
>> And maybe some other bits.
>>
>> Also the early setup code, a/p/k/setup*.c might be dicey.
>
> My philosphy was just to copy the GCOV excludes, although you're right
> that perhaps we want to be more aggressive in excluding here given that
> the UBSAN handlers print a bunch of stuff. I'm happy to respin with
> further exclusions - they're really easy to add.
>
>> In all of the above it's probably OK unless you actually hit a warning at the
>> wrong point, so testing will probably not find problems. Although I guess we
>> could add some deliberatly incorrect code at certain points and check we
>> survive the warning.
>
> Yep. I'll run a kvm guest on an instrumented kernel and let you know
> what happens!
>
>>
>> Is there an easy way to spot the calls to UBSAN in the generated code?
>
> Yes - because of the handler functions, they're *really* easy to spot.
> Here's some assembly for GregorianDay():
>
>
> c00000000002924c:       6d 26 7e 48     bl      c00000000080b8b8 <__ubsan_handle_mul_overflow+0x8>
> c000000000029250:       00 00 00 60     nop
> c000000000029254:       38 fe ff 4b     b       c00000000002908c <GregorianDay+0xdc>
> c000000000029258:       8c ff 62 3c     addis   r3,r2,-116
> c00000000002925c:       01 00 a0 38     li      r5,1
> c000000000029260:       78 db 64 7f     mr      r4,r27
> c000000000029264:       50 9c 63 38     addi    r3,r3,-25520
> c000000000029268:       41 26 7e 48     bl      c00000000080b8a8 <__ubsan_handle_sub_overflow+0x8>
> c00000000002926c:       00 00 00 60     nop
> c000000000029270:       44 fe ff 4b     b       c0000000000290b4 <GregorianDay+0x104>
> c000000000029274:       8c ff 62 3c     addis   r3,r2,-116
> c000000000029278:       78 d3 45 7f     mr      r5,r26
> c00000000002927c:       e0 9c 63 38     addi    r3,r3,-25376
> c000000000029280:       19 26 7e 48     bl      c00000000080b898 <__ubsan_handle_add_overflow+0x8>
> c000000000029284:       00 00 00 60     nop
> c000000000029288:       94 fe ff 4b     b       c00000000002911c <GregorianDay+0x16c>
> c00000000002928c:       8c ff 62 3c     addis   r3,r2,-116
> c000000000029290:       78 f3 c4 7f     mr      r4,r30
> c000000000029294:       68 9c 63 38     addi    r3,r3,-25496
> c000000000029298:       b1 21 7e 48     bl      c00000000080b448 <__ubsan_handle_out_of_bounds+0x8>
> c00000000002929c:       00 00 00 60     nop
> c0000000000292a0:       20 fe ff 4b     b       c0000000000290c0 <GregorianDay+0x110>
> c0000000000292a4:       14 4a 24 7d     add     r9,r4,r9
> c0000000000292a8:       40 48 a4 7f     cmpld   cr7,r4,r9
> c0000000000292ac:       2c fe fd 41     bgt+    cr7,c0000000000290d8 <GregorianDay+0x128>
>
> You can see that there are lots of calls to __ubsan_handle_blah_blah
> inserted.
>
> Regards,
> Daniel
>
>>
>> cheers
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1294713

FromMichael Ellerman <mpe@ellerman.id.au>
Date2015-12-18 12:50 +0100
Message-ID<qH1HP-55K-9@gated-at.bofh.it>
In reply to#1291824
On Tue, 2015-12-15 at 14:46 +1100, Daniel Axtens wrote:

> This hooks up UBSAN support for PowerPC.
> 
> So far it's found some interesting cases where we don't properly sanitise
> input to shifts, including one in our futex handling. It's also found an
> out of bounds read in an array. Nothing critical, but worth fixing.
> 
> Tested-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> CC: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
> 
> RFC -> v1:
>  - Update patch to use fixed spelling of SANITIZE.
>  - Include tested by tag from Andrew - Thanks!
> 
> This applies on top of next with Andrey's patches:
>  1) https://patchwork.kernel.org/patch/7761341/
>  2) https://patchwork.kernel.org/patch/7761351/
>  3) https://patchwork.kernel.org/patch/7761361/
>  4) https://patchwork.kernel.org/patch/7785791/
>  5) https://patchwork.kernel.org/patch/7819661/
> 
> -mm and therefore -next have these patches, and the RFC of this
> patch.

Thanks Daniel, looks good, if there's other files that need excluding we can
add them later.

akpm please take this version into mm.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers


> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 25283796a02e..171d4e4b015d 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -156,6 +156,7 @@ config PPC
>  	select EDAC_ATOMIC_SCRUB
>  	select ARCH_HAS_DMA_SET_COHERENT_MASK
>  	select HAVE_ARCH_SECCOMP_FILTER
> +	select ARCH_HAS_UBSAN_SANITIZE_ALL
>  
>  config GENERIC_CSUM
>  	def_bool CPU_LITTLE_ENDIAN
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index ba336930d448..794f22adf99d 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -136,12 +136,18 @@ endif
>  obj-$(CONFIG_EPAPR_PARAVIRT)	+= epapr_paravirt.o epapr_hcalls.o
>  obj-$(CONFIG_KVM_GUEST)		+= kvm.o kvm_emul.o
>  
> -# Disable GCOV in odd or sensitive code
> +# Disable GCOV & sanitizers in odd or sensitive code
>  GCOV_PROFILE_prom_init.o := n
> +UBSAN_SANITIZE_prom_init.o := n
>  GCOV_PROFILE_ftrace.o := n
> +UBSAN_SANITIZE_ftrace.o := n
>  GCOV_PROFILE_machine_kexec_64.o := n
> +UBSAN_SANITIZE_machine_kexec_64.o := n
>  GCOV_PROFILE_machine_kexec_32.o := n
> +UBSAN_SANITIZE_machine_kexec_32.o := n
>  GCOV_PROFILE_kprobes.o := n
> +UBSAN_SANITIZE_kprobes.o := n
> +UBSAN_SANITIZE_vdso.o := n
>  
>  extra-$(CONFIG_PPC_FPU)		+= fpu.o
>  extra-$(CONFIG_ALTIVEC)		+= vector.o
> diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
> index 6abffb7a8cd9..cbabd143acae 100644
> --- a/arch/powerpc/kernel/vdso32/Makefile
> +++ b/arch/powerpc/kernel/vdso32/Makefile
> @@ -15,6 +15,7 @@ targets := $(obj-vdso32) vdso32.so vdso32.so.dbg
>  obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
>  
>  GCOV_PROFILE := n
> +UBSAN_SANITIZE := n
>  
>  ccflags-y := -shared -fno-common -fno-builtin
>  ccflags-y += -nostdlib -Wl,-soname=linux-vdso32.so.1 \
> diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
> index 8c8f2ae43935..c710802b8fb6 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -8,6 +8,7 @@ targets := $(obj-vdso64) vdso64.so vdso64.so.dbg
>  obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
>  
>  GCOV_PROFILE := n
> +UBSAN_SANITIZE := n
>  
>  ccflags-y := -shared -fno-common -fno-builtin
>  ccflags-y += -nostdlib -Wl,-soname=linux-vdso64.so.1 \
> diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
> index 1278788d96e3..436062dbb6e2 100644
> --- a/arch/powerpc/xmon/Makefile
> +++ b/arch/powerpc/xmon/Makefile
> @@ -3,6 +3,7 @@
>  subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>  
>  GCOV_PROFILE := n
> +UBSAN_SANITIZE := n
>  
>  ccflags-$(CONFIG_PPC64) := $(NO_MINIMAL_TOC)
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web