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 <[email protected]>

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=talm65ren7yj...@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/[email protected]?part=1

Reply via email to