Re: [PATCH v4 2/3] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation

Yafang Shao
2025-05-22T06:28:10Z

On Thu, May 22, 2025 at 2:15 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> On Wed, May 21, 2025 at 2:24 PM Bhupesh <bhupesh@igalia.com> wrote:
> >
> > As Linus mentioned in [1], currently we have several memcpy() use-cases
> > which use 'current->comm' to copy the task name over to local copies.
> > For an example:
> >
> > ...
> > char comm[TASK_COMM_LEN];
> > memcpy(comm, current->comm, TASK_COMM_LEN);
> > ...
> >
> > These should be modified so that we can later implement approaches
> > to handle the task->comm's 16-byte length limitation (TASK_COMM_LEN)
> > is a more modular way (follow-up patches do the same):
> >
> > ...
> > char comm[TASK_COMM_LEN];
> > memcpy(comm, current->comm, TASK_COMM_LEN);
> > comm[TASK_COMM_LEN - 1] = '\0';
> > ...
> >
> > The relevant 'memcpy()' users were identified using the following search
> > pattern:
> > $ git grep 'memcpy.*->comm\>'
>
> Hello Bhupesh,
>
> Several BPF programs currently read task->comm directly, as seen in:
>
> // tools/testing/selftests/bpf/progs/test_skb_helpers.c [0]
> bpf_probe_read_kernel_str(&comm, sizeof(comm), &task->comm);
>
> This approach may cause issues after the follow-up patch.
> I believe we should replace it with the safer bpf_get_current_comm()
> or explicitly null-terminate it with "comm[sizeof(comm) - 1] = '\0'".
> Out-of-tree BPF programs like BCC[1] or bpftrace[2] relying on direct
> task->comm access may also break and require updates.
>
> [0]. https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/tools/testing/selftests/bpf/progs/test_skb_helpers.c#n26
> [1]. https://github.com/iovisor/bcc
> [2]. https://github.com/bpftrace/bpftrace

Hmm, upon checking, I confirmed that bpf_probe_read_kernel_str()
already ensures the destination string is null-terminated. Therefore,
this change is unnecessary. Please disregard my previous comment.

--
Regards
Yafang