On Sat, Aug 17, 2024 at 4:39 PM Alejandro Colomar <alx@xxxxxxxxxx> wrote: > > Hi Yafang, > > On Sat, Aug 17, 2024 at 10:56:20AM GMT, Yafang Shao wrote: > > 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 <laoar.shao@xxxxxxxxx> > > Reviewed-by: Quentin Monnet <qmo@xxxxxxxxxx> > > --- > > 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()? bpftool is a userspace tool, so strscpy() is only applicable in kernel code, correct? > Isn't the source terminated? It is currently terminated, but I believe we should avoid relying on the source. Making it independent of the source would reduce potential errors. > > 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? It's not ideal, but we should still convert it to a string, even if it ends up being truncated. -- Regards Yafang
