Re: [PATCH v6 2/3] treewide: Switch memcpy() users of 'task->comm' to a more safer implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 24, 2025 at 06:06:11PM +0530, Bhupesh wrote:
> As Linus mentioned in [1], currently we have several memcpy() use-cases
> which use 'current->comm' to copy the task name over to local copies.
> For an example:
> 
>  ...
>  char comm[TASK_COMM_LEN];
>  memcpy(comm, current->comm, TASK_COMM_LEN);
>  ...
> 
> These should be modified so that we can later implement approaches
> to handle the task->comm's 16-byte length limitation (TASK_COMM_LEN)
> in a more modular way (follow-up patch does the same):
> 
>  ...
>  char comm[TASK_COMM_LEN];
>  memcpy(comm, current->comm, TASK_COMM_LEN);
>  comm[TASK_COMM_LEN - 1] = '\0';
>  ...

Why not switch all of these to get_task_comm()? It will correctly handle
the size check and NUL termination.

In an earlier thread[1], I pointed out that since __set_task_comm() always
keeps a final NUL byte, we're always safe to use strscpy(). (We want to
block over-reads and over-writes but don't care about garbled reads.)

In the new case of copying into a smaller buffer, strscpy() will always
handle writing the final NUL byte.

The only special cases I can think of would be non-fixed-sized
destination buffers, which get_task_comm() doesn't like since it can't
validate how to safely terminate the buffer. The example you give above
isn't that, though.

-Kees

[1] https://lore.kernel.org/all/202411301244.381F2B8D17@keescook/

-- 
Kees Cook




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux