| Message ID | 20231215162217.3273876-1-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [topic/core-for-CI] perf: Fix perf_event_validate_size() lockdep splat | expand |
On Fri, Dec 15, 2023 at 08:22:17AM -0800, Lucas De Marchi wrote: > From: Mark Rutland <mark.rutland@arm.com> > > When lockdep is enabled, the for_each_sibling_event(sibling, event) > macro checks that event->ctx->mutex is held. When creating a new group > leader event, we call perf_event_validate_size() on a partially > initialized event where event->ctx is NULL, and so when > for_each_sibling_event() attempts to check event->ctx->mutex, we get a > splat, as reported by Lucas De Marchi: > > WARNING: CPU: 8 PID: 1471 at kernel/events/core.c:1950 __do_sys_perf_event_open+0xf37/0x1080 > > This only happens for a new event which is its own group_leader, and in > this case there cannot be any sibling events. Thus it's safe to skip the > check for siblings, which avoids having to make invasive and ugly > changes to for_each_sibling_event(). > > Avoid the splat by bailing out early when the new event is its own > group_leader. > > Fixes: 382c27f4ed28f803 ("perf: Fix perf_event_validate_size()") > Closes: https://lore.kernel.org/lkml/20231214000620.3081018-1-lucas.demarchi@intel.com/ > Closes: https://lore.kernel.org/lkml/ZXpm6gQ%2Fd59jGsuW@xpf.sh.intel.com/ > Reported-by: Lucas De Marchi <lucas.demarchi@intel.com> > Reported-by: Pengfei Xu <pengfei.xu@intel.com> > Signed-off-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Link: https://lkml.kernel.org/r/20231215112450.3972309-1-mark.rutland@arm.com > [ cherry pick from tip/urgent heading to 6.7-rc6 ] > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > kernel/events/core.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index acfc5a569818..a64165af45c1 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -1947,6 +1947,16 @@ static bool perf_event_validate_size(struct perf_event *event) > group_leader->nr_siblings + 1) > 16*1024) > return false; > > + /* > + * When creating a new group leader, group_leader->ctx is initialized > + * after the size has been validated, but we cannot safely use > + * for_each_sibling_event() until group_leader->ctx is set. A new group > + * leader cannot have any siblings yet, so we can safely skip checking > + * the non-existent siblings. > + */ > + if (event == group_leader) > + return true; > + > for_each_sibling_event(sibling, group_leader) { > if (__perf_event_read_size(sibling->attr.read_format, > group_leader->nr_siblings + 1) > 16*1024) > -- > 2.40.1 >
diff --git a/kernel/events/core.c b/kernel/events/core.c index acfc5a569818..a64165af45c1 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1947,6 +1947,16 @@ static bool perf_event_validate_size(struct perf_event *event) group_leader->nr_siblings + 1) > 16*1024) return false; + /* + * When creating a new group leader, group_leader->ctx is initialized + * after the size has been validated, but we cannot safely use + * for_each_sibling_event() until group_leader->ctx is set. A new group + * leader cannot have any siblings yet, so we can safely skip checking + * the non-existent siblings. + */ + if (event == group_leader) + return true; + for_each_sibling_event(sibling, group_leader) { if (__perf_event_read_size(sibling->attr.read_format, group_leader->nr_siblings + 1) > 16*1024)