Messages in this thread | | | Date | Fri, 12 Nov 2021 22:40:32 +0900 | From | Masami Hiramatsu <> | Subject | Re: [PATCH v4 02/10] user_events: Add minimal support for trace_event into ftrace |
| |
On Thu, 11 Nov 2021 09:33:34 -0800 Beau Belgrave <beaub@linux.microsoft.com> 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@linux.microsoft.com> 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@kernel.org> > > 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@kernel.org>
| |