Messages in this thread Patch in this message | | | From | Mark Rutland <> | Subject | [PATCH] perf/ring_buffer: ensure atomicity and order of updates | Date | Thu, 10 May 2018 14:06:32 +0100 |
| |
Userspace can read/write the user page at any point in time, and in perf_output_put_handle() we're very careful to use memory barriers to ensure ordering between updates to data and the user page.
We don't use barriers when updating aux_head, where similar ordering constraints apply. This could result in userspace seeing stale data, or data being overwritten while userspace was still consuming it.
Further, we update data_head and aux_head with plain assignments, which the compiler can tear, potentially resulting in userspace seeing erroneous values.
We can solve both of these problems by using smp_store_release to update data_head and aux_head, so let's do so.
Signed-off-by: Mark Rutland <mark.rutland@arm.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Will Deacon <will.deacon@arm.com> --- kernel/events/ring_buffer.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c index 6c6b3c48db71..839b207e4c77 100644 --- a/kernel/events/ring_buffer.c +++ b/kernel/events/ring_buffer.c @@ -63,10 +63,10 @@ static void perf_output_put_handle(struct perf_output_handle *handle) * kernel user * * if (LOAD ->data_tail) { LOAD ->data_head - * (A) smp_rmb() (C) + * (A) smp_rmb() (C) * STORE $data LOAD $data - * smp_wmb() (B) smp_mb() (D) - * STORE ->data_head STORE ->data_tail + * smp_mb() (D) + * RELEASE ->data_head (B) STORE ->data_tail * } * * Where A pairs with D, and B pairs with C. @@ -83,8 +83,7 @@ static void perf_output_put_handle(struct perf_output_handle *handle) * * See perf_output_begin(). */ - smp_wmb(); /* B, matches C */ - rb->user_page->data_head = head; + smp_store_release(&rb->user_page->data_head, head); /* B, matches C */ /* * Now check if we missed an update -- rely on previous implied @@ -464,7 +463,7 @@ void perf_aux_output_end(struct perf_output_handle *handle, unsigned long size) handle->aux_flags); } - rb->user_page->aux_head = rb->aux_head; + smp_store_release(&rb->user_page->aux_head, rb->aux_head); if (rb_need_aux_wakeup(rb)) wakeup = true; @@ -496,7 +495,7 @@ int perf_aux_output_skip(struct perf_output_handle *handle, unsigned long size) rb->aux_head += size; - rb->user_page->aux_head = rb->aux_head; + smp_store_release(&rb->user_page->aux_head, rb->aux_head); if (rb_need_aux_wakeup(rb)) { perf_output_wakeup(handle); handle->wakeup = rb->aux_wakeup + rb->aux_watermark; -- 2.11.0
| |