On Thu, May 12, 2022 at 03:45:31PM -0400, Mathieu Desnoyers wrote: > ----- On May 12, 2022, at 11:47 AM, Mathieu Desnoyers mathieu.desnoyers@xxxxxxxxxxxx wrote: > > > Hi Beau, > > > > I have queued a few questions I would like to discuss with respect to the > > proposed > > user events UAPI. I originally planned to expand further on them, but I now > > think it's > > best if I ask away right now and we clarify things through discussion: > > > > First, I find it odd that the event enable bitmask and the event ID and payload > > type registration must be combined. I can think of various use-cases where other > > tracers would be interested to use the event-enable bitmask facility without > > polluting the event ID/payload registration data structures with useless data. > > Can those be split into two distinct independent ABIs ? > > > > I can't help but notice that this new user-space instrumentation > > infrastructure/ABI > > can only be used for tracing user-space through kernel tracers. Considering that > > ABIs dictated by the kernel usually end up being de facto standards, I am > > concerned > > that if it is unable to allow purely user-space tracers to use it, then all > > applications > > will end up being statically instrumented in ways that prevent user-space > > tracers from > > hooking efficiently on the static instrumentation. As I have replied in an > > earlier > > thread, purely user-space tracers are not just marginally faster than kernel > > tracers > > for tracing user-space. They are an order of magnitude faster as soon as all the > > proper > > configuration steps are taken to ensure there are no system calls on the tracer > > fast path. Therefore, it would be sad to effectively dismiss efficient tracer > > implementations for the sake of easiness of implementation of today's user-event > > ABI. This will cause a precedent we will be stuck with later. > > > > Linux kernel developers involved in implementation of instrumentation within > > Linux > > have spent a lot of effort to make sure the instrumentation is orthogonal to the > > tracing technology (tracepoints, kprobe, kretprobe...). I understand that making > > sure the user-space instrumentation ABI keeps this orthogonal is a lot more > > work, > > but nobody said that exposing ABIs to user-space was easy. ;-) > > > > The other point I would like to raise is container awareness. I can't help but > > notice that the user events ABI is exposed to trace all containers, with the > > intent > > to be used (consumed) from some privileged namespace (e.g. root pid namespace). > > This works in use-cases where the user of the tracing data controls the entire > > machine (all containers), but not so much if the user is a single tenant within > > a multi-tenants systems. I would expect that a proper namespace-aware facility > > would take care of making sure that a trace consumer could observe what is > > instrumented within its own container, and within nested containers as well. > > The fact that all container questions are entirely dismissed, thus keeping a > > event-enable bitmask registry and event ID/type registry global to the entire > > system, is not compatible with consuming traces from a non-privileged container, > > and I suspect this may also be used as a side-channel to learn information about > > what other containers are doing in a multi-tenant system. > > One more thought: I may have simply missed it, but is there any user events code > which dynamically validates that the input from user-space writev() indeed match the > event description layout ? I'm thinking about wrong size, too short strings, too long > strings, missing null terminator and so on. Any user input that could make the trace > unreadable should never reach the tracing buffers. > Yes, there are validators attached to events to ensure that the minimum size is written and ensure types that require certain safety gaurantees are met. Currently I believe the only such requirement is for variable length strings to have proper null termination (beside the min size requirement). See user_event_validate() in kernel/trace/trace_events_user.c. This is called for both perf and ftrace buffers, buffers discard if the validation fails. Related patch: https://lore.kernel.org/all/20220118204326.2169-7-beaub@xxxxxxxxxxxxxxxxxxx/ > Thanks, > > Mathieu > > > > > Thanks, > > > > Mathieu > > > > -- > > Mathieu Desnoyers > > EfficiOS Inc. > > http://www.efficios.com > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com Thanks, -Beau