Skip to content

Commit

Permalink
perf: Protect perf_guest_cbs with RCU
Browse files Browse the repository at this point in the history
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 android-rpi#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 android-rpi#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 android-rpi#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 android-rpi#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 android-rpi#3.

Bug android-rpi#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 android-rpi#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 [android-rpi#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 <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Paolo Bonzini <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
sean-jc authored and gregkh committed Jan 20, 2022
1 parent bd2aed0 commit 9b45f20
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 35 deletions.
17 changes: 11 additions & 6 deletions arch/arm/kernel/perf_callchain.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ user_backtrace(struct frame_tail __user *tail,
void
perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct frame_tail __user *tail;

if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (guest_cbs && guest_cbs->is_in_guest()) {
/* We don't support guest os callchain now */
return;
}
Expand Down Expand Up @@ -98,9 +99,10 @@ callchain_trace(struct stackframe *fr,
void
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct stackframe fr;

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

unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
return perf_guest_cbs->get_guest_ip();
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();

if (guest_cbs && guest_cbs->is_in_guest())
return guest_cbs->get_guest_ip();

return instruction_pointer(regs);
}

unsigned long perf_misc_flags(struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
int misc = 0;

if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (perf_guest_cbs->is_user_mode())
if (guest_cbs && guest_cbs->is_in_guest()) {
if (guest_cbs->is_user_mode())
misc |= PERF_RECORD_MISC_GUEST_USER;
else
misc |= PERF_RECORD_MISC_GUEST_KERNEL;
Expand Down
18 changes: 12 additions & 6 deletions arch/arm64/kernel/perf_callchain.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ compat_user_backtrace(struct compat_frame_tail __user *tail,
void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();

if (guest_cbs && guest_cbs->is_in_guest()) {
/* We don't support guest os callchain now */
return;
}
Expand Down Expand Up @@ -147,9 +149,10 @@ static int callchain_trace(struct stackframe *frame, void *data)
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct stackframe frame;

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

unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
return perf_guest_cbs->get_guest_ip();
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();

if (guest_cbs && guest_cbs->is_in_guest())
return guest_cbs->get_guest_ip();

return instruction_pointer(regs);
}

unsigned long perf_misc_flags(struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
int misc = 0;

if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (perf_guest_cbs->is_user_mode())
if (guest_cbs && guest_cbs->is_in_guest()) {
if (guest_cbs->is_user_mode())
misc |= PERF_RECORD_MISC_GUEST_USER;
else
misc |= PERF_RECORD_MISC_GUEST_KERNEL;
Expand Down
6 changes: 4 additions & 2 deletions arch/csky/kernel/perf_callchain.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,
void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
unsigned long fp = 0;

/* C-SKY does not support virtualization. */
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
if (guest_cbs && guest_cbs->is_in_guest())
return;

fp = regs->regs[4];
Expand All @@ -110,10 +111,11 @@ void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct stackframe fr;

/* C-SKY does not support virtualization. */
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (guest_cbs && guest_cbs->is_in_guest()) {
pr_warn("C-SKY does not support perf in guest mode!");
return;
}
Expand Down
17 changes: 11 additions & 6 deletions arch/nds32/kernel/perf_event_cpu.c
Original file line number Diff line number Diff line change
Expand Up @@ -1363,6 +1363,7 @@ void
perf_callchain_user(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
unsigned long fp = 0;
unsigned long gp = 0;
unsigned long lp = 0;
Expand All @@ -1371,7 +1372,7 @@ perf_callchain_user(struct perf_callchain_entry_ctx *entry,

leaf_fp = 0;

if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (guest_cbs && guest_cbs->is_in_guest()) {
/* We don't support guest os callchain now */
return;
}
Expand Down Expand Up @@ -1479,9 +1480,10 @@ void
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct stackframe fr;

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

unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();

/* However, NDS32 does not support virtualization */
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
return perf_guest_cbs->get_guest_ip();
if (guest_cbs && guest_cbs->is_in_guest())
return guest_cbs->get_guest_ip();

return instruction_pointer(regs);
}

unsigned long perf_misc_flags(struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
int misc = 0;

/* However, NDS32 does not support virtualization */
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (perf_guest_cbs->is_user_mode())
if (guest_cbs && guest_cbs->is_in_guest()) {
if (guest_cbs->is_user_mode())
misc |= PERF_RECORD_MISC_GUEST_USER;
else
misc |= PERF_RECORD_MISC_GUEST_KERNEL;
Expand Down
7 changes: 5 additions & 2 deletions arch/riscv/kernel/perf_callchain.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ static unsigned long user_backtrace(struct perf_callchain_entry_ctx *entry,
void perf_callchain_user(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
unsigned long fp = 0;

/* RISC-V does not support perf in guest mode. */
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
if (guest_cbs && guest_cbs->is_in_guest())
return;

fp = regs->s0;
Expand All @@ -84,8 +85,10 @@ void notrace walk_stackframe(struct task_struct *task,
void perf_callchain_kernel(struct perf_callchain_entry_ctx *entry,
struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();

/* RISC-V does not support perf in guest mode. */
if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (guest_cbs && guest_cbs->is_in_guest()) {
pr_warn("RISC-V does not support perf in guest mode!");
return;
}
Expand Down
17 changes: 11 additions & 6 deletions arch/x86/events/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -2366,10 +2366,11 @@ static bool perf_hw_regs(struct pt_regs *regs)
void
perf_callchain_kernel(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct unwind_state state;
unsigned long addr;

if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (guest_cbs && guest_cbs->is_in_guest()) {
/* TODO: We don't support guest os callchain now */
return;
}
Expand Down Expand Up @@ -2475,10 +2476,11 @@ perf_callchain_user32(struct pt_regs *regs, struct perf_callchain_entry_ctx *ent
void
perf_callchain_user(struct perf_callchain_entry_ctx *entry, struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
struct stack_frame frame;
const unsigned long __user *fp;

if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (guest_cbs && guest_cbs->is_in_guest()) {
/* TODO: We don't support guest os callchain now */
return;
}
Expand Down Expand Up @@ -2562,18 +2564,21 @@ static unsigned long code_segment_base(struct pt_regs *regs)

unsigned long perf_instruction_pointer(struct pt_regs *regs)
{
if (perf_guest_cbs && perf_guest_cbs->is_in_guest())
return perf_guest_cbs->get_guest_ip();
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();

if (guest_cbs && guest_cbs->is_in_guest())
return guest_cbs->get_guest_ip();

return regs->ip + code_segment_base(regs);
}

unsigned long perf_misc_flags(struct pt_regs *regs)
{
struct perf_guest_info_callbacks *guest_cbs = perf_get_guest_cbs();
int misc = 0;

if (perf_guest_cbs && perf_guest_cbs->is_in_guest()) {
if (perf_guest_cbs->is_user_mode())
if (guest_cbs && guest_cbs->is_in_guest()) {
if (guest_cbs->is_user_mode())
misc |= PERF_RECORD_MISC_GUEST_USER;
else
misc |= PERF_RECORD_MISC_GUEST_KERNEL;
Expand Down
9 changes: 6 additions & 3 deletions arch/x86/events/intel/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -2333,6 +2333,7 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
{
struct perf_sample_data data;
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
struct perf_guest_info_callbacks *guest_cbs;
int bit;
int handled = 0;

Expand Down Expand Up @@ -2386,9 +2387,11 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
*/
if (__test_and_clear_bit(55, (unsigned long *)&status)) {
handled++;
if (unlikely(perf_guest_cbs && perf_guest_cbs->is_in_guest() &&
perf_guest_cbs->handle_intel_pt_intr))
perf_guest_cbs->handle_intel_pt_intr();

guest_cbs = perf_get_guest_cbs();
if (unlikely(guest_cbs && guest_cbs->is_in_guest() &&
guest_cbs->handle_intel_pt_intr))
guest_cbs->handle_intel_pt_intr();
else
intel_pt_interrupt();
}
Expand Down
13 changes: 12 additions & 1 deletion include/linux/perf_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,18 @@ extern void perf_event_bpf_event(struct bpf_prog *prog,
enum perf_bpf_event_type type,
u16 flags);

extern struct perf_guest_info_callbacks *perf_guest_cbs;
extern struct perf_guest_info_callbacks __rcu *perf_guest_cbs;
static inline struct perf_guest_info_callbacks *perf_get_guest_cbs(void)
{
/*
* Callbacks are RCU-protected and must be READ_ONCE to avoid reloading
* the callbacks between a !NULL check and dereferences, to ensure
* pending stores/changes to the callback pointers are visible before a
* non-NULL perf_guest_cbs is visible to readers, and to prevent a
* module from unloading callbacks while readers are active.
*/
return rcu_dereference(perf_guest_cbs);
}
extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);

Expand Down
13 changes: 10 additions & 3 deletions kernel/events/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -6045,18 +6045,25 @@ static void perf_pending_event(struct irq_work *entry)
* Later on, we might change it to a list if there is
* another virtualization implementation supporting the callbacks.
*/
struct perf_guest_info_callbacks *perf_guest_cbs;
struct perf_guest_info_callbacks __rcu *perf_guest_cbs;

int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
{
perf_guest_cbs = cbs;
if (WARN_ON_ONCE(rcu_access_pointer(perf_guest_cbs)))
return -EBUSY;

rcu_assign_pointer(perf_guest_cbs, cbs);
return 0;
}
EXPORT_SYMBOL_GPL(perf_register_guest_info_callbacks);

int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
{
perf_guest_cbs = NULL;
if (WARN_ON_ONCE(rcu_access_pointer(perf_guest_cbs) != cbs))
return -EINVAL;

rcu_assign_pointer(perf_guest_cbs, NULL);
synchronize_rcu();
return 0;
}
EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
Expand Down

0 comments on commit 9b45f20

Please sign in to comment.