patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support
@ 2024-10-08  4:22 Nathan Chancellor
  2024-10-08  4:22 ` [PATCH 1/3] powerpc: Fix stack protector Kconfig test for clang Nathan Chancellor
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Nathan Chancellor @ 2024-10-08  4:22 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Christophe Leroy, Naveen N Rao,
	Madhavan Srinivasan, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Keith Packard, linuxppc-dev, llvm, patches,
	Nathan Chancellor

This series prepares the powerpc Kconfig and Kbuild files for clang's
per-task stack protector support. clang requires
'-mstack-protector-guard-offset' to always be passed with the other
'-mstack-protector-guard' flags, which does not always happen with the
powerpc implementation, unlike arm, arm64, and riscv implementations.
This series brings powerpc in line with those other architectures, which
allows clang's support to work right away when it is merged.
Additionally, there is one other fix needed for the Kconfig test to work
correctly when targeting 32-bit.

I have tested this series in QEMU against LKDTM's REPORT_STACK_CANARY
with ppc64le_guest_defconfig and pmac32_defconfig built with a toolchain
that contains Keith's in-progress pull request, which should land for
LLVM 20:

https://github.com/llvm/llvm-project/pull/110928

---
Nathan Chancellor (3):
      powerpc: Fix stack protector Kconfig test for clang
      powerpc: Adjust adding stack protector flags to KBUILD_CLAGS for clang
      powerpc: Include -m32 / -m64 for stack protector Kconfig test

 arch/powerpc/Kconfig  |  4 ++--
 arch/powerpc/Makefile | 15 ++++++---------
 2 files changed, 8 insertions(+), 11 deletions(-)
---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241004-powerpc-fix-stackprotector-test-clang-84e67ed82f62

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>


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

* [PATCH 1/3] powerpc: Fix stack protector Kconfig test for clang
  2024-10-08  4:22 [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Nathan Chancellor
@ 2024-10-08  4:22 ` Nathan Chancellor
  2024-10-08  4:22 ` [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS " Nathan Chancellor
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2024-10-08  4:22 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Christophe Leroy, Naveen N Rao,
	Madhavan Srinivasan, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Keith Packard, linuxppc-dev, llvm, patches,
	Nathan Chancellor

Clang's in-progress per-task stack protector support [1] does not work
with the current Kconfig checks because '-mstack-protector-guard-offset'
is not provided, unlike all other architecture Kconfig checks.

  $ fd Kconfig -x rg -l mstack-protector-guard-offset
  ./arch/arm/Kconfig
  ./arch/riscv/Kconfig
  ./arch/arm64/Kconfig

This produces an error from clang, which is interpreted as the flags not
being supported at all when they really are.

  $ clang --target=powerpc64-linux-gnu \
          -mstack-protector-guard=tls \
          -mstack-protector-guard-reg=r13 \
          -c -o /dev/null -x c /dev/null
  clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default

This argument will always be provided by the build system, so mirror
other architectures and use '-mstack-protector-guard-offset=0' for
testing support, which fixes the issue for clang and does not regress
support with GCC.

Link: https://github.com/llvm/llvm-project/pull/110928 [1]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/powerpc/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8094a01974cca1d27002720e706f66bec2a2d035..eb98050b8c016bb23887a9d669d29e69d933c9c8 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -275,8 +275,8 @@ config PPC
 	select HAVE_RSEQ
 	select HAVE_SETUP_PER_CPU_AREA		if PPC64
 	select HAVE_SOFTIRQ_ON_OWN_STACK
-	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2)
-	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13)
+	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
+	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
 	select HAVE_STATIC_CALL			if PPC32
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_VIRT_CPU_ACCOUNTING

-- 
2.47.0


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

* [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS for clang
  2024-10-08  4:22 [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Nathan Chancellor
  2024-10-08  4:22 ` [PATCH 1/3] powerpc: Fix stack protector Kconfig test for clang Nathan Chancellor
