Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The treewide replacement of memcpy() with copy_task_comm() missed a 
tracepoint in the BPF test module.
--

commit 0b4391937405bf7ef33f5ddc8cc5b7e81c93d587
Author: André Almeida <[email protected]>

treewide: Replace memcpy(..., current->comm) with copy_task_comm()

This patch replaces treewide instances of memcpy() copying task->comm with a
new copy_task_comm() function. This ensures the destination buffer is always
NUL-terminated, preparing for future extensions of the task command name
length.

[ ... ]

> diff --git a/include/trace/events/task.h b/include/trace/events/task.h
> index b9a129eb54d9e..e3faaeb997ef2 100644
> --- a/include/trace/events/task.h
> +++ b/include/trace/events/task.h
> @@ -21,7 +21,7 @@ TRACE_EVENT(task_newtask,
>  
>       TP_fast_assign(
>               __entry->pid = task->pid;
> -             memcpy(__entry->comm, task->comm, TASK_COMM_LEN);
> +             copy_task_comm(__entry->comm, task, TASK_COMM_LEN);
>               __entry->clone_flags = clone_flags;
>               __entry->oom_score_adj = task->signal->oom_score_adj;
>       ),

[Severity: Medium]
Did this treewide replacement miss a tracepoint in the BPF test module?

Looking at TRACE_EVENT(bpf_testmod_test_read) in
tools/testing/selftests/bpf/test_kmods/bpf_testmod-events.h, it still uses
memcpy() instead of copy_task_comm():

memcpy(__entry->comm, task->comm, TASK_COMM_LEN);

If a task sets its name to 16 or more characters (which becomes possible
with the extension to 64 bytes in later patches) and the
bpf_testmod_test_read tracepoint is hit, the 16-byte __entry->comm buffer
will not be NUL-terminated.

Can this cause an out-of-bounds read when tracing consumers or TP_printk()
read the non-NUL-terminated comm buffer?

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to