linux-kernel - Re: [PATCH v1 00/10] Move uid filtering to BPF filters

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: <Z60nBQCetMzhRg5b@google.com>
Date: Wed, 12 Feb 2025 14:56:05 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>, Hao Ge <gehao@...inos.cn>,
	James Clark <james.clark@...aro.org>,
	Howard Chu <howardchu95@...il.com>,
	Dominique Martinet <asmadeus@...ewreck.org>,
	Levi Yun <yeoreum.yun@....com>, Xu Yang <xu.yang_2@....com>,
	Tengda Wu <wutengda@...weicloud.com>,
	Yang Jihong <yangjihong1@...wei.com>,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 00/10] Move uid filtering to BPF filters

On Wed, Feb 12, 2025 at 12:00:42PM -0800, Ian Rogers wrote:
> On Wed, Feb 12, 2025 at 10:46 AM Namhyung Kim <namhyung@...nel.org> wrote:
> > It's not completely broken and works sometimes.
> 
> No this is the definition of completely broken. If it only works
> sometimes then you can't use it, we can't put a test on it, there is
> no point in it. Even when it doesn't fail on perf_event_open, does it
> work for processes that start after /proc is scanned? No, it is
> completely broken.

Ok, we have a different definition for it.  Let's ignore the imaginary
users of the broken features.  Have you added a test for this change?

Anyway I've tested your change and found some issues:

1. It silently exited when BPF-skel is not built.  Better to put some
   messages at least.

  $ sudo ./perf record -u $(id -u) -- sleep 1

2. Even with BPF-skel, perf record doesn't work well.  It did something
   but failed to get sample data for some reason.

  $ sudo ./perf record -u $(id -u) -- sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.045 MB perf.data ]
   
   Oh, I think you now need to pass -a because it now works in
   system-wide mode and drops samples for other users.

3. With BPF-skel, non-root users will see this.

  $ ./perf record -u $(id -u) -- sleep 1
  cannot get fd for 'filters' map
  failed to set filter "BPF" on event cycles:P with 13 (Permission denied)

   I think it's confusing and better to tell user that you need to run 
   'perf record --setup-filter pin' as root first.  But maybe due to the
   issue #2, you still need to run it as root.

Thanks,
Namhyung