Messages in this thread | | | From | Arnaldo Carvalho de Melo <> | Date | Tue, 20 Aug 2019 14:13:42 -0300 | Subject | Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks |
| |
Em Mon, Aug 19, 2019 at 10:22:07PM +0000, Lubashev, Igor escreveu: > On Mon, August 19, 2019 at 12:51 PM Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > > On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote: > > Things are working properly on your perf/cap branch. I tested with on both > > x86 and ARM. > Mathieu, you are probably testing with euid==0. If you were to test > with euid!=0 but with CAP_SYSLOG and no libcap (and kptr_restrict=0, > perf_event_paranoid=2), you would likely hit the bug that you > identified in __perf_event__synthesize_kermel_mmap(). > See https://lkml.kernel.org/lkml/930a59730c0d495f8c5acf4f99048e8d@usma1ex-dag1mb6.msg.corp.akamai.com for the fix (Option 1 only or Options 1+2). > > Arnaldo, once we decide what the right fix is, I am happy to post the update (options 1, 1+2) as a patch series.
I think you should get the checks for ref_reloc_sym in place so as to make the code overall more robust, and also go on continuing to make the checks in tools/perf/ to match what is checked on the other side of the mirror, i.e. by the kernel, so from a quick read, please put first the robustness patches (check ref_reloc_sym) do your other suggestions and update the warnings, then refresh the two patches that still are not in my perf/core branch:
[acme@quaco perf]$ git rebase perf/core First, rewinding head to replay your work on top of it... Applying: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks Applying: perf symbols: Use CAP_SYSLOG with kptr_restrict checks [acme@quaco perf]$
I've pushed out perf/cap, so you can go from there as it is rebased on my current perf/core.
Then test all these cases: with/without libcap, with euid==0 and different than zero, with capabilities, etc, patch by patch so that we don't break bisection nor regress,
Thanks and keep up the good work!
- Arnaldo > - Igor > > > > > > I am not sure how this can be fixed. I counted a total of 19 > > > > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich > > > > care to check if kmap->ref_reloc_sym is valid before proceeding. As > > > > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is > > > > guaranteed to be valid. If I am correct then all we need is to > > > > check for a valid pointer in _perf_event__synthesize_kernel_mmap(). > > > > Otherwise it will be a little harder. > > > > > > > > Mathieu >
--
- Arnaldo
| |