| Message ID | 20241213054610.55843-6-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | vsprintf: Add %pTN to print Task Name | expand |
On Fri, Dec 13, 2024 at 12:47 AM Yafang Shao <laoar.shao@gmail.com> wrote: > > Since task->comm is guaranteed to be NUL-terminated, we can print it > directly without the need to copy it into a separate buffer. This > simplifies the code and avoids unnecessary operations. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > Cc: Kees Cook <kees@kernel.org> > Cc: Paul Moore <paul@paul-moore.com> > Cc: James Morris <jmorris@namei.org> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > --- > security/yama/yama_lsm.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) You need to wait for Kees' ACK, but this looks okay to me. Reviewed-by: Paul Moore <paul@paul-moore.com> > diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c > index e1a5e13ea269..4bdfa51ea6fd 100644 > --- a/security/yama/yama_lsm.c > +++ b/security/yama/yama_lsm.c > @@ -76,7 +76,6 @@ static void report_access(const char *access, struct task_struct *target, > struct task_struct *agent) > { > struct access_report_info *info; > - char agent_comm[sizeof(agent->comm)]; > > assert_spin_locked(&target->alloc_lock); /* for target->comm */ > > @@ -85,9 +84,8 @@ static void report_access(const char *access, struct task_struct *target, > * Imagine angry ranting about procfs here. > */ > pr_notice_ratelimited( > - "ptrace %s of \"%s\"[%d] was attempted by \"%s\"[%d]\n", > - access, target->comm, target->pid, > - get_task_comm(agent_comm, agent), agent->pid); > + "ptrace %s of \"%pTN\"[%d] was attempted by \"%pTN\"[%d]\n", > + access, target, target->pid, agent, agent->pid); > return; > } > > -- > 2.43.5
On Fri, Dec 13, 2024 at 01:46:08PM +0800, Yafang Shao wrote: > Since task->comm is guaranteed to be NUL-terminated, we can print it > directly without the need to copy it into a separate buffer. This > simplifies the code and avoids unnecessary operations. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Looks good to me; thanks! Acked-by: Kees Cook <kees@kernel.org>
On Thu, 12 Dec 2024 at 21:47, Yafang Shao <laoar.shao@gmail.com> wrote: > > Since task->comm is guaranteed to be NUL-terminated, we can print it > directly without the need to copy it into a separate buffer. So i think we should do the "without copying into a separate buffer" part of this series, but I do think we should just accept "%s" and "task->comm". IOW - getting rid of get_task_comm() is good. But the "%pTN" pointer format ends up being unnecessary. Linus
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index e1a5e13ea269..4bdfa51ea6fd 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -76,7 +76,6 @@ static void report_access(const char *access, struct task_struct *target, struct task_struct *agent) { struct access_report_info *info; - char agent_comm[sizeof(agent->comm)]; assert_spin_locked(&target->alloc_lock); /* for target->comm */ @@ -85,9 +84,8 @@ static void report_access(const char *access, struct task_struct *target, * Imagine angry ranting about procfs here. */ pr_notice_ratelimited( - "ptrace %s of \"%s\"[%d] was attempted by \"%s\"[%d]\n", - access, target->comm, target->pid, - get_task_comm(agent_comm, agent), agent->pid); + "ptrace %s of \"%pTN\"[%d] was attempted by \"%pTN\"[%d]\n", + access, target, target->pid, agent, agent->pid); return; }
Since task->comm is guaranteed to be NUL-terminated, we can print it directly without the need to copy it into a separate buffer. This simplifies the code and avoids unnecessary operations. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> Cc: Kees Cook <kees@kernel.org> Cc: Paul Moore <paul@paul-moore.com> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> --- security/yama/yama_lsm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)