Re: [PATCH v5 1/6] treewide: Get rid of get_task_comm()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit removes the `get_task_comm()` macro definition from `include/linux/sched.h`, but fails to update its usages in the Smack security module. This will result in an implicit function declaration or undeclared identifier error during compilation, breaking the build when `CONFIG_SECURITY_SMACK` is enabled.
--

commit 2f9c1f9e715d0c7c819d963ab3e8781e9ac3ae49
Author: André Almeida <andrealmeid@igalia.com>

treewide: Get rid of get_task_comm()

This commit replaces usages of get_task_comm() with strscpy_pad() across
the tree to correctly handle buffer size mismatches. It also removes the
definition of the get_task_comm() macro entirely from include/linux/sched.h.

Link: https://lore.kernel.org/lkml/CAHk-=wi5c=_-FBGo_88CowJd_F-Gi6Ud9d=TALm65ReN7YjrMw@mail.gmail.com/

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index c7cdbb3e4aa68..474ca86ec105e 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
[ ... ]
> @@ -2035,25 +2035,6 @@ extern void __set_task_comm(struct task_struct *tsk, const char *from, bool exec
>  	__set_task_comm(tsk, from, false);		\
>  })
>  
> -/*
> - * - Why not use task_lock()?
> - *   User space can randomly change their names anyway, so locking for readers
> - *   doesn't make sense. For writers, locking is probably necessary, as a race
> - *   condition could lead to long-term mixed results.
> - *   The strscpy_pad() in __set_task_comm() can ensure that the task comm is
> - *   always NUL-terminated and zero-padded. Therefore the race condition between
> - *   reader and writer is not an issue.
> - *
> - * - BUILD_BUG_ON() can help prevent the buf from being truncated.
> - *   Since the callers don't perform any return value checks, this safeguard is
> - *   necessary.
> - */
> -#define get_task_comm(buf, tsk) ({			\
> -	BUILD_BUG_ON(sizeof(buf) < TASK_COMM_LEN);	\
> -	strscpy_pad(buf, (tsk)->comm);			\
> -	buf;						\
> -})
> -

[Severity: Low]
Does this removal cause a build failure when CONFIG_SECURITY_SMACK is enabled?

There appear to be remaining callers of get_task_comm() in the Smack security 
module that were missed in this treewide replacement. 

For example, in security/smack/smack_access.c:smack_log_callback():

    get_task_comm(comm, sad->subj_tsk)

And in security/smack/smack_lsm.c:smk_bu_tsk_to_obj():

    acc, get_task_comm(comm, tsk), note);

Without the macro definition in sched.h, will this result in implicit 
function declaration or undeclared identifier errors during compilation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-tonyk-long_name-v5-0-5fa843782a00@igalia.com?part=1