Hi folks,
This series fixes __ro_after_init keys used in modules (courtesy of PeterZ) and
flags more keys as __ro_after_init. Further context for why I'm looking into
this here: http://lore.kernel.org/r/[email protected]
Compile & boot tested for x86_64_defconfig and i386_defconfig.
@Peter, regarding making __use_tsc x86_32, I hit a few snags.
Currently, for the static key to be enabled, we (mostly) need:
o X86_FEATURE_TSC is in CPUID
o determine_cpu_tsc_frequencies passes
All X86_64 systems have a TSC, so the CPUID feature is a given there.
Calibrating the TSC can end up depending on different things:
o CPUID accepting 0x16 as eax input (cf. cpu_khz_from_cpuid())
o MSR_FSB_FREQ being available (cf. cpu_khz_from_msr())
o pit_hpet_ptimer_calibrate_cpu() doesn't mess up
I couldn't find any guarantees for X86_64 on having the processor frequency
information CPUID leaf, nor for the FSB_FREQ MSR (both tsc_msr_cpu_ids and
the SDM seem to point at only a handful of models).
pit_hpet_ptimer_calibrate_cpu() relies on having either HPET or the ACPI PM
timer, the latter being widely available, though X86_PM_TIMER can be
disabled via EXPERT.
The question here is: are there any guarantees that at least one of these
can be relied upon for x86_64?
And with all of that, there is still the "apicpmtimer" cmdline option which
currently invokes notsc_setup() on x86_64. The justification I found for it was
in 0c3749c41f5e ("[PATCH] x86_64: Calibrate APIC timer using PM timer"):
"""
On some broken motherboards (at least one NForce3 based AMD64 laptop)
the PIT timer runs at a incorrect frequency. This patch adds a new
option "apicpmtimer" that allows to use the APIC timer and calibrate it
using the PMTimer.
"""
Revisions
=========
v1 -> v2
++++++++
- Collected tags (Josh, Sean)
- Fixed CONFIG_JUMP_LABEL=n compile fail (lkp)
Cheers,
Valentin
Peter Zijlstra (1):
jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
Valentin Schneider (4):
context_tracking: Make context_tracking_key __ro_after_init
x86/kvm: Make kvm_async_pf_enabled __ro_after_init
x86/speculation: Make mds_user_clear __ro_after_init
x86/tsc: Make __use_tsc __ro_after_init
arch/x86/kernel/cpu/bugs.c | 2 +-
arch/x86/kernel/kvm.c | 2 +-
arch/x86/kernel/tsc.c | 2 +-
include/asm-generic/sections.h | 5 ++++
include/linux/jump_label.h | 3 +++
init/main.c | 1 +
kernel/context_tracking.c | 2 +-
kernel/jump_label.c | 49 ++++++++++++++++++++++++++++++++++
8 files changed, 62 insertions(+), 4 deletions(-)
--
2.43.0
From: Peter Zijlstra <[email protected]>
When a static_key is marked ro_after_init, its state will never change
(after init), therefore jump_label_update() will never need to iterate
the entries, and thus module load won't actually need to track this --
avoiding the static_key::next write.
Therefore, mark these keys such that jump_label_add_module() might
recognise them and avoid the modification.
Use the special state: 'static_key_linked(key) && !static_key_mod(key)'
to denote such keys.
jump_label_add_module() does not exist under CONFIG_JUMP_LABEL=n, so the
newly-introduced jump_label_ro() can be defined as a nop for that
configuration.
Link: http://lore.kernel.org/r/[email protected]
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
[Added comments and build fix]
Signed-off-by: Valentin Schneider <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
---
include/asm-generic/sections.h | 5 ++++
include/linux/jump_label.h | 3 +++
init/main.c | 1 +
kernel/jump_label.c | 49 ++++++++++++++++++++++++++++++++++
4 files changed, 58 insertions(+)
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index db13bb620f527..c768de6f19a9a 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -180,6 +180,11 @@ static inline bool is_kernel_rodata(unsigned long addr)
addr < (unsigned long)__end_rodata;
}
+static inline bool is_kernel_ro_after_init(unsigned long addr)
+{
+ return addr >= (unsigned long)__start_ro_after_init &&
+ addr < (unsigned long)__end_ro_after_init;
+}
/**
* is_kernel_inittext - checks if the pointer address is located in the
* .init.text section
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index f0a949b7c9733..3b103d88c139e 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -216,6 +216,7 @@ extern struct jump_entry __start___jump_table[];
extern struct jump_entry __stop___jump_table[];
extern void jump_label_init(void);
+extern void jump_label_ro(void);
extern void jump_label_lock(void);
extern void jump_label_unlock(void);
extern void arch_jump_label_transform(struct jump_entry *entry,
@@ -265,6 +266,8 @@ static __always_inline void jump_label_init(void)
static_key_initialized = true;
}
+static __always_inline void jump_label_ro(void) { }
+
static __always_inline bool static_key_false(struct static_key *key)
{
if (unlikely_notrace(static_key_count(key) > 0))
diff --git a/init/main.c b/init/main.c
index e24b0780fdff7..5f51d8b910dc1 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1407,6 +1407,7 @@ static void mark_readonly(void)
* insecure pages which are W+X.
*/
rcu_barrier();
+ jump_label_ro();
mark_rodata_ro();
rodata_test();
} else
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d9c822bbffb8d..25a5ed1147841 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -530,6 +530,45 @@ void __init jump_label_init(void)
cpus_read_unlock();
}
+static inline bool static_key_sealed(struct static_key *key)
+{
+ return (key->type & JUMP_TYPE_LINKED) && !(key->type & ~JUMP_TYPE_MASK);
+}
+
+static inline void static_key_seal(struct static_key *key)
+{
+ unsigned long type = key->type & JUMP_TYPE_TRUE;
+ key->type = JUMP_TYPE_LINKED | type;
+}
+
+void jump_label_ro(void)
+{
+ struct jump_entry *iter_start = __start___jump_table;
+ struct jump_entry *iter_stop = __stop___jump_table;
+ struct jump_entry *iter;
+
+ if (WARN_ON_ONCE(!static_key_initialized))
+ return;
+
+ cpus_read_lock();
+ jump_label_lock();
+
+ for (iter = iter_start; iter < iter_stop; iter++) {
+ struct static_key *iterk = jump_entry_key(iter);
+
+ if (!is_kernel_ro_after_init((unsigned long)iterk))
+ continue;
+
+ if (static_key_sealed(iterk))
+ continue;
+
+ static_key_seal(iterk);
+ }
+
+ jump_label_unlock();
+ cpus_read_unlock();
+}
+
#ifdef CONFIG_MODULES
enum jump_label_type jump_label_init_type(struct jump_entry *entry)
@@ -650,6 +689,15 @@ static int jump_label_add_module(struct module *mod)
static_key_set_entries(key, iter);
continue;
}
+
+ /*
+ * If the key was sealed at init, then there's no need to keep a
+ * reference to its module entries - just patch them now and be
+ * done with it.
+ */
+ if (static_key_sealed(key))
+ goto do_poke;
+
jlm = kzalloc(sizeof(struct static_key_mod), GFP_KERNEL);
if (!jlm)
return -ENOMEM;
@@ -675,6 +723,7 @@ static int jump_label_add_module(struct module *mod)
static_key_set_linked(key);
/* Only update if we've changed from our initial state */
+do_poke:
if (jump_label_type(iter) != jump_label_init_type(iter))
__jump_label_update(key, iter, iter_stop, true);
}
--
2.43.0
context_tracking_key is only ever enabled in __init ct_cpu_tracker_user(),
so mark it as __ro_after_init.
Signed-off-by: Valentin Schneider <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
---
kernel/context_tracking.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 6ef0b35fc28c5..cc4f3a57f848c 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -432,7 +432,7 @@ static __always_inline void ct_kernel_enter(bool user, int offset) { }
#define CREATE_TRACE_POINTS
#include <trace/events/context_tracking.h>
-DEFINE_STATIC_KEY_FALSE(context_tracking_key);
+DEFINE_STATIC_KEY_FALSE_RO(context_tracking_key);
EXPORT_SYMBOL_GPL(context_tracking_key);
static noinstr bool context_tracking_recursion_enter(void)
--
2.43.0
mds_user_clear is only ever enabled in:
o __init mds_select_mitigation()
o __init taa_select_mitigation()
o __init mmio_select_mitigation()
mark it as __ro_after_init.
Signed-off-by: Valentin Schneider <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
---
arch/x86/kernel/cpu/bugs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index bb0ab8466b919..bab36096015d8 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -112,7 +112,7 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
DEFINE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
/* Control MDS CPU buffer clear before returning to user space */
-DEFINE_STATIC_KEY_FALSE(mds_user_clear);
+DEFINE_STATIC_KEY_FALSE_RO(mds_user_clear);
EXPORT_SYMBOL_GPL(mds_user_clear);
/* Control MDS CPU buffer clear before idling (halt, mwait) */
DEFINE_STATIC_KEY_FALSE(mds_idle_clear);
--
2.43.0
kvm_async_pf_enabled is only ever enabled in __init kvm_guest_init(), so
mark it as __ro_after_init.
Reviewed-by: Sean Christopherson <[email protected]>
Signed-off-by: Valentin Schneider <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
---
arch/x86/kernel/kvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index dfe9945b9bece..8f48ce7adfb28 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -44,7 +44,7 @@
#include <asm/svm.h>
#include <asm/e820/api.h>
-DEFINE_STATIC_KEY_FALSE(kvm_async_pf_enabled);
+DEFINE_STATIC_KEY_FALSE_RO(kvm_async_pf_enabled);
static int kvmapf = 1;
--
2.43.0
__use_tsc is only ever enabled in __init tsc_enable_sched_clock(), so mark
it as __ro_after_init.
Signed-off-by: Valentin Schneider <[email protected]>
Acked-by: Josh Poimboeuf <[email protected]>
---
arch/x86/kernel/tsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 15f97c0abc9d0..f19b42ea40573 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -44,7 +44,7 @@ EXPORT_SYMBOL(tsc_khz);
static int __read_mostly tsc_unstable;
static unsigned int __initdata tsc_early_khz;
-static DEFINE_STATIC_KEY_FALSE(__use_tsc);
+static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc);
int tsc_clocksource_reliable;
--
2.43.0
Hello,
kernel test robot noticed "WARNING:at_kernel/jump_label.c:#jump_label_del_module" on:
commit: b42c2f5e8e4d403b5891b566b015177c8f471858 ("[PATCH v2 2/5] context_tracking: Make context_tracking_key __ro_after_init")
url: https://github.com/intel-lab-lkp/linux/commits/Valentin-Schneider/jump_label-module-Don-t-alloc-static_key_mod-for-__ro_after_init-keys/20240207-014501
base: https://git.kernel.org/cgit/virt/kvm/kvm.git queue
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH v2 2/5] context_tracking: Make context_tracking_key __ro_after_init
in testcase: ltp
version: ltp-x86_64-14c1f76-1_20230715
with following parameters:
test: kernel_misc
compiler: gcc-12
test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz (Ivy Bridge) with 16G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]
[ 215.214621][ T3608] ------------[ cut here ]------------
[ 215.214849][ T3608] WARNING: CPU: 1 PID: 3608 at kernel/jump_label.c:764 jump_label_del_module (kernel/jump_label.c:764 (discriminator 1))
[ 215.215123][ T3608] Modules linked in: rcutorture(-) torture netconsole btrfs blake2b_generic xor raid6_pq zstd_compress libcrc32c intel_rapl_msr intel_rapl_common x86_pkg_temp_thermal intel_powerclamp sd_mod t10_pi coretemp crc64_rocksoft_generic crc64_rocksoft kvm_intel crc64 sg ipmi_devintf i915 ipmi_msghandler kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 drm_buddy rapl intel_gtt intel_cstate drm_display_helper mxm_wmi firewire_ohci ttm ahci firewire_core libahci drm_kms_helper intel_uncore crc_itu_t libata i2c_i801 video i2c_smbus lpc_ich wmi binfmt_misc fuse drm ip_tables
[ 215.216369][ T3608] CPU: 1 PID: 3608 Comm: modprobe Not tainted 6.7.0-rc7-00234-gb42c2f5e8e4d #1
[ 215.216637][ T3608] Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0097.2012.1228.1346 12/28/2012
[ 215.216918][ T3608] RIP: 0010:jump_label_del_module (kernel/jump_label.c:764 (discriminator 1))
[ 215.217162][ T3608] Code: 03 75 35 83 e6 03 48 09 f0 4c 89 fe 48 c1 ee 03 80 3c 2e 00 0f 85 cf 01 00 00 49 89 45 08 e9 bf fe ff ff 0f 0b e9 6b ff ff ff <0f> 0b e9 64 ff ff ff 0f 0b e9 e2 fe ff ff 0f 0b eb c7 0f 0b e9 19
All code
========
0: 03 75 35 add 0x35(%rbp),%esi
3: 83 e6 03 and $0x3,%esi
6: 48 09 f0 or %rsi,%rax
9: 4c 89 fe mov %r15,%rsi
c: 48 c1 ee 03 shr $0x3,%rsi
10: 80 3c 2e 00 cmpb $0x0,(%rsi,%rbp,1)
14: 0f 85 cf 01 00 00 jne 0x1e9
1a: 49 89 45 08 mov %rax,0x8(%r13)
1e: e9 bf fe ff ff jmpq 0xfffffffffffffee2
23: 0f 0b ud2
25: e9 6b ff ff ff jmpq 0xffffffffffffff95
2a:* 0f 0b ud2 <-- trapping instruction
2c: e9 64 ff ff ff jmpq 0xffffffffffffff95
31: 0f 0b ud2
33: e9 e2 fe ff ff jmpq 0xffffffffffffff1a
38: 0f 0b ud2
3a: eb c7 jmp 0x3
3c: 0f 0b ud2
3e: e9 .byte 0xe9
3f: 19 .byte 0x19
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: e9 64 ff ff ff jmpq 0xffffffffffffff6b
7: 0f 0b ud2
9: e9 e2 fe ff ff jmpq 0xfffffffffffffef0
e: 0f 0b ud2
10: eb c7 jmp 0xffffffffffffffd9
12: 0f 0b ud2
14: e9 .byte 0xe9
15: 19 .byte 0x19
[ 215.217593][ T3608] RSP: 0018:ffffc9000159fd38 EFLAGS: 00010297
[ 215.217877][ T3608] RAX: 1ffffffff089982f RBX: 0000000000000000 RCX: ffffffffc14dc060
[ 215.218140][ T3608] RDX: ffffffffc1c6b580 RSI: 0000000000000002 RDI: ffffffffc14dc000
[ 215.218409][ T3608] RBP: dffffc0000000000 R08: ffffffffc1c6b8b8 R09: fffff520002b3fa6
[ 215.218636][ T3608] R10: ffffc9000159fd37 R11: 0000000000000001 R12: ffffffff844cc178
[ 215.218935][ T3608] R13: ffffffff844cc170 R14: ffffffffc14dc008 R15: ffffffff844cc178
[ 215.219166][ T3608] FS: 00007f5e44068580(0000) GS:ffff888348080000(0000) knlGS:0000000000000000
[ 215.219459][ T3608] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 215.219717][ T3608] CR2: 00007fceb9a28008 CR3: 00000001a718e005 CR4: 00000000001706f0
[ 215.219956][ T3608] Call Trace:
[ 215.220126][ T3608] <TASK>
[ 215.220307][ T3608] ? __warn (kernel/panic.c:677)
[ 215.220503][ T3608] ? jump_label_del_module (kernel/jump_label.c:764 (discriminator 1))
[ 215.220745][ T3608] ? report_bug (lib/bug.c:180 lib/bug.c:219)
[ 215.220961][ T3608] ? handle_bug (arch/x86/kernel/traps.c:237)
[ 215.221126][ T3608] ? exc_invalid_op (arch/x86/kernel/traps.c:258 (discriminator 1))
[ 215.221288][ T3608] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:568)
[ 215.221543][ T3608] ? jump_label_del_module (kernel/jump_label.c:764 (discriminator 1))
[ 215.221835][ T3608] ? jump_label_add_module (kernel/jump_label.c:787)
[ 215.222061][ T3608] jump_label_module_notify (kernel/jump_label.c:32 kernel/jump_label.c:807)
[ 215.222255][ T3608] notifier_call_chain (kernel/notifier.c:95)
[ 215.222433][ T3608] ? mutex_unlock (arch/x86/include/asm/atomic64_64.h:109 include/linux/atomic/atomic-arch-fallback.h:4308 include/linux/atomic/atomic-long.h:1499 include/linux/atomic/atomic-instrumented.h:4446 kernel/locking/mutex.c:181 kernel/locking/mutex.c:540)
[ 215.222603][ T3608] blocking_notifier_call_chain (kernel/notifier.c:389 kernel/notifier.c:376)
[ 215.222845][ T3608] __do_sys_delete_module+0x32a/0x540
[ 215.223066][ T3608] ? module_flags (kernel/module/main.c:698)
[ 215.223239][ T3608] ? __x64_sys_close (fs/open.c:1590 fs/open.c:1572 fs/open.c:1572)
[ 215.223526][ T3608] ? kmem_cache_free (mm/slub.c:1826 mm/slub.c:3809 mm/slub.c:3831)
[ 215.223772][ T3608] do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
[ 215.224010][ T3608] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:129)
[ 215.224246][ T3608] RIP: 0033:0x7f5e44177847
[ 215.224423][ T3608] Code: 73 01 c3 48 8b 0d b9 85 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 89 85 0c 00 f7 d8 64 89 01 48
All code
========
0: 73 01 jae 0x3
2: c3 retq
3: 48 8b 0d b9 85 0c 00 mov 0xc85b9(%rip),%rcx # 0xc85c3
a: f7 d8 neg %eax
c: 64 89 01 mov %eax,%fs:(%rcx)
f: 48 83 c8 ff or $0xffffffffffffffff,%rax
13: c3 retq
14: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
1b: 00 00 00
1e: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
23: b8 b0 00 00 00 mov $0xb0,%eax
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 retq
33: 48 8b 0d 89 85 0c 00 mov 0xc8589(%rip),%rcx # 0xc85c3
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W
Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 73 01 jae 0x9
8: c3 retq
9: 48 8b 0d 89 85 0c 00 mov 0xc8589(%rip),%rcx # 0xc8599
10: f7 d8 neg %eax
12: 64 89 01 mov %eax,%fs:(%rcx)
15: 48 rex.W
[ 215.224940][ T3608] RSP: 002b:00007ffedaff1e88 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[ 215.225209][ T3608] RAX: ffffffffffffffda RBX: 0000562acd24d150 RCX: 00007f5e44177847
[ 215.225467][ T3608] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000562acd24d1b8
[ 215.225754][ T3608] RBP: 0000000000000000 R08: 1999999999999999 R09: 0000000000000000
[ 215.226033][ T3608] R10: 00007f5e441eaac0 R11: 0000000000000206 R12: 0000000000000000
[ 215.226303][ T3608] R13: 0000000000000000 R14: 00007ffedaff1ec0 R15: 00007ffedaff3378
[ 215.226579][ T3608] </TASK>
[ 215.226794][ T3608] ---[ end trace 0000000000000000 ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240219/[email protected]
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki