2024-02-09 12:11:16

by Imran Khan

Subject: [PATCH] connector/cn_proc: make comm length as TASK_COMM_LEN.

Since comm_proc_event::comm contains a task_struct::comm,
make its size same as TASK_COMM_LEN and avoid magic number
in buffer size.

Signed-off-by: Imran Khan <[email protected]>
---
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 f2afb7cc4926..fd876c9b284f 100644
--- a/include/uapi/linux/cn_proc.h
+++ b/include/uapi/linux/cn_proc.h
@@ -136,7 +136,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;

struct coredump_proc_event {
--
2.34.1

2024-02-09 15:50:42

by Jakub Kicinski

Subject: Re: [PATCH] connector/cn_proc: make comm length as TASK_COMM_LEN.

On Fri, 9 Feb 2024 23:10:46 +1100 Imran Khan wrote:
> Since comm_proc_event::comm contains a task_struct::comm,
> make its size same as TASK_COMM_LEN and avoid magic number
> in buffer size.

missed CCing netdev?

2024-02-09 23:19:58

by Imran Khan

Subject: Re: [PATCH] connector/cn_proc: make comm length as TASK_COMM_LEN.

Hello Jakub,
On 10/2/2024 2:50 am, Jakub Kicinski wrote:
> On Fri, 9 Feb 2024 23:10:46 +1100 Imran Khan wrote:
>> Since comm_proc_event::comm contains a task_struct::comm,
>> make its size same as TASK_COMM_LEN and avoid magic number
>> in buffer size.
>
> missed CCing netdev?
Thanks for getting back on this. Actually get_maintainer.pl for cn_proc.h was
not showing any maintainers, so I took the "To" list from cn_proc.c.
For CCing I stuck with what get_maintainer.pl was showing.
I have added netdev in CC now.

Thanks,
Imran

2024-02-16 17:33:02

by Simon Horman

Subject: Re: [PATCH] connector/cn_proc: make comm length as TASK_COMM_LEN.

On Sat, Feb 10, 2024 at 10:19:24AM +1100, Imran Khan wrote:
>
> Hello Jakub,
> On 10/2/2024 2:50 am, Jakub Kicinski wrote:
> > On Fri, 9 Feb 2024 23:10:46 +1100 Imran Khan wrote:
> >> Since comm_proc_event::comm contains a task_struct::comm,
> >> make its size same as TASK_COMM_LEN and avoid magic number
> >> in buffer size.
> >
> > missed CCing netdev?
> Thanks for getting back on this. Actually get_maintainer.pl for cn_proc.h was
> not showing any maintainers, so I took the "To" list from cn_proc.c.
> For CCing I stuck with what get_maintainer.pl was showing.
> I have added netdev in CC now.

Thanks Imran,

please consider submitting a patch to add cn_proc.h (x2?) to MAINTAINERS.

2024-02-16 18:57:31

by Alexey Dobriyan

Subject: Re: [PATCH] connector/cn_proc: make comm length as TASK_COMM_LEN.

> --- a/include/uapi/linux/cn_proc.h
> +++ b/include/uapi/linux/cn_proc.h
> @@ -136,7 +136,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;

No, 16 must remain 16.

This is ABI header, and TASK_COMM_LEN is in sched.h which is NOT ABI
header. This breaks compilation for everyone using it outside of kernel.

TASK_COMM_LEN is probably fixed at this point by countless trace events
but still... unless you've moving TASK_COMM_LEN to uapi/ too, this is
wrong.