Re: [PATCH v3 3/7] treewide: Replace memcpy(..., current->comm) with copy_task_comm()
From: David Laight
Date: Fri Jun 12 2026 - 14:53:37 EST
On Fri, 12 Jun 2026 13:20:16 -0300
André Almeida <andrealmeid@xxxxxxxxxx> wrote:
> In order to increase the size of current->comm[] and to avoid breaking any
> existing code, replace memcpy() with copy_task_comm(). This new function
> makes sure that the copy is NUL terminated. This is crucial given that the
> source buffer might be larger than the destination buffer and could
> truncate the NUL character out of it.
Aren't you re-inventing get_task_comm() that the previous patch removed?
...
> +/*
> + * Copy task name to a buffer. Final result is always a NUL-terminated string.
> + */
> +#define copy_task_comm(dst, tsk, len) \
> +{ \
> + const char *_src = (tsk)->comm; \
> + size_t _dst_len = len + __must_be_array(dst),
If you are using __must_be_array() then why not use sizeof to get the _dst_len?
> + _src_len = sizeof(_src); \
Isn't sizeof(_src) just the size of a pointer?
You need to use sizeof (tsk)->comm
> + char *_dst = dst; \
> + \
> + if (_dst_len <= _src_len) { \
> + memcpy(_dst, _src, _dst_len); \
> + dst[_dst_len - 1] = '\0'; \
If the lengths are equal you don't need to write the '\0'.
(and they should really both be compile time constants.)
> + } else { \
> + strscpy_pad(_dst, _src, _dst_len); \
> + } \
If you do the memcpy() the bytes after the first '\0' aren't guaranteed to
be '\0' - then can be random (usually part of an old version of the task name).
So I'm not sure the strscpy_pad() path is needed.
The most you might want to do is memset() the extra bytes.
But are there ever any????
There will be code that copies the task->comm to a short buffer, but are
there any places where it actually gets copied to a longer one - if so why?
David