rust: cfi: disable function merging if CFI is enabled
PATCH 1/1 rust: cfi: disable function merging if CFI is enabled
From: Gary Guo <[email protected]>
In Rust doc tests, there is a dummy __module_firmware_test_init function generated by the example in module_firmware!'s documentation, which just returns zero. Many other documentation generates function that produces zero. LKP test robot reports a Flags::zeroed instance; my local reproduction has a Bounded::new::<0>.
LLVM's MergeFunctionsPass incorrectly merge functions with the same KCFI type, causing __module_firmware_test_init being merged into one of the zero-returning function. As module init is invoked via indirect function call, KCFI is checked and this produces a KCFI failure.
I've reported this bug to upstream LLVM [1]; in the mean time, disable function merging if CFI is enabled. No separate treatment is needed for CONFIG_RUST_INLINE_HELPERS, as Clang does not enable function merging by default.
--- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Makefile +2 -1
@@ -1118,7 +1118,8 @@ endif ifdef CONFIG_RUST # Always pass -Zsanitizer-cfi-normalize-integers as CONFIG_RUST selects # CONFIG_CFI_ICALL_NORMALIZE_INTEGERS. - RUSTC_FLAGS_CFI := -Zsanitizer=kcfi -Zsanitizer-cfi-normalize-integers + # Disable function merging as LLVM incorrectly merge functions with the same KCFI type. + RUSTC_FLAGS_CFI := -Zsanitizer=kcfi -Zsanitizer-cfi-normalize-integers -Zmerge-functions=disabled KBUILD_RUSTFLAGS += $(RUSTC_FLAGS_CFI) export RUSTC_FLAGS_CFI endif
base-commit: e6664f2b33db9b6811eb4cec109f06cb2b4f458d
Signature · 1 line
2.54.0
Gary Guo
reviewerQuoted 9 lines · Gary Guo
From: Gary Guo <[email protected]>
In Rust doc tests, there is a dummy
__module_firmware_test_initfunction generated by the example inmodule_firmware!'s documentation, which just returns zero. Many other documentation generates function that produces zero. LKP test robot reports aFlags::zeroedinstance; my local reproduction has aBounded::new::<0>.LLVM's MergeFunctionsPass incorrectly merge functions with the same KCFI
This should read "with different KCFI type" (obviously).
Best,
Gary
Quoted 33 lines
type, causing
__module_firmware_test_initbeing merged into one of the zero-returning function. As module init is invoked via indirect function call, KCFI is checked and this produces a KCFI failure.I've reported this bug to upstream LLVM [1]; in the mean time, disable function merging if CFI is enabled. No separate treatment is needed for
CONFIG_RUST_INLINE_HELPERS, as Clang does not enable function merging by default.Reported-by kernel test robot <[email protected]>Signed-off-by Gary Guo <[email protected]>--- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Makefile +2
-1@@ -1118,7 +1118,8 @@ endif ifdef CONFIG_RUST # Always pass -Zsanitizer-cfi-normalize-integers as CONFIG_RUST selects # CONFIG_CFI_ICALL_NORMALIZE_INTEGERS. - RUSTC_FLAGS_CFI := -Zsanitizer=kcfi -Zsanitizer-cfi-normalize-integers + # Disable function merging as LLVM incorrectly merge functions with the same KCFI type. + RUSTC_FLAGS_CFI := -Zsanitizer=kcfi -Zsanitizer-cfi-normalize-integers -Zmerge-functions=disabled KBUILD_RUSTFLAGS += $(RUSTC_FLAGS_CFI) export RUSTC_FLAGS_CFI endifbase-commit:
e6664f2b33db9b6811eb4cec109f06cb2b4f458d
Sami Tolvanen
Reviewed-byQuoted 10 lines · Gary Guo
Quoted 8 lines · Gary Guo
From: Gary Guo <[email protected]>
In Rust doc tests, there is a dummy
__module_firmware_test_initfunction generated by the example inmodule_firmware!'s documentation, which just returns zero. Many other documentation generates function that produces zero. LKP test robot reports aFlags::zeroedinstance; my local reproduction has aBounded::new::<0>.
Quoted 3 lines
Quoted 1 line
LLVM's MergeFunctionsPass incorrectly merge functions with the same KCFI
This should read "with different KCFI type" (obviously).
Also the comment in the Makefile. With both instances fixed:
The LLVM fix itself is trivial, and it looks like a pull request for it already exists. I assume we'll want to drop this flag for fixed LLVM versions when they're available.
Sami
Quoted 4 lines · Sami Tolvanen
The LLVM fix itself is trivial, and it looks like a pull request for it already exists. I assume we'll want to drop this flag for fixed LLVM versions when they're available.
I will link it in the commit message when I pick it up soon -- already did in https://github.com/Rust-for-Linux/linux/issues/1132.
I also added the -Z flag to https://github.com/Rust-for-Linux/linux/issues/2.
The -Zmerge-functions flag is internal but has been there since Rust 1.34.0 in upstream commit:
b91d211b4030 ("Add a target option "merge-functions" taking values
in ("disabled", "trampolines", or "aliases (the default)) to allow targets to opt out of the MergeFunctions LLVM pass. Also add a corresponding -Z option with the same name and values.")
so that should be fine, but I will ping upstream Rust so that they are aware we are using it.
And, yeah, agreed, we probably should remove the flag conditionally later on, to confirm the bug is gone etc.
Cheers,
Miguel
Quoted 19 lines · Gary Guo
From: Gary Guo <[email protected]>
In Rust doc tests, there is a dummy
__module_firmware_test_initfunction generated by the example inmodule_firmware!'s documentation, which just returns zero. Many other documentation generates function that produces zero. LKP test robot reports aFlags::zeroedinstance; my local reproduction has aBounded::new::<0>.LLVM's MergeFunctionsPass incorrectly merge functions with the same KCFI type, causing
__module_firmware_test_initbeing merged into one of the zero-returning function. As module init is invoked via indirect function call, KCFI is checked and this produces a KCFI failure.I've reported this bug to upstream LLVM [1]; in the mean time, disable function merging if CFI is enabled. No separate treatment is needed for
CONFIG_RUST_INLINE_HELPERS, as Clang does not enable function merging by default.
Quoted 4 lines
Reported-by kernel test robot <[email protected]>Signed-off-by Gary Guo <[email protected]>
[ LLVM already has a pending PR:
https://github.com/llvm/llvm-project/pull/217665
which solves the issue. In addition, I asked upstream Rust if the unstable -Zmerge-functions=disabled flag will remain around:
and it does indeed look like that will be the case. - Miguel ]
[ Fixed typos as discussed. Reworded slightly for other typos. - Miguel ]
Cheers,
Miguel