Re: [PATCH v7 4/8] bpftool: Ensure task comm is always NUL-terminated
From: Alejandro Colomar <alx@kernel.org>
Date: 2024-08-17 08:39:03
Also in:
bpf, dri-devel, linux-fsdevel, linux-mm, linux-trace-kernel, netdev, selinux
Hi Yafang, On Sat, Aug 17, 2024 at 10:56:20AM GMT, Yafang Shao wrote:
quoted hunk ↗ jump to hunk
Let's explicitly ensure the destination string is NUL-terminated. This way, it won't be affected by changes to the source string. Signed-off-by: Yafang Shao <redacted> Reviewed-by: Quentin Monnet <qmo@kernel.org> --- tools/bpf/bpftool/pids.c | 2 ++ 1 file changed, 2 insertions(+)diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c index 9b898571b49e..23f488cf1740 100644 --- a/tools/bpf/bpftool/pids.c +++ b/tools/bpf/bpftool/pids.c@@ -54,6 +54,7 @@ static void add_ref(struct hashmap *map, struct pid_iter_entry *e) ref = &refs->refs[refs->ref_cnt]; ref->pid = e->pid; memcpy(ref->comm, e->comm, sizeof(ref->comm)); + ref->comm[sizeof(ref->comm) - 1] = '\0';
Why doesn't this use strscpy()? Isn't the source terminated? Both the source and the destination measure 16 characters. If it is true that the source is not terminated, then this copy might truncate the (non-)string by overwriting the last byte with a NUL. Is that truncation a good thing?
quoted hunk ↗ jump to hunk
refs->ref_cnt++; return;@@ -77,6 +78,7 @@ static void add_ref(struct hashmap *map, struct pid_iter_entry *e) ref = &refs->refs[0]; ref->pid = e->pid; memcpy(ref->comm, e->comm, sizeof(ref->comm)); + ref->comm[sizeof(ref->comm) - 1] = '\0';
Same question here.
refs->ref_cnt = 1; refs->has_bpf_cookie = e->has_bpf_cookie; refs->bpf_cookie = e->bpf_cookie; -- 2.43.5
-- <https://www.alejandro-colomar.es/>
Attachments
- signature.asc [application/pgp-signature] 833 bytes