Skip to content

Commit 723acd7

Browse files
sean-jcgregkh
sean-jc
authored and
gregkh
committed
perf: Protect perf_guest_cbs with RCU
commit ff083a2 upstream. Protect perf_guest_cbs with RCU to fix multiple possible errors. Luckily, all paths that read perf_guest_cbs already require RCU protection, e.g. to protect the callback chains, so only the direct perf_guest_cbs touchpoints need to be modified. Bug #1 is a simple lack of WRITE_ONCE/READ_ONCE behavior to ensure perf_guest_cbs isn't reloaded between a !NULL check and a dereference. Fixed via the READ_ONCE() in rcu_dereference(). Bug #2 is that on weakly-ordered architectures, updates to the callbacks themselves are not guaranteed to be visible before the pointer is made visible to readers. Fixed by the smp_store_release() in rcu_assign_pointer() when the new pointer is non-NULL. Bug #3 is that, because the callbacks are global, it's possible for readers to run in parallel with an unregisters, and thus a module implementing the callbacks can be unloaded while readers are in flight, resulting in a use-after-free. Fixed by a synchronize_rcu() call when unregistering callbacks. Bug #1 escaped notice because it's extremely unlikely a compiler will reload perf_guest_cbs in this sequence. perf_guest_cbs does get reloaded for future derefs, e.g. for ->is_user_mode(), but the ->is_in_guest() guard all but guarantees the consumer will win the race, e.g. to nullify perf_guest_cbs, KVM has to completely exit the guest and teardown down all VMs before KVM start its module unload / unregister sequence. This also makes it all but impossible to encounter bug #3. Bug #2 has not been a problem because all architectures that register callbacks are strongly ordered and/or have a static set of callbacks. But with help, unloading kvm_intel can trigger bug #1 e.g. wrapping perf_guest_cbs with READ_ONCE in perf_misc_flags() while spamming kvm_intel module load/unload leads to: BUG: kernel NULL pointer dereference, address: 0000000000000000 #PF: supervisor read access in kernel mode #PF: error_code(0x0000) - not-present page PGD 0 P4D 0 Oops: 0000 [#1] PREEMPT SMP CPU: 6 PID: 1825 Comm: stress Not tainted 5.14.0-rc2+ #459 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:perf_misc_flags+0x1c/0x70 Call Trace: perf_prepare_sample+0x53/0x6b0 perf_event_output_forward+0x67/0x160 __perf_event_overflow+0x52/0xf0 handle_pmi_common+0x207/0x300 intel_pmu_handle_irq+0xcf/0x410 perf_event_nmi_handler+0x28/0x50 nmi_handle+0xc7/0x260 default_do_nmi+0x6b/0x170 exc_nmi+0x103/0x130 asm_exc_nmi+0x76/0xbf Fixes: 39447b3 ("perf: Enhance perf to allow for guest statistic collection from host") Signed-off-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20211111020738.2512932-2-seanjc@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent eadde28 commit 723acd7

9 files changed

Lines changed: 82 additions & 35 deletions

File tree

