On Thu, Oct 10, 2024 at 03:54:11PM -0700, Nathan Chancellor wrote:
Hi Mike,
On Wed, Oct 09, 2024 at 09:08:14PM +0300, Mike Rapoport wrote:
From: "Mike Rapoport (Microsoft)" <rppt@xxxxxxxxxx>
When module text memory will be allocated with ROX permissions, the
memory at the actual address where the module will live will contain
invalid instructions and there will be a writable copy that contains the
actual module code.
Update relocations and alternatives patching to deal with it.
Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
I bisected a boot failure that I see with CONFIG_CFI_CLANG enabled to
this change as commit be712757cabd ("x86/module: perpare module loading
for ROX allocations of text") in -next.
[ 0.000000] Linux version 6.12.0-rc2-00140-gbe712757cabd (nathan@n3-xlarge-x86) (ClangBuiltLinux clang version 19.1.0 (https://github.com/llvm/llvm-project.git a4bf6cd7cfb1a1421ba92bca9d017b49936c55e4), ClangBuiltLinux LLD 19.1.0 (https://github.com/llvm/llvm-project.git a4bf6cd7cfb1a1421ba92bca9d017b49936c55e4)) #1 SMP PREEMPT_DYNAMIC Thu Oct 10 22:42:57 UTC 2024
...
[ 0.092204] Speculative Store Bypass: Mitigation: Speculative Store Bypass disabled via prctl
[ 0.093207] TAA: Mitigation: TSX disabled
[ 0.093711] MMIO Stale Data: Mitigation: Clear CPU buffers
[ 0.094228] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers'
[ 0.095203] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
[ 0.096203] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
[ 0.097203] x86/fpu: Supporting XSAVE feature 0x020: 'AVX-512 opmask'
[ 0.098003] x86/fpu: Supporting XSAVE feature 0x040: 'AVX-512 Hi256'
[ 0.098203] x86/fpu: Supporting XSAVE feature 0x080: 'AVX-512 ZMM_Hi256'
[ 0.099203] x86/fpu: Supporting XSAVE feature 0x200: 'Protection Keys User registers'
[ 0.100204] x86/fpu: xstate_offset[2]: 576, xstate_sizes[2]: 256
[ 0.101204] x86/fpu: xstate_offset[5]: 832, xstate_sizes[5]: 64
[ 0.102203] x86/fpu: xstate_offset[6]: 896, xstate_sizes[6]: 512
[ 0.103204] x86/fpu: xstate_offset[7]: 1408, xstate_sizes[7]: 1024
[ 0.104051] x86/fpu: xstate_offset[9]: 2432, xstate_sizes[9]: 8
[ 0.104204] x86/fpu: Enabled xstate features 0x2e7, context size is 2440 bytes, using 'compacted' format.
then nothing after that. Boot is successful if CFI is not enabled (the
initrd will just shutdown the machine after printing the version string).
If there is any further information I can provide or patches I can test,
I am more than happy to do so.
I overlooked how cfi_*_callers routines update addr.
This patch should fix it:
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 3b3fa93af3b1..cf782f431110 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1148,11 +1148,13 @@ static int cfi_disable_callers(s32 *start, s32 *end, struct module *mod)
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
- void *wr_addr = module_writable_address(mod, addr);
+ void *wr_addr;
u32 hash;
addr -= fineibt_caller_size;
- hash = decode_caller_hash(addr);
+ wr_addr = module_writable_address(mod, addr);
+ hash = decode_caller_hash(wr_addr);
+
if (!hash) /* nocfi callers */
continue;
@@ -1172,11 +1174,12 @@ static int cfi_enable_callers(s32 *start, s32 *end, struct module *mod)
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
- void *wr_addr = module_writable_address(mod, addr);
+ void *wr_addr;
u32 hash;
addr -= fineibt_caller_size;
- hash = decode_caller_hash(addr);
+ wr_addr = module_writable_address(mod, addr);
+ hash = decode_caller_hash(wr_addr);
if (!hash) /* nocfi callers */
continue;
@@ -1249,11 +1252,12 @@ static int cfi_rand_callers(s32 *start, s32 *end, struct module *mod)
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
- void *wr_addr = module_writable_address(mod, addr);
+ void *wr_addr;
u32 hash;
addr -= fineibt_caller_size;
- hash = decode_caller_hash(addr);
+ wr_addr = module_writable_address(mod, addr);
+ hash = decode_caller_hash(wr_addr);
if (hash) {
hash = -cfi_rehash(hash);
text_poke_early(wr_addr + 2, &hash, 4);
@@ -1269,14 +1273,15 @@ static int cfi_rewrite_callers(s32 *start, s32 *end, struct module *mod)
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
- void *wr_addr = module_writable_address(mod, addr);
+ void *wr_addr;
u32 hash;
addr -= fineibt_caller_size;
- hash = decode_caller_hash(addr);
+ wr_addr = module_writable_address(mod, addr);
+ hash = decode_caller_hash(wr_addr);
if (hash) {
text_poke_early(wr_addr, fineibt_caller_start, fineibt_caller_size);
- WARN_ON(*(u32 *)(addr + fineibt_caller_hash) != 0x12345678);
+ WARN_ON(*(u32 *)(wr_addr + fineibt_caller_hash) != 0x12345678);
text_poke_early(wr_addr + fineibt_caller_hash, &hash, 4);
}
/* rely on apply_retpolines() */
Cheers,
Nathan
--
Sincerely yours,
Mike.
