* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-08 17:37 [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures Alice Ryhl
@ 2024-10-08 18:03 ` Matthew Maurer
2024-10-09 5:29 ` WANG Rui
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: Matthew Maurer @ 2024-10-08 18:03 UTC (permalink / raw)
To: Alice Ryhl
Cc: Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
Peter Zijlstra (Intel), Sami Tolvanen, linux-arm-kernel,
linux-kernel, loongarch, linux-riscv, rust-for-linux
Reviewed-By: Matthew Maurer <mmaurer@google.com>
On Tue, Oct 8, 2024 at 10:37 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> The KCFI sanitizer stores the CFI tag of a function just before its
> machine code. However, the patchable-function-entry flag can be used to
> introduce additional nop instructions before the machine code, taking up
> the space that normally holds the CFI tag. In this case, a backwards
> offset is applied to the CFI tag to move them out of the way of the nop
> instructions. To ensure that C and Rust agree on the offset used by CFI
> tags, pass the -Zpatchable-function-entry to rustc whenever it is passed
> to the C compiler.
>
> The required rustc version is bumped to 1.81.0 to ensure that the
> -Zpatchable-function-entry flag is available when CFI is used.
>
> Fixes: ca627e636551 ("rust: cfi: add support for CFI_CLANG with Rust")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Note that this fix uses rustc-option which has a pending fix:
> https://lore.kernel.org/all/20241008-rustc-option-bootstrap-v2-1-e6e155b8f9f3@google.com/
> ---
> arch/arm64/Makefile | 2 ++
> arch/loongarch/Makefile | 1 +
> arch/riscv/Makefile | 2 ++
> init/Kconfig | 2 +-
> 4 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 9efd3f37c2fd..d7ec0bb09fc4 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -143,9 +143,11 @@ CHECKFLAGS += -D__aarch64__
> ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=4,2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=4$(comma)2)
> else ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_ARGS),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
>
> ifeq ($(CONFIG_KASAN_SW_TAGS), y)
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index ae3f80622f4c..f9cef31d1f0e 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -44,6 +44,7 @@ endif
> ifdef CONFIG_DYNAMIC_FTRACE
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> +KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
>
> ifdef CONFIG_64BIT
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index d469db9f46f4..65d4dcba309a 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -16,8 +16,10 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> ifeq ($(CONFIG_RISCV_ISA_C),y)
> CC_FLAGS_FTRACE := -fpatchable-function-entry=4
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=4)
> else
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
> endif
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 530a382ee0fe..43434b681c3f 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1946,7 +1946,7 @@ config RUST
> depends on !GCC_PLUGIN_RANDSTRUCT
> depends on !RANDSTRUCT
> depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
> - depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
> + depends on !CFI_CLANG || RUSTC_VERSION >= 108100 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
> select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG
> depends on !CALL_PADDING || RUSTC_VERSION >= 108100
> depends on !KASAN_SW_TAGS
>
> ---
> base-commit: 4a335f920bc78e51b1d7d216d11f2ecbb6dd949f
> change-id: 20241008-cfi-patchable-all-ddd6275eaf4f
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-08 17:37 [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures Alice Ryhl
2024-10-08 18:03 ` Matthew Maurer
@ 2024-10-09 5:29 ` WANG Rui
2024-10-09 16:48 ` Sami Tolvanen
2024-10-09 17:43 ` Mark Rutland
3 siblings, 0 replies; 18+ messages in thread
From: WANG Rui @ 2024-10-09 5:29 UTC (permalink / raw)
To: Alice Ryhl
Cc: Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
Matthew Maurer, Peter Zijlstra (Intel), Sami Tolvanen,
linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
rust-for-linux
On Wed, Oct 9, 2024 at 1:37 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> The KCFI sanitizer stores the CFI tag of a function just before its
> machine code. However, the patchable-function-entry flag can be used to
> introduce additional nop instructions before the machine code, taking up
> the space that normally holds the CFI tag. In this case, a backwards
> offset is applied to the CFI tag to move them out of the way of the nop
> instructions. To ensure that C and Rust agree on the offset used by CFI
> tags, pass the -Zpatchable-function-entry to rustc whenever it is passed
> to the C compiler.
>
> The required rustc version is bumped to 1.81.0 to ensure that the
> -Zpatchable-function-entry flag is available when CFI is used.
>
> Fixes: ca627e636551 ("rust: cfi: add support for CFI_CLANG with Rust")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Note that this fix uses rustc-option which has a pending fix:
> https://lore.kernel.org/all/20241008-rustc-option-bootstrap-v2-1-e6e155b8f9f3@google.com/
> ---
> arch/arm64/Makefile | 2 ++
> arch/loongarch/Makefile | 1 +
> arch/riscv/Makefile | 2 ++
> init/Kconfig | 2 +-
> 4 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 9efd3f37c2fd..d7ec0bb09fc4 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -143,9 +143,11 @@ CHECKFLAGS += -D__aarch64__
> ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=4,2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=4$(comma)2)
> else ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_ARGS),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
>
> ifeq ($(CONFIG_KASAN_SW_TAGS), y)
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index ae3f80622f4c..f9cef31d1f0e 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -44,6 +44,7 @@ endif
> ifdef CONFIG_DYNAMIC_FTRACE
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> +KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
Tested-by: WANG Rui <wangrui@loongson.cn>
> endif
>
> ifdef CONFIG_64BIT
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index d469db9f46f4..65d4dcba309a 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -16,8 +16,10 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> ifeq ($(CONFIG_RISCV_ISA_C),y)
> CC_FLAGS_FTRACE := -fpatchable-function-entry=4
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=4)
> else
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
> endif
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 530a382ee0fe..43434b681c3f 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1946,7 +1946,7 @@ config RUST
> depends on !GCC_PLUGIN_RANDSTRUCT
> depends on !RANDSTRUCT
> depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
> - depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
> + depends on !CFI_CLANG || RUSTC_VERSION >= 108100 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
> select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG
> depends on !CALL_PADDING || RUSTC_VERSION >= 108100
> depends on !KASAN_SW_TAGS
>
> ---
> base-commit: 4a335f920bc78e51b1d7d216d11f2ecbb6dd949f
> change-id: 20241008-cfi-patchable-all-ddd6275eaf4f
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-08 17:37 [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures Alice Ryhl
2024-10-08 18:03 ` Matthew Maurer
2024-10-09 5:29 ` WANG Rui
@ 2024-10-09 16:48 ` Sami Tolvanen
2024-10-09 17:43 ` Mark Rutland
3 siblings, 0 replies; 18+ messages in thread
From: Sami Tolvanen @ 2024-10-09 16:48 UTC (permalink / raw)
To: Alice Ryhl
Cc: Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
Matthew Maurer, Peter Zijlstra (Intel), linux-arm-kernel,
linux-kernel, loongarch, linux-riscv, rust-for-linux
On Tue, Oct 8, 2024 at 10:37 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> The KCFI sanitizer stores the CFI tag of a function just before its
> machine code. However, the patchable-function-entry flag can be used to
> introduce additional nop instructions before the machine code, taking up
> the space that normally holds the CFI tag. In this case, a backwards
> offset is applied to the CFI tag to move them out of the way of the nop
> instructions. To ensure that C and Rust agree on the offset used by CFI
> tags, pass the -Zpatchable-function-entry to rustc whenever it is passed
> to the C compiler.
>
> The required rustc version is bumped to 1.81.0 to ensure that the
> -Zpatchable-function-entry flag is available when CFI is used.
>
> Fixes: ca627e636551 ("rust: cfi: add support for CFI_CLANG with Rust")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Sami
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-08 17:37 [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures Alice Ryhl
` (2 preceding siblings ...)
2024-10-09 16:48 ` Sami Tolvanen
@ 2024-10-09 17:43 ` Mark Rutland
2024-10-09 20:15 ` Alice Ryhl
3 siblings, 1 reply; 18+ messages in thread
From: Mark Rutland @ 2024-10-09 17:43 UTC (permalink / raw)
To: Alice Ryhl
Cc: Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
Matthew Maurer, Peter Zijlstra (Intel), Sami Tolvanen,
linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
rust-for-linux
Hi Alice,
On Tue, Oct 08, 2024 at 05:37:16PM +0000, Alice Ryhl wrote:
> The KCFI sanitizer stores the CFI tag of a function just before its
> machine code. However, the patchable-function-entry flag can be used to
> introduce additional nop instructions before the machine code, taking up
> the space that normally holds the CFI tag.
To clarify, when you say "before the machine code", do you mean when
NOPs are placed before the function entry point? e.g. if we compiled
with -fpatchable-function-entry=M,N where N > 0? I'll refer tho this as
"pre-function NOPs" below.
There's an existing incompatibility between CFI and pre-function NOPs
for C code, because we override -fpatchable-function-entry on a
per-function basis (e.g. for noinstr and notrace), and we don't
currently have a mechanism to ensure the CFI tag is in the same place
regardless. This is why arm64 has CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
depend on !CFI.
For C code at least, just using regular -fpatchable-function-entry=M or
-fpatchable-function-entry=M,0 shouldn't change the location of the CFI
tag relative to the function entrypoint, and so should have no adverse
effect on CFI.
Is Rust any different here?
> In this case, a backwards offset is applied to the CFI tag to move
> them out of the way of the nop instructions. To ensure that C and Rust
> agree on the offset used by CFI tags, pass the
> -Zpatchable-function-entry to rustc whenever it is passed to the C
> compiler.
As above, I suspect this isn't necessary to make CFI work, for any case
that works with C today, due to -fpatchable-funtion-entry being
overridden on a per-function basis. Are you seeing a problem in
practice, or was this found by inspection?
However IIUC this will allow rust to be traced via ftrace (assuming rust
records the instrumented locations as gcc and clang do); is that the
case? Assuming so, is there any ABI difference that might bite us? On
arm64 we require that anything marked instrumented with
patchable-function-entry strictly follows the AAPCS64 calling convention
and our ftrace trampolines save/restore the minimal set of necessary
registers, and I don't know how rust whether rust will behave the same
or e.g. use specialized calling conventions internally.
Mark.
> The required rustc version is bumped to 1.81.0 to ensure that the
> -Zpatchable-function-entry flag is available when CFI is used.
>
> Fixes: ca627e636551 ("rust: cfi: add support for CFI_CLANG with Rust")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Note that this fix uses rustc-option which has a pending fix:
> https://lore.kernel.org/all/20241008-rustc-option-bootstrap-v2-1-e6e155b8f9f3@google.com/
> ---
> arch/arm64/Makefile | 2 ++
> arch/loongarch/Makefile | 1 +
> arch/riscv/Makefile | 2 ++
> init/Kconfig | 2 +-
> 4 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 9efd3f37c2fd..d7ec0bb09fc4 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -143,9 +143,11 @@ CHECKFLAGS += -D__aarch64__
> ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=4,2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=4$(comma)2)
> else ifeq ($(CONFIG_DYNAMIC_FTRACE_WITH_ARGS),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
>
> ifeq ($(CONFIG_KASAN_SW_TAGS), y)
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index ae3f80622f4c..f9cef31d1f0e 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -44,6 +44,7 @@ endif
> ifdef CONFIG_DYNAMIC_FTRACE
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> +KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
>
> ifdef CONFIG_64BIT
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index d469db9f46f4..65d4dcba309a 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -16,8 +16,10 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y)
> KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY
> ifeq ($(CONFIG_RISCV_ISA_C),y)
> CC_FLAGS_FTRACE := -fpatchable-function-entry=4
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=4)
> else
> CC_FLAGS_FTRACE := -fpatchable-function-entry=2
> + KBUILD_RUSTFLAGS += $(call rustc-option,-Zpatchable-function-entry=2)
> endif
> endif
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 530a382ee0fe..43434b681c3f 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1946,7 +1946,7 @@ config RUST
> depends on !GCC_PLUGIN_RANDSTRUCT
> depends on !RANDSTRUCT
> depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
> - depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
> + depends on !CFI_CLANG || RUSTC_VERSION >= 108100 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
> select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG
> depends on !CALL_PADDING || RUSTC_VERSION >= 108100
> depends on !KASAN_SW_TAGS
>
> ---
> base-commit: 4a335f920bc78e51b1d7d216d11f2ecbb6dd949f
> change-id: 20241008-cfi-patchable-all-ddd6275eaf4f
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-09 17:43 ` Mark Rutland
@ 2024-10-09 20:15 ` Alice Ryhl
2024-10-09 20:32 ` Sami Tolvanen
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Alice Ryhl @ 2024-10-09 20:15 UTC (permalink / raw)
To: Mark Rutland, Matthew Maurer, Sami Tolvanen
Cc: Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
Peter Zijlstra (Intel), linux-arm-kernel, linux-kernel, loongarch,
linux-riscv, rust-for-linux
On Wed, Oct 9, 2024 at 7:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> Hi Alice,
>
> On Tue, Oct 08, 2024 at 05:37:16PM +0000, Alice Ryhl wrote:
> > The KCFI sanitizer stores the CFI tag of a function just before its
> > machine code. However, the patchable-function-entry flag can be used to
> > introduce additional nop instructions before the machine code, taking up
> > the space that normally holds the CFI tag.
>
> To clarify, when you say "before the machine code", do you mean when
> NOPs are placed before the function entry point? e.g. if we compiled
> with -fpatchable-function-entry=M,N where N > 0? I'll refer tho this as
> "pre-function NOPs" below.
>
> There's an existing incompatibility between CFI and pre-function NOPs
> for C code, because we override -fpatchable-function-entry on a
> per-function basis (e.g. for noinstr and notrace), and we don't
> currently have a mechanism to ensure the CFI tag is in the same place
> regardless. This is why arm64 has CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
> depend on !CFI.
>
> For C code at least, just using regular -fpatchable-function-entry=M or
> -fpatchable-function-entry=M,0 shouldn't change the location of the CFI
> tag relative to the function entrypoint, and so should have no adverse
> effect on CFI.
>
> Is Rust any different here?
Ah, no it shouldn't be. Sami can you confirm?
> > In this case, a backwards offset is applied to the CFI tag to move
> > them out of the way of the nop instructions. To ensure that C and Rust
> > agree on the offset used by CFI tags, pass the
> > -Zpatchable-function-entry to rustc whenever it is passed to the C
> > compiler.
>
> As above, I suspect this isn't necessary to make CFI work, for any case
> that works with C today, due to -fpatchable-funtion-entry being
> overridden on a per-function basis. Are you seeing a problem in
> practice, or was this found by inspection?
>
> However IIUC this will allow rust to be traced via ftrace (assuming rust
> records the instrumented locations as gcc and clang do); is that the
> case? Assuming so, is there any ABI difference that might bite us? On
> arm64 we require that anything marked instrumented with
> patchable-function-entry strictly follows the AAPCS64 calling convention
> and our ftrace trampolines save/restore the minimal set of necessary
> registers, and I don't know how rust whether rust will behave the same
> or e.g. use specialized calling conventions internally.
Well, I was told that it's a problem and was able to trigger a failure
on x86. I didn't manage to trigger one on arm64, but I wasn't sure
whether that was me doing something wrong, or whether the problem only
exists on x86. We already have the flag on x86 for FINEIBT, but I
thought on the off chance that it's not a problem in practice on arm,
it still doesn't hurt to add the flag.
Regarding the AAPCS64 calling convention thing ... rustc uses the Rust
calling convention for functions internally in Rust code and I don't
know whether that changes anything relevant for what you mention.
Matthew/Sami do you know?
Alice
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-09 20:15 ` Alice Ryhl
@ 2024-10-09 20:32 ` Sami Tolvanen
2024-10-09 20:38 ` Matthew Maurer
2024-10-10 10:45 ` Mark Rutland
2 siblings, 0 replies; 18+ messages in thread
From: Sami Tolvanen @ 2024-10-09 20:32 UTC (permalink / raw)
To: Alice Ryhl
Cc: Mark Rutland, Matthew Maurer, Catalin Marinas, Will Deacon,
Huacai Chen, WANG Xuerui, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Kees Cook, Peter Zijlstra (Intel), linux-arm-kernel,
linux-kernel, loongarch, linux-riscv, rust-for-linux
On Wed, Oct 9, 2024 at 8:15 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Wed, Oct 9, 2024 at 7:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > There's an existing incompatibility between CFI and pre-function NOPs
> > for C code, because we override -fpatchable-function-entry on a
> > per-function basis (e.g. for noinstr and notrace), and we don't
> > currently have a mechanism to ensure the CFI tag is in the same place
> > regardless. This is why arm64 has CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
> > depend on !CFI.
> >
> > For C code at least, just using regular -fpatchable-function-entry=M or
> > -fpatchable-function-entry=M,0 shouldn't change the location of the CFI
> > tag relative to the function entrypoint, and so should have no adverse
> > effect on CFI.
> >
> > Is Rust any different here?
>
> Ah, no it shouldn't be. Sami can you confirm?
KCFI is implemented in the LLVM back-end, so the behavior is exactly
the same for both C and Rust.
> > As above, I suspect this isn't necessary to make CFI work, for any case
> > that works with C today, due to -fpatchable-funtion-entry being
> > overridden on a per-function basis. Are you seeing a problem in
> > practice, or was this found by inspection?
> >
[..]
> Well, I was told that it's a problem and was able to trigger a failure
> on x86. I didn't manage to trigger one on arm64, but I wasn't sure
> whether that was me doing something wrong, or whether the problem only
> exists on x86. We already have the flag on x86 for FINEIBT, but I
> thought on the off chance that it's not a problem in practice on arm,
> it still doesn't hurt to add the flag.
This only impacts KCFI on x86 at the moment. However, we should
nevertheless pass the same patchable-function-entry flags to both
compilers on other architectures too.
> Regarding the AAPCS64 calling convention thing ... rustc uses the Rust
> calling convention for functions internally in Rust code and I don't
> know whether that changes anything relevant for what you mention.
> Matthew/Sami do you know?
AFAIK this shouldn't be a problem, but Matt knows this much better, so
I'll let him explain.
Sami
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-09 20:15 ` Alice Ryhl
2024-10-09 20:32 ` Sami Tolvanen
@ 2024-10-09 20:38 ` Matthew Maurer
2024-10-10 10:45 ` Mark Rutland
2 siblings, 0 replies; 18+ messages in thread
From: Matthew Maurer @ 2024-10-09 20:38 UTC (permalink / raw)
To: Alice Ryhl
Cc: Mark Rutland, Sami Tolvanen, Catalin Marinas, Will Deacon,
Huacai Chen, WANG Xuerui, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Kees Cook, Peter Zijlstra (Intel), linux-arm-kernel,
linux-kernel, loongarch, linux-riscv, rust-for-linux
On Wed, Oct 9, 2024 at 1:15 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Wed, Oct 9, 2024 at 7:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > Hi Alice,
> >
> > On Tue, Oct 08, 2024 at 05:37:16PM +0000, Alice Ryhl wrote:
> > > The KCFI sanitizer stores the CFI tag of a function just before its
> > > machine code. However, the patchable-function-entry flag can be used to
> > > introduce additional nop instructions before the machine code, taking up
> > > the space that normally holds the CFI tag.
> >
> > To clarify, when you say "before the machine code", do you mean when
> > NOPs are placed before the function entry point? e.g. if we compiled
> > with -fpatchable-function-entry=M,N where N > 0? I'll refer tho this as
> > "pre-function NOPs" below.
> >
> > There's an existing incompatibility between CFI and pre-function NOPs
> > for C code, because we override -fpatchable-function-entry on a
> > per-function basis (e.g. for noinstr and notrace), and we don't
> > currently have a mechanism to ensure the CFI tag is in the same place
> > regardless. This is why arm64 has CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
> > depend on !CFI.
> >
> > For C code at least, just using regular -fpatchable-function-entry=M or
> > -fpatchable-function-entry=M,0 shouldn't change the location of the CFI
> > tag relative to the function entrypoint, and so should have no adverse
> > effect on CFI.
> >
> > Is Rust any different here?
>
> Ah, no it shouldn't be. Sami can you confirm?
>
> > > In this case, a backwards offset is applied to the CFI tag to move
> > > them out of the way of the nop instructions. To ensure that C and Rust
> > > agree on the offset used by CFI tags, pass the
> > > -Zpatchable-function-entry to rustc whenever it is passed to the C
> > > compiler.
> >
> > As above, I suspect this isn't necessary to make CFI work, for any case
> > that works with C today, due to -fpatchable-funtion-entry being
> > overridden on a per-function basis. Are you seeing a problem in
> > practice, or was this found by inspection?
> >
> > However IIUC this will allow rust to be traced via ftrace (assuming rust
> > records the instrumented locations as gcc and clang do); is that the
> > case? Assuming so, is there any ABI difference that might bite us? On
> > arm64 we require that anything marked instrumented with
> > patchable-function-entry strictly follows the AAPCS64 calling convention
> > and our ftrace trampolines save/restore the minimal set of necessary
> > registers, and I don't know how rust whether rust will behave the same
> > or e.g. use specialized calling conventions internally.
>
> Well, I was told that it's a problem and was able to trigger a failure
> on x86. I didn't manage to trigger one on arm64, but I wasn't sure
> whether that was me doing something wrong, or whether the problem only
> exists on x86. We already have the flag on x86 for FINEIBT, but I
> thought on the off chance that it's not a problem in practice on arm,
> it still doesn't hurt to add the flag.
>
> Regarding the AAPCS64 calling convention thing ... rustc uses the Rust
> calling convention for functions internally in Rust code and I don't
> know whether that changes anything relevant for what you mention.
> Matthew/Sami do you know?
tl;dr: Rust uses AAPCS64 on aarch64. It's not likely to change.
The "rust" calling convention only modifies typed argument lowering
today, not which registers are available. How typed values are lowered
onto these registers may change (has in the past and likely will in
the near future), but the actual register-set has not. It will use the
register calling convention that is default for the architecture,
which for aarch64 is AAPCS64. Technically, upstream does not make a
hard promise that "rust" calling convention functions will adhere to
this, but in practice the compiler structure, desire to work with
third party tools (debuggers/unwinders), and a history of never
changing this makes it very unlikely that they would change this.
>
> Alice
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-09 20:15 ` Alice Ryhl
2024-10-09 20:32 ` Sami Tolvanen
2024-10-09 20:38 ` Matthew Maurer
@ 2024-10-10 10:45 ` Mark Rutland
2024-10-10 11:03 ` Peter Zijlstra
2024-10-10 12:29 ` Alice Ryhl
2 siblings, 2 replies; 18+ messages in thread
From: Mark Rutland @ 2024-10-10 10:45 UTC (permalink / raw)
To: Alice Ryhl
Cc: Matthew Maurer, Sami Tolvanen, Catalin Marinas, Will Deacon,
Huacai Chen, WANG Xuerui, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Kees Cook, Peter Zijlstra (Intel), linux-arm-kernel,
linux-kernel, loongarch, linux-riscv, rust-for-linux
On Wed, Oct 09, 2024 at 10:15:35PM +0200, Alice Ryhl wrote:
> On Wed, Oct 9, 2024 at 7:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > Hi Alice,
> >
> > On Tue, Oct 08, 2024 at 05:37:16PM +0000, Alice Ryhl wrote:
> > > The KCFI sanitizer stores the CFI tag of a function just before its
> > > machine code. However, the patchable-function-entry flag can be used to
> > > introduce additional nop instructions before the machine code, taking up
> > > the space that normally holds the CFI tag.
> >
> > To clarify, when you say "before the machine code", do you mean when
> > NOPs are placed before the function entry point? e.g. if we compiled
> > with -fpatchable-function-entry=M,N where N > 0? I'll refer tho this as
> > "pre-function NOPs" below.
> >
> > There's an existing incompatibility between CFI and pre-function NOPs
> > for C code, because we override -fpatchable-function-entry on a
> > per-function basis (e.g. for noinstr and notrace), and we don't
> > currently have a mechanism to ensure the CFI tag is in the same place
> > regardless. This is why arm64 has CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
> > depend on !CFI.
> >
> > For C code at least, just using regular -fpatchable-function-entry=M or
> > -fpatchable-function-entry=M,0 shouldn't change the location of the CFI
> > tag relative to the function entrypoint, and so should have no adverse
> > effect on CFI.
> >
> > Is Rust any different here?
>
> Ah, no it shouldn't be. Sami can you confirm?
>
> > > In this case, a backwards offset is applied to the CFI tag to move
> > > them out of the way of the nop instructions. To ensure that C and Rust
> > > agree on the offset used by CFI tags, pass the
> > > -Zpatchable-function-entry to rustc whenever it is passed to the C
> > > compiler.
> >
> > As above, I suspect this isn't necessary to make CFI work, for any case
> > that works with C today, due to -fpatchable-funtion-entry being
> > overridden on a per-function basis. Are you seeing a problem in
> > practice, or was this found by inspection?
> >
> > However IIUC this will allow rust to be traced via ftrace (assuming rust
> > records the instrumented locations as gcc and clang do); is that the
> > case? Assuming so, is there any ABI difference that might bite us? On
> > arm64 we require that anything marked instrumented with
> > patchable-function-entry strictly follows the AAPCS64 calling convention
> > and our ftrace trampolines save/restore the minimal set of necessary
> > registers, and I don't know how rust whether rust will behave the same
> > or e.g. use specialized calling conventions internally.
>
> Well, I was told that it's a problem and was able to trigger a failure
> on x86. I didn't manage to trigger one on arm64, but I wasn't sure
> whether that was me doing something wrong, or whether the problem only
> exists on x86. We already have the flag on x86 for FINEIBT,
I believe that hte problem only exists on x86, becaause they use
patchable-function-entry for their FINEIBT patching (and use -mfentry
for ftrace), whereas everyone else uses patchable-function-entry for
ftrace.
> but I thought on the off chance that it's not a problem in practice on
> arm, it still doesn't hurt to add the flag.
It won't adversely affect CFI, but it will open up rust code for ftrace,
so I'm not sure that "it doesn't hurt".
AFAICT at the moment this isn't necessary for CFI, so can we drop this
patch for now?
If we want to pass these flags for !x86, the justification should be to
enable ftrace for rust code, and we should test that actually works
(e.g. by testing ftrace with rust code).
What happens on x86 for ftrace and rust?
> Regarding the AAPCS64 calling convention thing ... rustc uses the Rust
> calling convention for functions internally in Rust code and I don't
> know whether that changes anything relevant for what you mention.
> Matthew/Sami do you know?
From their replies it sounds like that happens to be true in practice
today, but as above I think we should go test this actually works.
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 10:45 ` Mark Rutland
@ 2024-10-10 11:03 ` Peter Zijlstra
2024-10-10 11:37 ` Peter Zijlstra
2024-10-10 12:29 ` Alice Ryhl
1 sibling, 1 reply; 18+ messages in thread
From: Peter Zijlstra @ 2024-10-10 11:03 UTC (permalink / raw)
To: Mark Rutland
Cc: Alice Ryhl, Matthew Maurer, Sami Tolvanen, Catalin Marinas,
Will Deacon, Huacai Chen, WANG Xuerui, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Kees Cook, linux-arm-kernel, linux-kernel,
loongarch, linux-riscv, rust-for-linux
On Thu, Oct 10, 2024 at 11:45:07AM +0100, Mark Rutland wrote:
> What happens on x86 for ftrace and rust?
I can't seem to enable CONFIG_RUST, so I can't tell you :/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 11:03 ` Peter Zijlstra
@ 2024-10-10 11:37 ` Peter Zijlstra
2024-10-10 11:44 ` Miguel Ojeda
0 siblings, 1 reply; 18+ messages in thread
From: Peter Zijlstra @ 2024-10-10 11:37 UTC (permalink / raw)
To: Mark Rutland
Cc: Alice Ryhl, Matthew Maurer, Sami Tolvanen, Catalin Marinas,
Will Deacon, Huacai Chen, WANG Xuerui, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Miguel Ojeda, Alex Gaynor, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Kees Cook, linux-arm-kernel, linux-kernel,
loongarch, linux-riscv, rust-for-linux
On Thu, Oct 10, 2024 at 01:03:44PM +0200, Peter Zijlstra wrote:
> On Thu, Oct 10, 2024 at 11:45:07AM +0100, Mark Rutland wrote:
>
> > What happens on x86 for ftrace and rust?
>
> I can't seem to enable CONFIG_RUST, so I can't tell you :/
So much for the debian 'rust-all' package actually including all. Thanks
Boqun for telling me about the rustavailable make target.
Anyway, confirmed (x86_64) rust code does not have __fentry__ hooks and
cannot be traced :-(
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 11:37 ` Peter Zijlstra
@ 2024-10-10 11:44 ` Miguel Ojeda
2024-10-10 13:04 ` Peter Zijlstra
0 siblings, 1 reply; 18+ messages in thread
From: Miguel Ojeda @ 2024-10-10 11:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Alice Ryhl, Matthew Maurer, Sami Tolvanen,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
rust-for-linux
On Thu, Oct 10, 2024 at 1:37 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> So much for the debian 'rust-all' package actually including all. Thanks
> Boqun for telling me about the rustavailable make target.
Yeah, `rust-all` does not seem to include `bindgen`, which I guess was
the issue (?).
I guess you have it working now, but for the next time / others, we
have the `apt` line for Debian at
https://docs.kernel.org/rust/quick-start.html#debian.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 11:44 ` Miguel Ojeda
@ 2024-10-10 13:04 ` Peter Zijlstra
2024-10-10 14:48 ` Miguel Ojeda
0 siblings, 1 reply; 18+ messages in thread
From: Peter Zijlstra @ 2024-10-10 13:04 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Mark Rutland, Alice Ryhl, Matthew Maurer, Sami Tolvanen,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
rust-for-linux
On Thu, Oct 10, 2024 at 01:44:14PM +0200, Miguel Ojeda wrote:
> On Thu, Oct 10, 2024 at 1:37 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > So much for the debian 'rust-all' package actually including all. Thanks
> > Boqun for telling me about the rustavailable make target.
>
> Yeah, `rust-all` does not seem to include `bindgen`, which I guess was
> the issue (?).
Also not rust-src, because for some reason you're not actually
freestanding :/
> I guess you have it working now, but for the next time / others, we
> have the `apt` line for Debian at
> https://docs.kernel.org/rust/quick-start.html#debian.
Yeah, but then you have to first know that we have this file to begin
with.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 13:04 ` Peter Zijlstra
@ 2024-10-10 14:48 ` Miguel Ojeda
2024-10-11 10:51 ` Peter Zijlstra
0 siblings, 1 reply; 18+ messages in thread
From: Miguel Ojeda @ 2024-10-10 14:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Alice Ryhl, Matthew Maurer, Sami Tolvanen,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
rust-for-linux
On Thu, Oct 10, 2024 at 3:04 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Also not rust-src, because for some reason you're not actually
> freestanding :/
It is there, but it is a suggested package, so I think you would need
e.g. `--install-suggests`:
https://packages.debian.org/trixie/rust-all
Regarding freestanding, we use `no_std`, which one could argue is
similar to C's freestanding. That is, we use `core`, which is a subset
of the full (hosted) standard library, just like C requires some
headers even in freestanding.
However, I think you mean not even using those
headers/sources/libraries, which is fair. Rust calls that `no_core`,
but it is currently impractical to use/maintain and there are a lot of
useful things in `core` we want to use anyway, such as the `Result`
type:
https://doc.rust-lang.org/core/
There are some things that we could remove, though, which is why I
asked long ago for ways to remove unneeded things for the kernel
("modularization of `core`"). We got `no_fp_fmt_parse`, implemented by
Gary back in 2021, and we could perhaps get more in the future if
really needed.
> Yeah, but then you have to first know that we have this file to begin
> with.
That is fair, but hard to fix. We try our best though: the "Rust"
entry is in the front page of the kernel docs already, and the first
paragraph of that page links to the "Quick Start" guide.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 14:48 ` Miguel Ojeda
@ 2024-10-11 10:51 ` Peter Zijlstra
2024-10-11 11:32 ` Miguel Ojeda
0 siblings, 1 reply; 18+ messages in thread
From: Peter Zijlstra @ 2024-10-11 10:51 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Mark Rutland, Alice Ryhl, Matthew Maurer, Sami Tolvanen,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
rust-for-linux
On Thu, Oct 10, 2024 at 04:48:41PM +0200, Miguel Ojeda wrote:
> On Thu, Oct 10, 2024 at 3:04 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Also not rust-src, because for some reason you're not actually
> > freestanding :/
>
> It is there, but it is a suggested package, so I think you would need
> e.g. `--install-suggests`:
>
> https://packages.debian.org/trixie/rust-all
>
> Regarding freestanding, we use `no_std`, which one could argue is
> similar to C's freestanding. That is, we use `core`, which is a subset
> of the full (hosted) standard library, just like C requires some
> headers even in freestanding.
>
> However, I think you mean not even using those
> headers/sources/libraries, which is fair. Rust calls that `no_core`,
> but it is currently impractical to use/maintain and there are a lot of
> useful things in `core` we want to use anyway, such as the `Result`
> type:
>
> https://doc.rust-lang.org/core/
>
> There are some things that we could remove, though, which is why I
> asked long ago for ways to remove unneeded things for the kernel
> ("modularization of `core`"). We got `no_fp_fmt_parse`, implemented by
> Gary back in 2021, and we could perhaps get more in the future if
> really needed.
So you could just copy the bits from core you need into the kernel tree
and leave out those bits you do not, and ocassionally update them when
needed, right?
> > Yeah, but then you have to first know that we have this file to begin
> > with.
>
> That is fair, but hard to fix. We try our best though: the "Rust"
> entry is in the front page of the kernel docs already, and the first
> paragraph of that page links to the "Quick Start" guide.
I'm not really a browser centric kinda guy, the front page is bash in
the source tree. Using the browser is below bitching about stuff on IRC
:-)
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-11 10:51 ` Peter Zijlstra
@ 2024-10-11 11:32 ` Miguel Ojeda
0 siblings, 0 replies; 18+ messages in thread
From: Miguel Ojeda @ 2024-10-11 11:32 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Mark Rutland, Alice Ryhl, Matthew Maurer, Sami Tolvanen,
Catalin Marinas, Will Deacon, Huacai Chen, WANG Xuerui,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Kees Cook,
linux-arm-kernel, linux-kernel, loongarch, linux-riscv,
rust-for-linux
On Fri, Oct 11, 2024 at 12:51 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> So you could just copy the bits from core you need into the kernel tree
> and leave out those bits you do not, and ocassionally update them when
> needed, right?
Yes, but in practice it is a substantial amount of non-trivial code to
maintain and some of it is too tied to the compiler (e.g. using
compiler internal features), thus it would likely require maintaining
several variations in-tree to make it work across compiler versions.
Maybe when kernel has been compiling with stable Rust for a while, and
especially if `core` is split into "really tied to the compiler" and
"the rest", we could consider something like that. Personally I would
like to be in a position where we can try. Perhaps it could even help
to simplify gccrs' life compiling a smaller `core`.
Even then, we would need to see whether the tradeoff is worth it: we
may be able to customize `core` here and there, yes, but, for
instance, we could lose the ability to easily import other code out
there (virtually all Rust code uses `core`).
Cheers,
Miguel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 10:45 ` Mark Rutland
2024-10-10 11:03 ` Peter Zijlstra
@ 2024-10-10 12:29 ` Alice Ryhl
2024-10-11 11:00 ` Mark Rutland
1 sibling, 1 reply; 18+ messages in thread
From: Alice Ryhl @ 2024-10-10 12:29 UTC (permalink / raw)
To: Mark Rutland
Cc: Matthew Maurer, Sami Tolvanen, Catalin Marinas, Will Deacon,
Huacai Chen, WANG Xuerui, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Kees Cook, Peter Zijlstra (Intel), linux-arm-kernel,
linux-kernel, loongarch, linux-riscv, rust-for-linux
On Thu, Oct 10, 2024 at 12:45 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Wed, Oct 09, 2024 at 10:15:35PM +0200, Alice Ryhl wrote:
> > On Wed, Oct 9, 2024 at 7:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > Hi Alice,
> > >
> > > On Tue, Oct 08, 2024 at 05:37:16PM +0000, Alice Ryhl wrote:
> > > > The KCFI sanitizer stores the CFI tag of a function just before its
> > > > machine code. However, the patchable-function-entry flag can be used to
> > > > introduce additional nop instructions before the machine code, taking up
> > > > the space that normally holds the CFI tag.
> > >
> > > To clarify, when you say "before the machine code", do you mean when
> > > NOPs are placed before the function entry point? e.g. if we compiled
> > > with -fpatchable-function-entry=M,N where N > 0? I'll refer tho this as
> > > "pre-function NOPs" below.
> > >
> > > There's an existing incompatibility between CFI and pre-function NOPs
> > > for C code, because we override -fpatchable-function-entry on a
> > > per-function basis (e.g. for noinstr and notrace), and we don't
> > > currently have a mechanism to ensure the CFI tag is in the same place
> > > regardless. This is why arm64 has CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS
> > > depend on !CFI.
> > >
> > > For C code at least, just using regular -fpatchable-function-entry=M or
> > > -fpatchable-function-entry=M,0 shouldn't change the location of the CFI
> > > tag relative to the function entrypoint, and so should have no adverse
> > > effect on CFI.
> > >
> > > Is Rust any different here?
> >
> > Ah, no it shouldn't be. Sami can you confirm?
> >
> > > > In this case, a backwards offset is applied to the CFI tag to move
> > > > them out of the way of the nop instructions. To ensure that C and Rust
> > > > agree on the offset used by CFI tags, pass the
> > > > -Zpatchable-function-entry to rustc whenever it is passed to the C
> > > > compiler.
> > >
> > > As above, I suspect this isn't necessary to make CFI work, for any case
> > > that works with C today, due to -fpatchable-funtion-entry being
> > > overridden on a per-function basis. Are you seeing a problem in
> > > practice, or was this found by inspection?
> > >
> > > However IIUC this will allow rust to be traced via ftrace (assuming rust
> > > records the instrumented locations as gcc and clang do); is that the
> > > case? Assuming so, is there any ABI difference that might bite us? On
> > > arm64 we require that anything marked instrumented with
> > > patchable-function-entry strictly follows the AAPCS64 calling convention
> > > and our ftrace trampolines save/restore the minimal set of necessary
> > > registers, and I don't know how rust whether rust will behave the same
> > > or e.g. use specialized calling conventions internally.
> >
> > Well, I was told that it's a problem and was able to trigger a failure
> > on x86. I didn't manage to trigger one on arm64, but I wasn't sure
> > whether that was me doing something wrong, or whether the problem only
> > exists on x86. We already have the flag on x86 for FINEIBT,
>
> I believe that hte problem only exists on x86, becaause they use
> patchable-function-entry for their FINEIBT patching (and use -mfentry
> for ftrace), whereas everyone else uses patchable-function-entry for
> ftrace.
>
> > but I thought on the off chance that it's not a problem in practice on
> > arm, it still doesn't hurt to add the flag.
>
> It won't adversely affect CFI, but it will open up rust code for ftrace,
> so I'm not sure that "it doesn't hurt".
>
> AFAICT at the moment this isn't necessary for CFI, so can we drop this
> patch for now?
I'm okay with dropping this patch for now.
Alice
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures
2024-10-10 12:29 ` Alice Ryhl
@ 2024-10-11 11:00 ` Mark Rutland
0 siblings, 0 replies; 18+ messages in thread
From: Mark Rutland @ 2024-10-11 11:00 UTC (permalink / raw)
To: Alice Ryhl
Cc: Matthew Maurer, Sami Tolvanen, Catalin Marinas, Will Deacon,
Huacai Chen, WANG Xuerui, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Kees Cook, Peter Zijlstra (Intel), linux-arm-kernel,
linux-kernel, loongarch, linux-riscv, rust-for-linux
On Thu, Oct 10, 2024 at 02:29:18PM +0200, Alice Ryhl wrote:
> On Thu, Oct 10, 2024 at 12:45 PM Mark Rutland <mark.rutland@arm.com> wrote:
> > AFAICT at the moment this isn't necessary for CFI, so can we drop this
> > patch for now?
>
> I'm okay with dropping this patch for now.
Cool; thanks!
Mark.
^ permalink raw reply [flat|nested] 18+ messages in thread