From: sashiko-bot@kernel.org
To: "Thaumy Cheng" <thaumy.love@gmail.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf/core: Strengthen userpage update ordering
Date: Sun, 06 Sep 2026 12:33:35 +0000 [thread overview]
Message-ID: <20260906123336.1BC681F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260906122350.24305-1-thaumy.love@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Preexisting seqlock writer re-entrancy bug in perf_event_update_userpage leading to corrupted data for userspace readers.
--
commit 019372a4e61c18f0c5a22ddb487c7acef14ffc7e
Author: Thaumy Cheng <thaumy.love@gmail.com>
perf/core: Strengthen userpage update ordering
This commit replaces compiler barriers with smp_wmb() around the userpage
payload update in perf_event_update_userpage(). It aims to strengthen the
publication ordering for cross-CPU readers of time conversion fields on
weakly ordered architectures.
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a31104..9c6dfb79fd77d 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6858,7 +6858,8 @@ void perf_event_update_userpage(struct perf_event *event)
> userpg = rb->user_page;
>
> ++userpg->lock;
> - barrier();
> + /* Publish the odd lock value before updating the payload. */
> + smp_wmb();
[Severity: High]
This is a pre-existing issue, but does perf_event_update_userpage() have a
seqlock writer re-entrancy bug here that defeats the synchronization?
Since this function disables preemption (via preempt_disable() earlier in
the function) but leaves hardware interrupts enabled when called from task
context (such as from perf_mmap_rb()), what happens if a PMU hardware
interrupt like an NMI triggers during this window?
If the NMI handler also calls perf_event_update_userpage() for the same
event, it would increment userpg->lock again, advancing it to an even value.
A concurrent userspace reader on another CPU would then see an even lock
value and could read actively-torn payload data (like time_mult, time_shift,
or time_offset), leading to bogus timestamps.
Should hardware interrupts be disabled before incrementing the seqlock to
prevent this writer re-entrancy?
> userpg->index = perf_event_index(event);
> userpg->offset = perf_event_count(event, false);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260906122350.24305-1-thaumy.love@gmail.com?part=1
prev parent reply other threads:[~2026-09-06 12:33 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-06 12:23 [PATCH] perf/core: Strengthen userpage update ordering Thaumy Cheng
2026-09-06 12:33 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260906123336.1BC681F00A3A@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=thaumy.love@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.