linux-kernel - Re: [PATCH v4 02/10] user_events: Add minimal support for trace

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: <20211110225630.babcd70ec85f97e369b0e446@kernel.org>
Date:   Wed, 10 Nov 2021 22:56:30 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Beau Belgrave <beaub@...ux.microsoft.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        linux-trace-devel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 02/10] user_events: Add minimal support for
 trace_event into ftrace

On Tue, 9 Nov 2021 11:08:44 -0800
Beau Belgrave <beaub@...ux.microsoft.com> wrote:

> > > I'd like to know if I do fix them that the features like filtering will still
> > > be available to user_events or if it's better to just add flags to disable
> > > kernel filtering?
> > 
> > I would rather like that the filters will be available on the user_events.
> > 
> > My question is that you need to log the dynamic data or strings via user-
> > events or not. Since the other user-events, like SDT doesn't support the
> > string variables to trace, I guess that is not a high priority.
> > 
> > Moreover, since now we can use eprobes, if user event records the address of
> > user-string, the eprobes can fetch it.
> > 
> > So, my suggestion is implmenting with fixed-size parameters as the first step
> > and keep filter/histograms/eprobes available on the user-events.
> > If you find any performance issue, you can expand the user-events to support
> > dynamic (array) data and strings.
> > 
> 
> We need strings to be able to be emitted and recorded in eBPF, perf and
> ftrace. So I would rather go after a solution that lets us keep these in
> the ring buffers, even if it means a perf hit.

OK, my concern is based on the current implementation, so in that case
you can add some additional verification. That is good.

> Guess what's left is to where the best place to check is, checking in
> the filter with unsafe flags would let us keep most of the perf (we just
> check the undersize case, 1 branch). When these unsafe types are
> filtered then a perf tax is imposed to keep things safe.

I would like to keep verifying in writer side then we can ensure the
data on ring buffer (of perf and of ftrace) is sane. If you add the unsafe
flag, you have to change all the code which access the ring buffer, not only
the filter but also eprobes, histograms, perf-tools, and other user-space
tracing tools which reads the tracing buffer directly.

> It sounded like Steven wanted to think about this a bit, so I'll wait a
> bit before poking again for consensus :)
> 
> Do you have any strong feelings about where it goes?

I recommend you to start verifying the writer side, it should make the
change as small as possible. Unsafe flag idea may involve many other
tools. And it is not fundamentary required for user-events.

Thank you,

-- 
Masami Hiramatsu <mhiramat@...nel.org>