‎arch/arm/kernel/perf_callchain.c‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,10 @@ user_backtrace(struct frame_tail __user *tail,
6262
void
6363
perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
6464
{
65+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
6566
struct frame_tail __user *tail;
6667

67-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
68+
if (guest_cbs && guest_cbs->is_in_guest()) {
6869
/* We don't support guest os callchain now */
6970
return;
7071
}
@@ -98,9 +99,10 @@ callchain_trace(struct stackframe *fr,
9899
void
99100
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
100101
{
102+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
101103
struct stackframe fr;
102104

103-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
105+
if (guest_cbs && guest_cbs->is_in_guest()) {
104106
/* We don't support guest os callchain now */
105107
return;
106108
}
@@ -111,18 +113,21 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *re
111113

112114
unsigned long perf_instruction_pointer(struct pt_regs *regs)
113115
{
114-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
115-
return perf_guest_cbs->get_guest_ip();
116+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
117+
118+
if (guest_cbs && guest_cbs->is_in_guest())
119+
return guest_cbs->get_guest_ip();
116120

117121
return instruction_pointer(regs);
118122
}
119123

120124
unsigned long perf_misc_flags(struct pt_regs *regs)
121125
{
126+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
122127
int misc = 0;
123128

124-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
125-
if (perf_guest_cbs->is_user_mode())
129+
if (guest_cbs && guest_cbs->is_in_guest()) {
130+
if (guest_cbs->is_user_mode())
126131
misc |= PERF_RECORD_MISC_GUEST_USER;
127132
else
128133
misc |= PERF_RECORD_MISC_GUEST_KERNEL;

‎arch/arm64/kernel/perf_callchain.c‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,
102102
void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
103103
struct pt_regs *regs)
104104
{
105-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
105+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
106+
107+
if (guest_cbs && guest_cbs->is_in_guest()) {
106108
/* We don't support guest os callchain now */
107109
return;
108110
}
@@ -147,9 +149,10 @@ static bool callchain_trace(void *data, unsigned long pc)
147149
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
148150
struct pt_regs *regs)
149151
{
152+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
150153
struct stackframe frame;
151154

152-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
155+
if (guest_cbs && guest_cbs->is_in_guest()) {
153156
/* We don't support guest os callchain now */
154157
return;
155158
}
@@ -160,18 +163,21 @@ void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
160163

161164
unsigned long perf_instruction_pointer(struct pt_regs *regs)
162165
{
163-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
164-
return perf_guest_cbs->get_guest_ip();
166+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
167+
168+
if (guest_cbs && guest_cbs->is_in_guest())
169+
return guest_cbs->get_guest_ip();
165170

166171
return instruction_pointer(regs);
167172
}
168173

169174
unsigned long perf_misc_flags(struct pt_regs *regs)
170175
{
176+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
171177
int misc = 0;
172178

173-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
174-
if (perf_guest_cbs->is_user_mode())
179+
if (guest_cbs && guest_cbs->is_in_guest()) {
180+
if (guest_cbs->is_user_mode())
175181
misc |= PERF_RECORD_MISC_GUEST_USER;
176182
else
177183
misc |= PERF_RECORD_MISC_GUEST_KERNEL;

‎arch/csky/kernel/perf_callchain.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,11 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,
8686
void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
8787
struct pt_regs *regs)
8888
{
89+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
8990
unsigned long fp = 0;
9091

9192
/* C-SKY does not support virtualization. */
92-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
93+
if (guest_cbs && guest_cbs->is_in_guest())
9394
return;
9495

9596
fp = regs->regs[4];
@@ -110,10 +111,11 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
110111
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
111112
struct pt_regs *regs)
112113
{
114+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
113115
struct stackframe fr;
114116

115117
/* C-SKY does not support virtualization. */
116-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
118+
if (guest_cbs && guest_cbs->is_in_guest()) {
117119
pr_warn("C-SKY does not support perf in guest mode!");
118120
return;
119121
}

‎arch/nds32/kernel/perf_event_cpu.c‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,6 +1363,7 @@ void
13631363
perf_callchain_user(struct perf_callchain_entry_ctx *entry,
13641364
struct pt_regs *regs)
13651365
{
1366+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
13661367
unsigned long fp = 0;
13671368
unsigned long gp = 0;
13681369
unsigned long lp = 0;
@@ -1371,7 +1372,7 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry,
13711372

13721373
leaf_fp = 0;
13731374

1374-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
1375+
if (guest_cbs && guest_cbs->is_in_guest()) {
13751376
/* We don't support guest os callchain now */
13761377
return;
13771378
}
@@ -1479,9 +1480,10 @@ void
14791480
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
14801481
struct pt_regs *regs)
14811482
{
1483+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
14821484
struct stackframe fr;
14831485

1484-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
1486+
if (guest_cbs && guest_cbs->is_in_guest()) {
14851487
/* We don't support guest os callchain now */
14861488
return;
14871489
}
@@ -1493,20 +1495,23 @@ perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
14931495

14941496
unsigned long perf_instruction_pointer(struct pt_regs *regs)
14951497
{
1498+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
1499+
14961500
/* However, NDS32 does not support virtualization */
1497-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
1498-
return perf_guest_cbs->get_guest_ip();
1501+
if (guest_cbs && guest_cbs->is_in_guest())
1502+
return guest_cbs->get_guest_ip();
14991503

15001504
return instruction_pointer(regs);
15011505
}
15021506

15031507
unsigned long perf_misc_flags(struct pt_regs *regs)
15041508
{
1509+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
15051510
int misc = 0;
15061511

15071512
/* However, NDS32 does not support virtualization */
1508-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
1509-
if (perf_guest_cbs->is_user_mode())
1513+
if (guest_cbs && guest_cbs->is_in_guest()) {
1514+
if (guest_cbs->is_user_mode())
15101515
misc |= PERF_RECORD_MISC_GUEST_USER;
15111516
else
15121517
misc |= PERF_RECORD_MISC_GUEST_KERNEL;

‎arch/riscv/kernel/perf_callchain.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,11 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,
6060
void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
6161
struct pt_regs *regs)
6262
{
63+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
6364
unsigned long fp = 0;
6465

6566
/* RISC-V does not support perf in guest mode. */
66-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
67+
if (guest_cbs && guest_cbs->is_in_guest())
6768
return;
6869

6970
fp = regs->s0;
@@ -84,8 +85,10 @@ void notrace walk_stackframe(struct task_struct *task,
8485
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
8586
struct pt_regs *regs)
8687
{
88+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
89+
8790
/* RISC-V does not support perf in guest mode. */
88-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
91+
if (guest_cbs && guest_cbs->is_in_guest()) {
8992
pr_warn("RISC-V does not support perf in guest mode!");
9093
return;
9194
}

‎arch/x86/events/core.c‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2545,10 +2545,11 @@ static bool perf_hw_regs(struct pt_regs *regs)
25452545
void
25462546
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
25472547
{
2548+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
25482549
struct unwind_state state;
25492550
unsigned long addr;
25502551

2551-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
2552+
if (guest_cbs && guest_cbs->is_in_guest()) {
25522553
/* TODO: We don't support guest os callchain now */
25532554
return;
25542555
}
@@ -2648,10 +2649,11 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent
26482649
void
26492650
perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
26502651
{
2652+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
26512653
struct stack_frame frame;
26522654
const struct stack_frame __user *fp;
26532655

2654-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
2656+
if (guest_cbs && guest_cbs->is_in_guest()) {
26552657
/* TODO: We don't support guest os callchain now */
26562658
return;
26572659
}
@@ -2728,18 +2730,21 @@ static unsigned long code_segment_base(struct pt_regs *regs)
27282730

27292731
unsigned long perf_instruction_pointer(struct pt_regs *regs)
27302732
{
2731-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
2732-
return perf_guest_cbs->get_guest_ip();
2733+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
2734+
2735+
if (guest_cbs && guest_cbs->is_in_guest())
2736+
return guest_cbs->get_guest_ip();
27332737

27342738
return regs->ip + code_segment_base(regs);
27352739
}
27362740

27372741
unsigned long perf_misc_flags(struct pt_regs *regs)
27382742
{
2743+
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
27392744
int misc = 0;
27402745

2741-
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
2742-
if (perf_guest_cbs->is_user_mode())
2746+
if (guest_cbs && guest_cbs->is_in_guest()) {
2747+
if (guest_cbs->is_user_mode())
27432748
misc |= PERF_RECORD_MISC_GUEST_USER;
27442749
else
27452750
misc |= PERF_RECORD_MISC_GUEST_KERNEL;

‎arch/x86/events/intel/core.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2586,6 +2586,7 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
25862586
{
25872587
struct perf_sample_data data;
25882588
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
2589+
struct perf_guest_info_callbacks *guest_cbs;
25892590
int bit;
25902591
int handled = 0;
25912592

@@ -2651,9 +2652,11 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
26512652
*/
26522653
if (__test_and_clear_bit(GLOBAL_STATUS_TRACE_TOPAPMI_BIT, (unsigned long *)&status)) {
26532654
handled++;
2654-
if (unlikely(perf_guest_cbs && perf_guest_cbs->is_in_guest() &&
2655-
perf_guest_cbs->handle_intel_pt_intr))
2656-
perf_guest_cbs->handle_intel_pt_intr();
2655+
2656+
guest_cbs = perf_get_guest_cbs();
2657+
if (unlikely(guest_cbs && guest_cbs->is_in_guest() &&
2658+
guest_cbs->handle_intel_pt_intr))
2659+
guest_cbs->handle_intel_pt_intr();
26572660
else
26582661
intel_pt_interrupt();
26592662
}

‎include/linux/perf_event.h‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1235,7 +1235,18 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,
12351235
enum perf_bpf_event_type type,
12361236
u16 flags);
12371237

1238-
extern struct perf_guest_info_callbacks *perf_guest_cbs;
1238+
extern struct perf_guest_info_callbacks __rcu *perf_guest_cbs;
1239+
static inline struct perf_guest_info_callbacks *perf_get_guest_cbs(void)
1240+
{
1241+
/*
1242+
* Callbacks are RCU-protected and must be READ_ONCE to avoid reloading
1243+
* the callbacks between a !NULL check and dereferences, to ensure
1244+
* pending stores/changes to the callback pointers are visible before a
1245+
* non-NULL perf_guest_cbs is visible to readers, and to prevent a
1246+
* module from unloading callbacks while readers are active.
1247+
*/
1248+
return rcu_dereference(perf_guest_cbs);
1249+
}
12391250
extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
12401251
extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
12411252

‎kernel/events/core.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6395,18 +6395,25 @@ static void perf_pending_event(struct irq_work *entry)
63956395
* Later on, we might change it to a list if there is
63966396
* another virtualization implementation supporting the callbacks.
63976397
*/
6398-
struct perf_guest_info_callbacks *perf_guest_cbs;
6398+
struct perf_guest_info_callbacks __rcu *perf_guest_cbs;
63996399

64006400
int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
64016401
{
6402-
perf_guest_cbs = cbs;
6402+
if (WARN_ON_ONCE(rcu_access_pointer(perf_guest_cbs)))
6403+
return -EBUSY;
6404+
6405+
rcu_assign_pointer(perf_guest_cbs, cbs);
64036406
return 0;
64046407
}
64056408
EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);
64066409

64076410
int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
64086411
{
6409-
perf_guest_cbs = NULL;
6412+
if (WARN_ON_ONCE(rcu_access_pointer(perf_guest_cbs) != cbs))
6413+
return -EINVAL;
6414+
6415+
rcu_assign_pointer(perf_guest_cbs, NULL);
6416+
synchronize_rcu();
64106417
return 0;
64116418
}
64126419
EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);

0 commit comments

Comments
 (0)