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

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

[Imran Khan]

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 <imran.f.khan@oracle.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 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

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

[Jakub Kicinski]

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?

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

[Imran Khan]

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

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

[Simon Horman]

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.

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

[Alexey Dobriyan]

> --- 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.