Message-Id: <20240202145022.56eba9be71df744331e8c58d@kernel.org>
Date: Fri, 2 Feb 2024 14:50:22 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Beau Belgrave <beaub@...ux.microsoft.com>
Cc: rostedt@...dmis.org, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org, mathieu.desnoyers@...icios.com
Subject: Re: [PATCH 0/4] tracing/user_events: Introduce multi-format events

On Tue, 30 Jan 2024 10:25:49 -0800
Beau Belgrave <beaub@...ux.microsoft.com> wrote:

> On Tue, Jan 30, 2024 at 11:09:33AM +0900, Masami Hiramatsu wrote:
> > Hi Beau,
> > 
> > On Tue, 23 Jan 2024 22:08:40 +0000
> > Beau Belgrave <beaub@...ux.microsoft.com> wrote:
> > 
> > > Currently user_events supports 1 event with the same name and must have
> > > the exact same format when referenced by multiple programs. This opens
> > > an opportunity for malicous or poorly thought through programs to
> > > create events that others use with different formats. Another scenario
> > > is user programs wishing to use the same event name but add more fields
> > > later when the software updates. Various versions of a program may be
> > > running side-by-side, which is prevented by the current single format
> > > requirement.
> > > 
> > > Add a new register flag (USER_EVENT_REG_MULTI_FORMAT) which indicates
> > > the user program wishes to use the same user_event name, but may have
> > > several different formats of the event in the future. When this flag is
> > > used, create the underlying tracepoint backing the user_event with a
> > > unique name per-version of the format. It's important that existing ABI
> > > users do not get this logic automatically, even if one of the multi
> > > format events matches the format. This ensures existing programs that
> > > create events and assume the tracepoint name will match exactly continue
> > > to work as expected. Add logic to only check multi-format events with
> > > other multi-format events and single-format events to only check
> > > single-format events during find.
> > 
> > Thanks for this work! This will allow many instance to use the same
> > user-events at the same time.
> > 
> > BTW, can we force this flag set by default? My concern is if any user
> > program use this user-event interface in the container (maybe it is
> > possible if we bind-mount it). In this case, the user program can
> > detect the other program is using the event if this flag is not set.
> > Moreover, if there is a malicious program running in the container,
> > it can prevent using the event name from other programs even if it
> > is isolated by the name-space.
> > 
> 
> The multi-format use a different system name (user_events_multi). So you
> cannot use the single-format flag to detect multi-format names, etc. You
> can only use it to find other single-format names like you could always do.
> 
> Likewise, you cannot use the single-event flag to block or prevent
> multi-format events from being created.

Hmm, got it.

> 
> Changing this behavior to default would break all of our environments.
> So I'm pretty sure it would break others. The current environment
> expects tracepoints to show up as their registered name under the
> user_events system name. If this changed out from under us on a specific
> kernel version, that would be bad.
> 
> I'd like eventually to have a tracer namespace concept for containers.
> Then we would have a user_event_group per tracer namespace. Then all
> user_events within the container have a unique system name which fully
> isolates them. However, even with that isolation, we still need a way to
> allow programs in the same container to have different versions of the
> same event name.

Agreed.

> 
> Multi-format events fixes this problem. I think isolation should be
> dealt with via true namespace isolation at the tracing level.
> 
> > Steve suggested that if a user program which is running in a namespace
> > uses user-event without this flag, we can reject that by default.
> > 
> > What would you think about?
> > 
> 
> This would break all of our environments. It would make previously
> compiled programs using the existing ABI from working as expected.
> 
> I'd much prefer that level of isolation to happen at the namespace level
> and why user_events as plumbing for different groups to achieve this.
> It's also why the ABI does not allow programs to state a system name.
> I'm trying to reserve the system name for the group/tracer/namespace
> isolation work.

OK, that's reasonable enough.

Thank you!

> 
> Thanks,
> -Beau
> 
> > Thank you,
> > 
> > 
> > > 
> > > Add a register_name (reg_name) to the user_event struct which allows for
> > > split naming of events. We now have the name that was used to register
> > > within user_events as well as the unique name for the tracepoint. Upon
> > > registering events ensure matches based on first the reg_name, followed
> > > by the fields and format of the event. This allows for multiple events
> > > with the same registered name to have different formats. The underlying
> > > tracepoint will have a unique name in the format of {reg_name}:[unique_id].
> > > The unique_id is the time, in nanoseconds, of the event creation converted
> > > to hex. Since this is done under the register mutex, it is extremely
> > > unlikely for these IDs to ever match. It's also very unlikely a malicious
> > > program could consistently guess what the name would be and attempt to
> > > squat on it via the single format ABI.
> > > 
> > > For example, if both "test u32 value" and "test u64 value" are used with
> > > the USER_EVENT_REG_MULTI_FORMAT the system would have 2 unique
> > > tracepoints. The dynamic_events file would then show the following:
> > >   u:test u64 count
> > >   u:test u32 count
> > > 
> > > The actual tracepoint names look like this:
> > >   test:[d5874fdac44]
> > >   test:[d5914662cd4]
> > > 
> > > Deleting events via "!u:test u64 count" would only delete the first
> > > tracepoint that matched that format. When the delete ABI is used all
> > > events with the same name will be attempted to be deleted. If
> > > per-version deletion is required, user programs should either not use
> > > persistent events or delete them via dynamic_events.
> > > 
> > > Beau Belgrave (4):
> > >   tracing/user_events: Prepare find/delete for same name events
> > >   tracing/user_events: Introduce multi-format events
> > >   selftests/user_events: Test multi-format events
> > >   tracing/user_events: Document multi-format flag
> > > 
> > >  Documentation/trace/user_events.rst           |  23 +-
> > >  include/uapi/linux/user_events.h              |   6 +-
> > >  kernel/trace/trace_events_user.c              | 224 +++++++++++++-----
> > >  .../testing/selftests/user_events/abi_test.c  | 134 +++++++++++
> > >  4 files changed, 325 insertions(+), 62 deletions(-)
> > > 
> > > 
> > > base-commit: 610a9b8f49fbcf1100716370d3b5f6f884a2835a
> > > -- 
> > > 2.34.1
> > > 
> > 
> > 
> > -- 
> > Masami Hiramatsu (Google) <mhiramat@...nel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>

Powered by blists - more mailing lists