Messages in this thread | | | From | Yafang Shao <> | Date | Thu, 7 Oct 2021 23:45:08 +0800 | Subject | Re: [PATCH v2 1/4] cn_proc.h: use TASK_COMM_LEN instread of 16 in struct proc_event |
| |
On Thu, Oct 7, 2021 at 11:09 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Oct 07, 2021 at 07:51:35AM -0700, Kees Cook wrote: > > On Thu, Oct 07, 2021 at 12:07:49PM +0000, Yafang Shao wrote: > > > struct comm_proc_event was introduced in commit > > > f786ecba4158 ("connector: add comm change event report to proc connector"). > > > It seems that there is no strong reason we must define the comm as a > > > hardcode 16 bytes. So we can use TASK_COMM_LEN instead. > > > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > > Cc: Vladimir Zapolskiy <vzapolskiy@gmail.com> > > > Cc: Kees Cook <keescook@chromium.org> > > > Cc: Al Viro <viro@zeniv.linux.org.uk> > > > Cc: Petr Mladek <pmladek@suse.com> > > > --- > > > include/uapi/linux/cn_proc.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/uapi/linux/cn_proc.h b/include/uapi/linux/cn_proc.h > > > index db210625cee8..351d02786350 100644 > > > --- a/include/uapi/linux/cn_proc.h > > > +++ b/include/uapi/linux/cn_proc.h > > > @@ -110,7 +110,7 @@ struct proc_event { > > > struct comm_proc_event { > > > __kernel_pid_t process_pid; > > > __kernel_pid_t process_tgid; > > > - char comm[16]; > > > + char comm[TASK_COMM_LEN]; > > > } comm; > > > > Hrmm. This is UAPI -- we can't change it without potentially breaking > > things (i.e. userspace binaries have this size built in, so we can't > > just change the size). This will either need to stay truncated, or may > > need a new interface with a variable-sized structure... > > Specifically, this is needed for this series: > > > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c > index 646ad385e490..34bcba25c488 100644 > --- a/drivers/connector/cn_proc.c > +++ b/drivers/connector/cn_proc.c > @@ -230,7 +230,9 @@ void proc_comm_connector(struct task_struct *task) > ev->what = PROC_EVENT_COMM; > ev->event_data.comm.process_pid = task->pid; > ev->event_data.comm.process_tgid = task->tgid; > - get_task_comm(ev->event_data.comm.comm, task); > + /* This may get truncated. */ > + __get_task_comm(ev->event_data.comm.comm, > + sizeof(ev->event_data.comm.comm), task); > > memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id)); > msg->ack = 0; /* not used */ >
Thanks for the suggestion. I will keep the UAPI code as-is and change it as you suggested.
-- Thanks Yafang
| |