Re: [PATCH] tracing/user_events: Run BPF program if attached

On Wed, May 17, 2023 at 7:08 PM Steven Rostedt [off-list ref] wrote:

On Wed, 17 May 2023 18:18:14 -0700
Beau Belgrave [off-list ref] wrote:

quoted

We should tell folks to use a group and give access to the group like
Steven said earlier.

Could we possibly add an "owner" attribute to the event. That is, the
creator of the event is the only one that can write to that event. Or at
least by the user that created it (not actually the process).

So if the user "rostedt" creates an event, only "rostedt" can write to or
delete the event.

That can work, but why name is global in the first place?
Just make everything per-fd.

The delete IOCTL is different than reg/unreg. I don't see a problem with
adding a CAP_SYSADMIN check on the delete IOCTL (and other delete paths)
to prevent this. It shouldn't affect anything we are doing to add this
and it makes it so non-admins cannot delete any events if they are given
write access to the user_events_data file.

sysadmin for delete is a pointless.
user_events_ioctl_reg() has the same issue.
Two different processes using you fancy TRACELOGGING_DEFINE_PROVIDER()
macro and picking the same name will race.

TRACELOGGING_DEFINE_PROVIDER( // defines the MyProvider symbol
    MyProvider, // Name of the provider symbol to define
    "MyCompany_MyComponent_MyProvider", // Human-readable provider
name, no ' ' or ':' chars.
    // {d5b90669-1aad-5db8-16c9-6286a7fcfe33} // Provider guid
(ignored on Linux)
    (0xd5b90669,0x1aad,0x5db8,0x16,0xc9,0x62,0x86,0xa7,0xfc,0xfe,0x33));

I totally get it that Beau is copy pasting these ideas from windows,
but windows is likely similarly broken if it's registering names
globally.

FD should be the isolation boundary.
fd = open("/sys/kernel/tracing/user_event")
and make sure all events are bound to that file.
when file is closed the events _should be auto deleted_.

That's another issue I just spotted.
Looks like user_events_release() is leaking memory.
user_event_refs are just lost.

tbh the more I look into the code the more I want to suggest to mark it
depends on BROKEN
and go back to redesign.