@ 2024-10-08  4:22 ` Nathan Chancellor
  2024-10-08  5:10   ` Christophe Leroy
  2024-10-08  4:22 ` [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test Nathan Chancellor
  2024-10-08 16:09 ` [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Keith Packard
  3 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2024-10-08  4:22 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Christophe Leroy, Naveen N Rao,
	Madhavan Srinivasan, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Keith Packard, linuxppc-dev, llvm, patches,
	Nathan Chancellor

After fixing the HAVE_STACKPROTECTER checks for clang's in-progress
per-task stack protector support [1], the build fails during prepare0
because '-mstack-protector-guard-offset' has not been added to
KBUILD_CFLAGS yet but the other '-mstack-protector-guard' flags have.

  clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
  clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
  make[4]: *** [scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
  make[4]: *** [scripts/Makefile.build:102: scripts/mod/devicetable-offsets.s] Error 1

Mirror other architectures and add all '-mstack-protector-guard' flags
to KBUILD_CFLAGS atomically during stack_protector_prepare, which
resolves the issue and allows clang's implementation to fully work with
the kernel.

Link: https://github.com/llvm/llvm-project/pull/110928 [1]
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/powerpc/Makefile | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index bbfe4a1f06ef9db9b2f2e48e02096b1e0500a14b..8f9e387c4c7dd06d8756dca3eac808cc344a937d 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -100,13 +100,6 @@ KBUILD_AFLAGS	+= -m$(BITS)
 KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
 endif
 
-cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
-ifdef CONFIG_PPC64
-cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r13
-else
-cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
-endif
-
 LDFLAGS_vmlinux-y := -Bstatic
 LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
 LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
@@ -402,9 +395,13 @@ prepare: stack_protector_prepare
 PHONY += stack_protector_prepare
 stack_protector_prepare: prepare0
 ifdef CONFIG_PPC64
-	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
+	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls \
+				-mstack-protector-guard-reg=r13 \
+				-mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
 else
-	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
+	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls \
+				-mstack-protector-guard-reg=r2 \
+				-mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
 endif
 endif
 

-- 
2.47.0


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

* [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test
  2024-10-08  4:22 [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Nathan Chancellor
  2024-10-08  4:22 ` [PATCH 1/3] powerpc: Fix stack protector Kconfig test for clang Nathan Chancellor
  2024-10-08  4:22 ` [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS " Nathan Chancellor
@ 2024-10-08  4:22 ` Nathan Chancellor
  2024-10-08  5:14   ` Christophe Leroy
  2024-10-08 16:09 ` [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Keith Packard
  3 siblings, 1 reply; 10+ messages in thread
From: Nathan Chancellor @ 2024-10-08  4:22 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Nicholas Piggin, Christophe Leroy, Naveen N Rao,
	Madhavan Srinivasan, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Keith Packard, linuxppc-dev, llvm, patches,
	Nathan Chancellor

Kbuild uses the powerpc64le-linux-gnu target for clang, which causes the
Kconfig check for 32-bit powerpc stack protector support to fail because
nothing flips the target to 32-bit:

  $ clang --target=powerpc64le-linux-gnu \
          -mstack-protector-guard=tls
          -mstack-protector-guard-reg=r2 \
          -mstack-protector-guard-offset=0 \
          -x c -c -o /dev/null /dev/null
  clang: error: invalid value 'r2' in 'mstack-protector-guard-reg=', expected one of: r13

Use the Kconfig macro '$(m32-flag)', which expands to '-m32' when
supported, in the stack protector support cc-option call to properly
switch the target to a 32-bit one, which matches what happens in Kbuild.
While the 64-bit macro does not strictly need it, add the equivalent
64-bit option for symmetry.

Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/powerpc/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index eb98050b8c016bb23887a9d669d29e69d933c9c8..6aaca48955a34b2a38af1415bfa36f74f35c3f3e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -275,8 +275,8 @@ config PPC
 	select HAVE_RSEQ
 	select HAVE_SETUP_PER_CPU_AREA		if PPC64
 	select HAVE_SOFTIRQ_ON_OWN_STACK
-	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
-	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
+	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,$(m32-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
+	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,$(m64-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
 	select HAVE_STATIC_CALL			if PPC32
 	select HAVE_SYSCALL_TRACEPOINTS
 	select HAVE_VIRT_CPU_ACCOUNTING

-- 
2.47.0


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

* Re: [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS for clang
  2024-10-08  4:22 ` [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS " Nathan Chancellor
@ 2024-10-08  5:10   ` Christophe Leroy
  2024-10-08 13:39     ` Nathan Chancellor
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2024-10-08  5:10 UTC (permalink / raw)
  To: Nathan Chancellor, Michael Ellerman
  Cc: Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Keith Packard,
	linuxppc-dev, llvm, patches



Le 08/10/2024 à 06:22, Nathan Chancellor a écrit :
> After fixing the HAVE_STACKPROTECTER checks for clang's in-progress
> per-task stack protector support [1], the build fails during prepare0
> because '-mstack-protector-guard-offset' has not been added to
> KBUILD_CFLAGS yet but the other '-mstack-protector-guard' flags have.
> 
>    clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
>    clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
>    make[4]: *** [scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
>    make[4]: *** [scripts/Makefile.build:102: scripts/mod/devicetable-offsets.s] Error 1
> 
> Mirror other architectures and add all '-mstack-protector-guard' flags
> to KBUILD_CFLAGS atomically during stack_protector_prepare, which
> resolves the issue and allows clang's implementation to fully work with
> the kernel.
> 
> Link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fpull%2F110928&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cc099707fa5464808c49108dce750d65e%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638639581583232977%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=b%2BcqodA6k739DD%2F74EJMXUtIHAZDt1DGcyd7mzzhbd8%3D&reserved=0 [1]
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Is it worth a Fixes: tag so that it gets applied on stable, or wont 
CLANG 20 be used with kernels older than 6.13 ?

> ---
>   arch/powerpc/Makefile | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index bbfe4a1f06ef9db9b2f2e48e02096b1e0500a14b..8f9e387c4c7dd06d8756dca3eac808cc344a937d 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -100,13 +100,6 @@ KBUILD_AFLAGS	+= -m$(BITS)
>   KBUILD_LDFLAGS	+= -m elf$(BITS)$(LDEMULATION)
>   endif
>   
> -cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard=tls
> -ifdef CONFIG_PPC64
> -cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r13
> -else
> -cflags-$(CONFIG_STACKPROTECTOR)	+= -mstack-protector-guard-reg=r2
> -endif
> -
>   LDFLAGS_vmlinux-y := -Bstatic
>   LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
>   LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) += -z notext
> @@ -402,9 +395,13 @@ prepare: stack_protector_prepare
>   PHONY += stack_protector_prepare
>   stack_protector_prepare: prepare0
>   ifdef CONFIG_PPC64
> -	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls \
> +				-mstack-protector-guard-reg=r13 \

Can you put both above lines on a single line to avoid having too many 
lines at the end:

+	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls 
-mstack-protector-guard-reg=r13 \

> +				-mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
>   else
> -	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls \
> +				-mstack-protector-guard-reg=r2 \

Same

> +				-mstack-protector-guard-offset=$(shell awk '{if ($$2 == "TASK_CANARY") print $$3;}' include/generated/asm-offsets.h))
>   endif
>   endif
>   
> 

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

* Re: [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test
  2024-10-08  4:22 ` [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test Nathan Chancellor
@ 2024-10-08  5:14   ` Christophe Leroy
  2024-10-08 13:52     ` Nathan Chancellor
  2024-10-08 16:08     ` Keith Packard
  0 siblings, 2 replies; 10+ messages in thread
From: Christophe Leroy @ 2024-10-08  5:14 UTC (permalink / raw)
  To: Nathan Chancellor, Michael Ellerman
  Cc: Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Keith Packard,
	linuxppc-dev, llvm, patches



Le 08/10/2024 à 06:22, Nathan Chancellor a écrit :
> Kbuild uses the powerpc64le-linux-gnu target for clang, which causes the
> Kconfig check for 32-bit powerpc stack protector support to fail because
> nothing flips the target to 32-bit:
> 
>    $ clang --target=powerpc64le-linux-gnu \
>            -mstack-protector-guard=tls
>            -mstack-protector-guard-reg=r2 \
>            -mstack-protector-guard-offset=0 \
>            -x c -c -o /dev/null /dev/null
>    clang: error: invalid value 'r2' in 'mstack-protector-guard-reg=', expected one of: r13

Why is there any restriction at all on which register can be used ? I 
can't see such restriction in GCC documentation : 
https://gcc.gnu.org/onlinedocs/gcc/RS_002f6000-and-PowerPC-Options.html



> 
> Use the Kconfig macro '$(m32-flag)', which expands to '-m32' when
> supported, in the stack protector support cc-option call to properly
> switch the target to a 32-bit one, which matches what happens in Kbuild.
> While the 64-bit macro does not strictly need it, add the equivalent
> 64-bit option for symmetry.
> 
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>   arch/powerpc/Kconfig | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index eb98050b8c016bb23887a9d669d29e69d933c9c8..6aaca48955a34b2a38af1415bfa36f74f35c3f3e 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -275,8 +275,8 @@ config PPC
>   	select HAVE_RSEQ
>   	select HAVE_SETUP_PER_CPU_AREA		if PPC64
>   	select HAVE_SOFTIRQ_ON_OWN_STACK
> -	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
> -	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
> +	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,$(m32-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
> +	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,$(m64-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)

You modify the exact same line than Patch 1, if this patch is really 
required it should be squashed into patch 1 I think.

>   	select HAVE_STATIC_CALL			if PPC32
>   	select HAVE_SYSCALL_TRACEPOINTS
>   	select HAVE_VIRT_CPU_ACCOUNTING
> 

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

* Re: [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS for clang
  2024-10-08  5:10   ` Christophe Leroy
@ 2024-10-08 13:39     ` Nathan Chancellor
  0 siblings, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2024-10-08 13:39 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Michael Ellerman, Nicholas Piggin, Naveen N Rao,
	Madhavan Srinivasan, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Keith Packard, linuxppc-dev, llvm, patches

Hi Christophe,

First of all, thanks a lot for the quick review.

On Tue, Oct 08, 2024 at 07:10:36AM +0200, Christophe Leroy wrote:
> Le 08/10/2024 à 06:22, Nathan Chancellor a écrit :
> > After fixing the HAVE_STACKPROTECTER checks for clang's in-progress
> > per-task stack protector support [1], the build fails during prepare0
> > because '-mstack-protector-guard-offset' has not been added to
> > KBUILD_CFLAGS yet but the other '-mstack-protector-guard' flags have.
> > 
> >    clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
> >    clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
> >    make[4]: *** [scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
> >    make[4]: *** [scripts/Makefile.build:102: scripts/mod/devicetable-offsets.s] Error 1
> > 
> > Mirror other architectures and add all '-mstack-protector-guard' flags
> > to KBUILD_CFLAGS atomically during stack_protector_prepare, which
> > resolves the issue and allows clang's implementation to fully work with
> > the kernel.
> > 
> > Link: https://github.com/llvm/llvm-project/pull/110928 [1]
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> Is it worth a Fixes: tag so that it gets applied on stable, or wont CLANG 20
> be used with kernels older than 6.13 ?

Yes, I did consider adding Fixes: + Cc: stable. I think it is probably
worth doing since the fixes are rather benign. Do you have a suggestion
for a good Fixes: commit? It always feels somewhat wrong to blame a
commit for not having foresight into how a future implementation might
do things differently :)

> > -	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
> > +	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls \
> > +				-mstack-protector-guard-reg=r13 \
> 
> Can you put both above lines on a single line to avoid having too many lines
> at the end:
> 
> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 \

Ack, I have made this change locally for v2.

Cheers,
Nathan

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

* Re: [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test
  2024-10-08  5:14   ` Christophe Leroy
@ 2024-10-08 13:52     ` Nathan Chancellor
  2024-10-08 16:08     ` Keith Packard
  1 sibling, 0 replies; 10+ messages in thread
From: Nathan Chancellor @ 2024-10-08 13:52 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Michael Ellerman, Nicholas Piggin, Naveen N Rao,
	Madhavan Srinivasan, Nick Desaulniers, Bill Wendling,
	Justin Stitt, Keith Packard, linuxppc-dev, llvm, patches

On Tue, Oct 08, 2024 at 07:14:26AM +0200, Christophe Leroy wrote:
> Le 08/10/2024 à 06:22, Nathan Chancellor a écrit :
> > Kbuild uses the powerpc64le-linux-gnu target for clang, which causes the
> > Kconfig check for 32-bit powerpc stack protector support to fail because
> > nothing flips the target to 32-bit:
> > 
> >    $ clang --target=powerpc64le-linux-gnu \
> >            -mstack-protector-guard=tls
> >            -mstack-protector-guard-reg=r2 \
> >            -mstack-protector-guard-offset=0 \
> >            -x c -c -o /dev/null /dev/null
> >    clang: error: invalid value 'r2' in 'mstack-protector-guard-reg=', expected one of: r13
> 
> Why is there any restriction at all on which register can be used ? I can't
> see such restriction in GCC documentation :
> https://gcc.gnu.org/onlinedocs/gcc/RS_002f6000-and-PowerPC-Options.html

Keith may be able to answer this better than I can but I believe this is
an existing restriction in the LLVM code that dates back to the original
stack protector support:

https://github.com/llvm/llvm-project/commit/a1d8bc559761c94dded3d1e7200cb264a982d1c5

It always uses r2 for 32-bit and r13 for 64-bit for loading the stack
canary. Keith's patch basically just allows customizing the offset,
rather than the register (ultimately for simplicity in the compiler
patch I believe, since we really only care about supporting what the
kernel uses).

> > Use the Kconfig macro '$(m32-flag)', which expands to '-m32' when
> > supported, in the stack protector support cc-option call to properly
> > switch the target to a 32-bit one, which matches what happens in Kbuild.
> > While the 64-bit macro does not strictly need it, add the equivalent
> > 64-bit option for symmetry.
> > 
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >   arch/powerpc/Kconfig | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index eb98050b8c016bb23887a9d669d29e69d933c9c8..6aaca48955a34b2a38af1415bfa36f74f35c3f3e 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -275,8 +275,8 @@ config PPC
> >   	select HAVE_RSEQ
> >   	select HAVE_SETUP_PER_CPU_AREA		if PPC64
> >   	select HAVE_SOFTIRQ_ON_OWN_STACK
> > -	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
> > -	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
> > +	select HAVE_STACKPROTECTOR		if PPC32 && $(cc-option,$(m32-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
> > +	select HAVE_STACKPROTECTOR		if PPC64 && $(cc-option,$(m64-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
> 
> You modify the exact same line than Patch 1, if this patch is really
> required it should be squashed into patch 1 I think.

I believe it will still be required based on my comment above. I can
certainly squash it into patch 1, although the commit message might get
a bit wordy with both explanations in there. I suppose it still makes
sense to do so since "fix the Kconfig test" can encompass both issues
easily.

Cheers,
Nathan

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

* Re: [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test
  2024-10-08  5:14   ` Christophe Leroy
  2024-10-08 13:52     ` Nathan Chancellor
@ 2024-10-08 16:08     ` Keith Packard
  1 sibling, 0 replies; 10+ messages in thread
From: Keith Packard @ 2024-10-08 16:08 UTC (permalink / raw)
  To: Christophe Leroy, Nathan Chancellor, Michael Ellerman
  Cc: Nicholas Piggin, Naveen N Rao, Madhavan Srinivasan,
	Nick Desaulniers, Bill Wendling, Justin Stitt, linuxppc-dev, llvm,
	patches

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


> Why is there any restriction at all on which register can be used ? I 
> can't see such restriction in GCC documentation : 
> https://gcc.gnu.org/onlinedocs/gcc/RS_002f6000-and-PowerPC-Options.html

The clang implementation shares the same code paths as the user space
thread local storage implementation. That code uses a fixed register (2
for 32-bit, 13 for 64-bit) and a fixed offset (-0x7008 for 32-bit and
-0x7010 for 64-bit).

The new code controls the offset value with a command line parameter.

I didn't see any need to make the changes more complicated by including
support for arbitrary registers. The implementation would be reasonably
straightforward, but it would make testing a bunch harder.

The command line parsing code validates that you've selected the correct
register — if we allow other registers in the future, you'll be able to
verify whether the compiler supports that by testing at build time.

-- 
-keith

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

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

* Re: [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support
  2024-10-08  4:22 [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Nathan Chancellor
                   ` (2 preceding siblings ...)
  2024-10-08  4:22 ` [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test Nathan Chancellor
@ 2024-10-08 16:09 ` Keith Packard
  3 siblings, 0 replies; 10+ messages in thread
From: Keith Packard @ 2024-10-08 16:09 UTC (permalink / raw)
  To: Nathan Chancellor, Michael Ellerman
  Cc: Nicholas Piggin, Christophe Leroy, Naveen N Rao,
	Madhavan Srinivasan, Nick Desaulniers, Bill Wendling,
	Justin Stitt, linuxppc-dev, llvm, patches, Nathan Chancellor

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


> This series prepares the powerpc Kconfig and Kbuild files for clang's
> per-task stack protector support.

Reviewed-by: Keith Packard <keithp@keithp.com>
Tested-by: Keith Packard <keithp@keithp.com>

-- 
-keith

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

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

end of thread, other threads:[~2024-10-08 16:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-08  4:22 [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Nathan Chancellor
2024-10-08  4:22 ` [PATCH 1/3] powerpc: Fix stack protector Kconfig test for clang Nathan Chancellor
2024-10-08  4:22 ` [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS " Nathan Chancellor
2024-10-08  5:10   ` Christophe Leroy
2024-10-08 13:39     ` Nathan Chancellor
2024-10-08  4:22 ` [PATCH 3/3] powerpc: Include -m32 / -m64 for stack protector Kconfig test Nathan Chancellor
2024-10-08  5:14   ` Christophe Leroy
2024-10-08 13:52     ` Nathan Chancellor
2024-10-08 16:08     ` Keith Packard
2024-10-08 16:09 ` [PATCH 0/3] powerpc: Prepare for clang's per-task stack protector support Keith Packard

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