Re: [PATCH 0/4] tracing/user_events: Introduce multi-format events

[Google]

Hi Beau,

On Tue, 23 Jan 2024 22:08:40 +0000
Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx> 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.

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?

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@xxxxxxxxxx>