linux-kernel - Re: [PATCH 1/8] perf: Allow to block process in syscall tracepoints

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: <20181207085839.GC2237@hirez.programming.kicks-ass.net>
Date:   Fri, 7 Dec 2018 09:58:39 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Luis Claudio R. Goncalves" <lclaudio@...g.org>, ldv@...linux.org,
        esyr@...hat.com, Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH 1/8] perf: Allow to block process in syscall tracepoints

On Thu, Dec 06, 2018 at 01:19:46PM -0500, Steven Rostedt wrote:
> On Thu, 6 Dec 2018 09:34:00 +0100
> Peter Zijlstra <peterz@...radead.org> wrote:
> 
> > > 
> > > I don't understand this.. why are we using schedule_timeout() and all
> > > that?  
> > 
> > Urgh.. in fact, the more I look at this the more I hate it.
> > 
> > We want to block in __perf_output_begin(), but we cannot because both
> > tracepoints and perf will have preemptability disabled down there.
> > 
> > So what we do is fail the event, fake the lost count and go all the way
> > up that callstack, detect the failure and then poll-wait and retry.
> > 
> > And only do this for a few special events...  *yuck*
> 
> Since this is a special case, we should add a new option to the perf
> system call that, 1 states that it wants the traced process to block
> (and must have PTRACE permission to do so) and 2, after it reads from
> the buffer, it needs to check a bit that says "this process is blocked,
> please wake it up" and then do another perf call to kick the process to
> continue.
> 
> I really dislike the polling too. But because this is not a default
> case, and is a new feature, we can add more infrastructure to make it
> work properly, instead of trying to hack the current method into
> something that does something poorly.

So why are we doing this? What makes the syscall tracepoints so much
more special than many of the others that we need to overhaul our
fundamental design principles for them?

These patches give no justification *what*so*ever* for why we're doing
ugly arse things like this. And why does this, whatever this is, need to
be done in perf?

IOW, what problem are we solving ?