linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc: Enable UBSAN support
@ 2015-12-15  3:46 Daniel Axtens
  2015-12-15  4:35 ` Michael Ellerman
  2015-12-18 11:48 ` Michael Ellerman
  0 siblings, 2 replies; 5+ messages in thread
From: Daniel Axtens @ 2015-12-15  3:46 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: mpe, linux-kernel, andrew.donnellan, akpm, Daniel Axtens,
	Andrey Ryabinin

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc: Enable UBSAN support
  2015-12-15  3:46 [PATCH] powerpc: Enable UBSAN support Daniel Axtens
@ 2015-12-15  4:35 ` Michael Ellerman
  2015-12-15  5:11   ` Daniel Axtens
  2015-12-18 11:48 ` Michael Ellerman
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2015-12-15  4:35 UTC (permalink / raw)
  To: Daniel Axtens, linuxppc-dev
  Cc: linux-kernel, andrew.donnellan, akpm, Andrey Ryabinin

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc: Enable UBSAN support
  2015-12-15  4:35 ` Michael Ellerman
@ 2015-12-15  5:11   ` Daniel Axtens
  2015-12-15  6:46     ` Daniel Axtens
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Axtens @ 2015-12-15  5:11 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
  Cc: linux-kernel, andrew.donnellan, akpm, Andrey Ryabinin

[-- Attachment #1: Type: text/plain, Size: 3272 bytes --]

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/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 859 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc: Enable UBSAN support
  2015-12-15  5:11   ` Daniel Axtens
@ 2015-12-15  6:46     ` Daniel Axtens
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Axtens @ 2015-12-15  6:46 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
  Cc: linux-kernel, andrew.donnellan, akpm, Andrey Ryabinin

[-- Attachment #1: Type: text/plain, Size: 3576 bytes --]


>> 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/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 859 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc: Enable UBSAN support
  2015-12-15  3:46 [PATCH] powerpc: Enable UBSAN support Daniel Axtens
  2015-12-15  4:35 ` Michael Ellerman
@ 2015-12-18 11:48 ` Michael Ellerman
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2015-12-18 11:48 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, andrew.donnellan, Andrey Ryabinin, Daniel Axtens,
	linuxppc-dev list

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)
>  


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-12-18 11:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-15  3:46 [PATCH] powerpc: Enable UBSAN support Daniel Axtens
2015-12-15  4:35 ` Michael Ellerman
2015-12-15  5:11   ` Daniel Axtens
2015-12-15  6:46     ` Daniel Axtens
2015-12-18 11:48 ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).