Re: [PATCH v4 02/10] user_events: Add minimal support for trace_event into ftrace

[Masami Hiramatsu]

On Thu, 11 Nov 2021 09:33:34 -0800
Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx> wrote:

> On Wed, Nov 10, 2021 at 10:56:30PM +0900, Masami Hiramatsu wrote:
> > On Tue, 9 Nov 2021 11:08:44 -0800
> > Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx> wrote:
> 
> > 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@xxxxxxxxxx>
> 
> Ok, I will start there.
> 
> Are static string buffers required as well for the null check?
> 
> Or is this only for dyn strings that require the check?

Good question! The dynamic strings is ensured to be null-terminated,
but the static string is not because the size is fixed (at least
event filter checked that.)

BTW, I found that the hist_triger_elt_update() doesn't check the
field size for fixed-size string (only use STR_VAR_LEN_MAX to limit.)
It seems buggy if the fixed-size char [] field is not null terminated.
(e.g. it is used for storing array-data)
Let me fix that.

> Also, I am assuming that __rel_loc offset is based after the __rel_loc
> payload, IE: Offset 0 of __rel_loc is immediately after the 4 byte
> __rel_loc description?

Yes, so if the field is the last one, the offset can be 0.

Thank you,

-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>