On Sat, Jun 1, 2024 at 10:38 PM Yafang Shao <laoar.shao@xxxxxxxxx> wrote: > > Quoted from Linus [0]: > > selinux never wanted a lock, and never wanted any kind of *consistent* > result, it just wanted a *stable* result. > > Using __get_task_comm() to read the task comm ensures that the name is > always NUL-terminated, regardless of the source string. This approach also > facilitates future extensions to the task comm. > > Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx> > LINK: https://lore.kernel.org/all/CAHk-=wivfrF0_zvf+oj6==Sh=-npJooP8chLPEfaFV0oNYTTBA@xxxxxxxxxxxxxx/ [0] > Cc: Paul Moore <paul@xxxxxxxxxxxxxx> > Cc: James Morris <jmorris@xxxxxxxxx> > Cc: "Serge E. Hallyn" <serge@xxxxxxxxxx> > Cc: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> > Cc: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > --- > security/lsm_audit.c | 4 ++-- > security/selinux/selinuxfs.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) Similar to the audit change, as long as you sort out the __get_task_comm() issues such that it can operate without task_lock() this should be fine. Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx> > diff --git a/security/lsm_audit.c b/security/lsm_audit.c > index 849e832719e2..a922e4339dd5 100644 > --- a/security/lsm_audit.c > +++ b/security/lsm_audit.c > @@ -207,7 +207,7 @@ static void dump_common_audit_data(struct audit_buffer *ab, > BUILD_BUG_ON(sizeof(a->u) > sizeof(void *)*2); > > audit_log_format(ab, " pid=%d comm=", task_tgid_nr(current)); > - audit_log_untrustedstring(ab, memcpy(comm, current->comm, sizeof(comm))); > + audit_log_untrustedstring(ab, __get_task_comm(comm, sizeof(comm), current)); > > switch (a->type) { > case LSM_AUDIT_DATA_NONE: > @@ -302,7 +302,7 @@ static void dump_common_audit_data(struct audit_buffer *ab, > char comm[sizeof(tsk->comm)]; > audit_log_format(ab, " opid=%d ocomm=", pid); > audit_log_untrustedstring(ab, > - memcpy(comm, tsk->comm, sizeof(comm))); > + __get_task_comm(comm, sizeof(comm), tsk)); > } > } > break; > diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c > index e172f182b65c..a8a2ec742576 100644 > --- a/security/selinux/selinuxfs.c > +++ b/security/selinux/selinuxfs.c > @@ -708,7 +708,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf, > if (new_value) { > char comm[sizeof(current->comm)]; > > - memcpy(comm, current->comm, sizeof(comm)); > + __get_task_comm(comm, sizeof(comm), current); > pr_err("SELinux: %s (%d) set checkreqprot to 1. This is no longer supported.\n", > comm, current->pid); > } > -- > 2.39.1 -- paul-moore.com
