Re: [PATCH v2 01/13] perf: Ensure perf_guest_cbs aren't reloaded between !NULL check and deref
From: Peter Zijlstra <peterz@infradead.org>
Date: 2021-08-28 19:45:31
Also in:
kvm, kvmarm, linux-perf-users, linux-riscv, lkml, xen-devel
On Fri, Aug 27, 2021 at 05:35:46PM -0700, Sean Christopherson wrote:
quoted hunk ↗ jump to hunk
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 2d510ad750ed..6b0405e578c1 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h@@ -1237,6 +1237,14 @@ extern void perf_event_bpf_event(struct bpf_prog *prog, u16 flags); extern struct perf_guest_info_callbacks *perf_guest_cbs; +static inline struct perf_guest_info_callbacks *perf_get_guest_cbs(void) +{ + /* Reg/unreg perf_guest_cbs waits for readers via synchronize_rcu(). */ + lockdep_assert_preemption_disabled(); + + /* Prevent reloading between a !NULL check and dereferences. */ + return READ_ONCE(perf_guest_cbs); +}
Nice..
quoted hunk ↗ jump to hunk
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);diff --git a/kernel/events/core.c b/kernel/events/core.c index 464917096e73..2126f6327321 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c@@ -6491,14 +6491,19 @@ struct perf_guest_info_callbacks *perf_guest_cbs; int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) { - perf_guest_cbs = cbs; + if (WARN_ON_ONCE(perf_guest_cbs)) + return -EBUSY; + + WRITE_ONCE(perf_guest_cbs, cbs); + synchronize_rcu();
You're waiting for all NULL users to go away? :-) IOW, we can do without this synchronize_rcu() call.
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(perf_guest_cbs != cbs)) return -EBUSY; ?
+ WRITE_ONCE(perf_guest_cbs, NULL); + synchronize_rcu(); return 0; } EXPORT_SYMBOL_GPL(perf_unregister_guest_info_callbacks);
Yes, this ought to work fine. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel