linux-kernel - Re: [PATCH] perf jevents: Raise exception for no definition of a arch std event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fVsePTVU2qeeUzVFkWA7Oxj6bgSq+yWzO=t3y283vOgOw@mail.gmail.com>
Date:   Thu, 10 Aug 2023 12:27:14 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     Ilkka Koskinen <ilkka@...amperecomputing.com>
Cc:     John Garry <john.g.garry@...cle.com>, acme@...nel.org,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf jevents: Raise exception for no definition of a arch
 std event

On Wed, Aug 9, 2023 at 10:11 PM Ilkka Koskinen
<ilkka@...amperecomputing.com> wrote:
>
>
> Hi John,
>
> On Mon, 7 Aug 2023, John Garry wrote:
> > Recently Ilkka reported that the JSONs for the AmpereOne arm64-based
> > platform included a dud event which referenced a non-existent arch std
> > event [0].
>
> I wish I had found the bug in my patch a long time ago but, in fact, it
> was Dave Kleikamp who initially pointed it out to me and figured out the
> difference between jevents.c and jevents.py when porting the patch to 5.15
> kernel.
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2023-June/844874.html
>
> >
> > Previously in the times of jevents.c, we would raise an exception for this.
> >
> > This is still invalid, even though the current code just ignores such an
> > event.
> >
> > Re-introduce code to raise an exception for when no definition exists to
> > help catch as many invalid JSONs as possible.
> >
> > [0] https://lore.kernel.org/linux-perf-users/9e851e2a-26c7-ba78-cb20-be4337b2916a@oracle.com/
> >
> > Signed-off-by: John Garry <john.g.garry@...cle.com>
>
> Thanks for the patch! I quickly tested it and it worked as expected. Just
> in case this is needed:
>
> Tested-by: Ilkka Koskinen <ilkka@...amperecomputing.com>

Reviewed-by: Ian Rogers <irogers@...gle.com>

Thanks,
Ian

> Cheers, Ilkka
>
> > ---
> > Please do not apply before [0], above.
> >
> > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> > index 8cd561aa606a..98cccc3fcbbd 100755
> > --- a/tools/perf/pmu-events/jevents.py
> > +++ b/tools/perf/pmu-events/jevents.py
> > @@ -347,12 +347,15 @@ class JsonEvent:
> >       if self.desc and not self.desc.endswith('. '):
> >         self.desc += '. '
> >       self.desc = (self.desc if self.desc else '') + ('Unit: ' + self.pmu + ' ')
> > -    if arch_std and arch_std.lower() in _arch_std_events:
> > -      event = _arch_std_events[arch_std.lower()].event
> > -      # Copy from the architecture standard event to self for undefined fields.
> > -      for attr, value in _arch_std_events[arch_std.lower()].__dict__.items():
> > -        if hasattr(self, attr) and not getattr(self, attr):
> > -          setattr(self, attr, value)
> > +    if arch_std:
> > +      if arch_std.lower() in _arch_std_events:
> > +        event = _arch_std_events[arch_std.lower()].event
> > +        # Copy from the architecture standard event to self for undefined fields.
> > +        for attr, value in _arch_std_events[arch_std.lower()].__dict__.items():
> > +          if hasattr(self, attr) and not getattr(self, attr):
> > +            setattr(self, attr, value)
> > +      else:
> > +        raise argparse.ArgumentTypeError('Cannot find arch std event:', arch_std)
> >
> >     self.event = real_event(self.name, event)
> >
> > --
> > 2.35.3
> >
> >