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