Re: [PATCH net-next v2] af_unix: Refine UNIX pathname sockets autobind identifier length

[Kuniyuki Iwashima]

> Subject: [PATCH net-next v2] af_unix: Refine UNIX pathname sockets autobind identifier length

s/pathname/abstract/

In the v1 thread, I meant "filesystem-based sockets" is now called
pathname sockets.  sockets whose name starts with \0 are abstract
sockets.


From: Liang Jie <buaajxlj@xxxxxxx>
Date: Thu,  6 Feb 2025 13:44:51 +0800
> Refines autobind identifier length for UNIX pathname sockets, addressing

same here, abstract sockets.


> issues of memory waste and code readability.
> 
> The previous implementation in the unix_autobind function of UNIX pathname
> sockets used hardcoded values such as 16 and 6 for memory allocation and

nit: 6 isn't used for mem alloc.


> setting the length of the autobind identifier, which was not only
> inflexible but also led to reduced code clarity. Additionally, allocating

you need not mention inflexibility as the length are fixed and won't be
changed (it was changed once though)


> 16 bytes of memory for the autobind path was excessive, given that only 6
> bytes were ultimately used.
> 
> To mitigate these issues, introduces the following changes:
>  - A new macro UNIX_AUTOBIND_LEN is defined to clearly represent the total
>    length of the autobind identifier, which improves code readability and
>    maintainability. It is set to 6 bytes to accommodate the unique autobind
>    process identifier.
>  - Memory allocation for the autobind path is now precisely based on
>    UNIX_AUTOBIND_LEN, thereby preventing memory waste.
>  - To avoid buffer overflow and ensure that only the intended number of
>    bytes are written, sprintf is replaced by snprintf with the proper
>    buffer size set explicitly.
> 
> The modifications result in a leaner memory footprint and elevated code
> quality, ensuring that the functional aspect of autobind behavior in UNIX
> pathname sockets remains intact.

s/pathname/abstract/

Overall, the commit message is a bit wordy.  It can be simplified just like

  unix_autobind() allocates 16 bytes but uses 6 bytes only.

  Let's allocate 6 bytes only and use snprintf() to avoid
  unwanted null-termination.


> @@ -1203,12 +1205,12 @@ static int unix_autobind(struct sock *sk)
>  		goto out;
>  
>  	err = -ENOMEM;
> -	addr = kzalloc(sizeof(*addr) +
> -		       offsetof(struct sockaddr_un, sun_path) + 16, GFP_KERNEL);
> +	addr = kzalloc(sizeof(*addr) + offsetof(struct sockaddr_un, sun_path) +
> +			UNIX_AUTOBIND_LEN, GFP_KERNEL);

nit: indent is wrong here.

If you are using emacs, add the following to your config:

(setq-default c-default-style "linux")