Re: [PATCH v3 3/7] treewide: Replace memcpy(..., current->comm) with copy_task_comm()

David Laight <[email protected]> Fri, 12 Jun 2026 19:53:30 +0100
Newsgroups org.kernel.vger.linux-api,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <20260612195330.1653d041@pumpkin>
On Fri, 12 Jun 2026 13:20:16 -0300
AndrĂ© Almeida <[email protected]> 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