linux-kernel - Re: [PATCH v8 2/4] libperf: Add evsel mmap support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YHcuIKjNDkOUCupx@kernel.org>
Date:   Wed, 14 Apr 2021 15:02:08 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Rob Herring <robh@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Jiri Olsa <jolsa@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Will Deacon <will@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Mark Rutland <mark.rutland@....com>,
        Itaru Kitayama <itaru.kitayama@...il.com>
Subject: Re: [PATCH v8 2/4] libperf: Add evsel mmap support

Em Thu, Apr 15, 2021 at 01:41:35AM +0900, Namhyung Kim escreveu:
> Hello,
> 
> On Thu, Apr 15, 2021 at 1:07 AM Rob Herring <robh@...nel.org> wrote:
> > +void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread)
> > +{
> > +       if (FD(evsel, cpu, thread) < 0 || MMAP(evsel, cpu, thread) == NULL)
> > +               return NULL;
> 
> I think you should check the cpu and the thread is in
> a valid range.  Currently xyarray__entry() simply accesses
> the content without checking the boundaries.

So, since xyarray has the bounds, it should check it, i.e. we need to
have a __xyarray__entry() that is what xyarray__entry() does, i.e.
assume the values have been bounds checked, then a new method,
xyarray__entry() that does bounds check, if it fails, return NULL,
otherwise calls __xyarray__entry().

I see this is frustrating and I should've chimed in earlier, but at
least now this is getting traction, and the end result will be better
not just for the feature you've been dilligently working on,

Thank you for your persistence,

- Arnaldo
 
> Thanks,
> Namhyung
> 
> 
> > +
> > +       return MMAP(evsel, cpu, thread)->base;
> > +}
> > +
> >  int perf_evsel__read_size(struct perf_evsel *evsel)
> >  {
> >         u64 read_format = evsel->attr.read_format;
> > diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h
> > index 1ffd083b235e..1c067d088bc6 100644
> > --- a/tools/lib/perf/include/internal/evsel.h
> > +++ b/tools/lib/perf/include/internal/evsel.h
> > @@ -41,6 +41,7 @@ struct perf_evsel {
> >         struct perf_cpu_map     *own_cpus;
> >         struct perf_thread_map  *threads;
> >         struct xyarray          *fd;
> > +       struct xyarray          *mmap;
> >         struct xyarray          *sample_id;
> >         u64                     *id;
> >         u32                      ids;
> > diff --git a/tools/lib/perf/include/perf/evsel.h b/tools/lib/perf/include/perf/evsel.h
> > index c82ec39a4ad0..60eae25076d3 100644
> > --- a/tools/lib/perf/include/perf/evsel.h
> > +++ b/tools/lib/perf/include/perf/evsel.h
> > @@ -27,6 +27,9 @@ LIBPERF_API int perf_evsel__open(struct perf_evsel *evsel, struct perf_cpu_map *
> >                                  struct perf_thread_map *threads);
> >  LIBPERF_API void perf_evsel__close(struct perf_evsel *evsel);
> >  LIBPERF_API void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> > +LIBPERF_API int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> > +LIBPERF_API void perf_evsel__munmap(struct perf_evsel *evsel);
> > +LIBPERF_API void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
> >  LIBPERF_API int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> >                                  struct perf_counts_values *count);
> >  LIBPERF_API int perf_evsel__enable(struct perf_evsel *evsel);
> > diff --git a/tools/lib/perf/libperf.map b/tools/lib/perf/libperf.map
> > index 7be1af8a546c..c0c7ceb11060 100644
> > --- a/tools/lib/perf/libperf.map
> > +++ b/tools/lib/perf/libperf.map
> > @@ -23,6 +23,9 @@ LIBPERF_0.0.1 {
> >                 perf_evsel__disable;
> >                 perf_evsel__open;
> >                 perf_evsel__close;
> > +               perf_evsel__mmap;
> > +               perf_evsel__munmap;
> > +               perf_evsel__mmap_base;
> >                 perf_evsel__read;
> >                 perf_evsel__cpus;
> >                 perf_evsel__threads;
> > --
> > 2.27.0

-- 

- Arnaldo