Re: [PATCH bpf-next 2/3] tracing: Expose tracepoint BTF ids via tracefs

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 5/15/26 7:25 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The global `event_mutex` is held across a `copy_to_user()` operation, creating a denial-of-service vulnerability and latency spikes.
> --
> 
> commit 5bcf593e0ef00b79b0e87af32e5fdb4e5481d768
> Author: Mykyta Yatsenko <yatsenko@meta.com>
> 
> tracing: Expose tracepoint BTF ids via tracefs
> 
> This commit introduces a new tracefs file btf_ids for each event. It exposes
> the BTF object ID and tracepoint/raw BTF IDs populated by resolve_btfids at
> link time to allow raw_tp and tp_btf BPF programs to properly attach.
> 
>> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
>> index c46e623e7e0de..d463fb44c790a 100644
>> --- a/kernel/trace/trace_events.c
>> +++ b/kernel/trace/trace_events.c
> 
> [ ... ]
> 
>> @@ -2200,6 +2201,62 @@ event_id_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
>>  }
>>  #endif
>>  
>> +#ifdef CONFIG_BPF_EVENTS
>> +static ssize_t
>> +event_btf_ids_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos)
>> +{
>> +	struct trace_event_file *file;
>> +	struct trace_event_call *call;
>> +	const struct btf_type *t;
>> +	struct module *mod = NULL;
>> +	u32 raw_id, tp_id;
>> +	const u32 *ids;
>> +	struct btf *btf;
>> +	char buf[128];
>> +	int len;
>> +
>> +	/* Module unload could free call->class and ids[] mid-read. */
>> +	guard(mutex)(&event_mutex);
>> +
>> +	file = event_file_file(filp);
>> +	if (!file)
>> +		return -ENODEV;
> 
> [ ... ]
> 
>> +	len = scnprintf(buf, sizeof(buf),
>> +			"btf_obj_id: %u\nraw_btf_id: %u\ntp_btf_id: %u\n",
>> +			btf_obj_id(btf), raw_id, tp_id);
>> +	btf_put(btf);
>> +
>> +	return simple_read_from_buffer(ubuf, cnt, ppos, buf, len);
> 
> Does this hold the global event_mutex during copy_to_user()?
> 

Right, I don't need to hold mutext for that long, should drop it before
scnprintf() call

> Because guard(mutex)(&event_mutex) holds the lock until the function returns,
> the mutex remains locked during simple_read_from_buffer(), which internally
> calls copy_to_user().
> 
> Could this allow a user to intentionally stall the global tracing subsystem?
> 
> If the user-space buffer is backed by userfaultfd or a slow FUSE mount, the
> page fault could block indefinitely while holding event_mutex, which protects
> system-wide tracing operations (including module loading and unloading).
> 
> Looking at event_filter_read() nearby, it explicitly releases event_mutex
> before calling simple_read_from_buffer(). Would it be safer to use a similar
> pattern here by dropping the lock once the buffer string is populated?
> 
>> +}
>> +#endif
>