netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport
@ 2024-07-30 16:04 Dmitry Antipov
  2024-07-30 18:41 ` Gustavo A. R. Silva
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dmitry Antipov @ 2024-07-30 16:04 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: Kees Cook, netdev, linux-hardening, Dmitry Antipov

According to '__reuseport_alloc()', annotate trailing VLA 'sock' of
'struct sock_reuseport' with '__counted_by()' and use convenient
'struct_size()' to simplify the math used in 'kzalloc()'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 include/net/sock_reuseport.h | 2 +-
 net/core/sock_reuseport.c    | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
index 6ec140b0a61b..6e4faf3ee76f 100644
--- a/include/net/sock_reuseport.h
+++ b/include/net/sock_reuseport.h
@@ -26,7 +26,7 @@ struct sock_reuseport {
 	unsigned int		bind_inany:1;
 	unsigned int		has_conns:1;
 	struct bpf_prog __rcu	*prog;		/* optional BPF sock selector */
-	struct sock		*socks[];	/* array of sock pointers */
+	struct sock		*socks[] __counted_by(max_socks);
 };
 
 extern int reuseport_alloc(struct sock *sk, bool bind_inany);
diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index 5a165286e4d8..5eea73aaeb0f 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -173,11 +173,10 @@ static bool __reuseport_detach_closed_sock(struct sock *sk,
 
 static struct sock_reuseport *__reuseport_alloc(unsigned int max_socks)
 {
-	unsigned int size = sizeof(struct sock_reuseport) +
-		      sizeof(struct sock *) * max_socks;
-	struct sock_reuseport *reuse = kzalloc(size, GFP_ATOMIC);
+	struct sock_reuseport *reuse =
+		kzalloc(struct_size(reuse, socks, max_socks), GFP_ATOMIC);
 
-	if (!reuse)
+	if (unlikely(!reuse))
 		return NULL;
 
 	reuse->max_socks = max_socks;
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport
  2024-07-30 16:04 [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport Dmitry Antipov
@ 2024-07-30 18:41 ` Gustavo A. R. Silva
  2024-07-30 19:06 ` Kuniyuki Iwashima
  2024-07-31  0:01 ` Jakub Kicinski
  2 siblings, 0 replies; 6+ messages in thread
From: Gustavo A. R. Silva @ 2024-07-30 18:41 UTC (permalink / raw)
  To: Dmitry Antipov, Paolo Abeni; +Cc: Kees Cook, netdev, linux-hardening



On 30/07/24 10:04, Dmitry Antipov wrote:
> According to '__reuseport_alloc()', annotate trailing VLA 'sock' of

`socks` is a flexible-array member[1], not a VLA[2].

> 'struct sock_reuseport' with '__counted_by()' and use convenient
> 'struct_size()' to simplify the math used in 'kzalloc()'. >
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Looks correct.

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

> ---
>   include/net/sock_reuseport.h | 2 +-
>   net/core/sock_reuseport.c    | 7 +++----
>   2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
> index 6ec140b0a61b..6e4faf3ee76f 100644
> --- a/include/net/sock_reuseport.h
> +++ b/include/net/sock_reuseport.h
> @@ -26,7 +26,7 @@ struct sock_reuseport {
>   	unsigned int		bind_inany:1;
>   	unsigned int		has_conns:1;
>   	struct bpf_prog __rcu	*prog;		/* optional BPF sock selector */
> -	struct sock		*socks[];	/* array of sock pointers */
> +	struct sock		*socks[] __counted_by(max_socks);
>   };
>   
>   extern int reuseport_alloc(struct sock *sk, bool bind_inany);
> diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
> index 5a165286e4d8..5eea73aaeb0f 100644
> --- a/net/core/sock_reuseport.c
> +++ b/net/core/sock_reuseport.c
> @@ -173,11 +173,10 @@ static bool __reuseport_detach_closed_sock(struct sock *sk,
>   
>   static struct sock_reuseport *__reuseport_alloc(unsigned int max_socks)
>   {
> -	unsigned int size = sizeof(struct sock_reuseport) +
> -		      sizeof(struct sock *) * max_socks;
> -	struct sock_reuseport *reuse = kzalloc(size, GFP_ATOMIC);
> +	struct sock_reuseport *reuse =
> +		kzalloc(struct_size(reuse, socks, max_socks), GFP_ATOMIC);
>   
> -	if (!reuse)
> +	if (unlikely(!reuse))
>   		return NULL;
>   
>   	reuse->max_socks = max_socks;
Thanks
--
Gustavo

[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport
  2024-07-30 16:04 [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport Dmitry Antipov
  2024-07-30 18:41 ` Gustavo A. R. Silva
@ 2024-07-30 19:06 ` Kuniyuki Iwashima
  2024-07-31  0:01 ` Jakub Kicinski
  2 siblings, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2024-07-30 19:06 UTC (permalink / raw)
  To: dmantipov; +Cc: kees, linux-hardening, netdev, pabeni, Kuniyuki Iwashima

From: Dmitry Antipov <dmantipov@yandex.ru>
Date: Tue, 30 Jul 2024 19:04:49 +0300
> According to '__reuseport_alloc()', annotate trailing VLA 'sock' of
> 'struct sock_reuseport' with '__counted_by()' and use convenient
> 'struct_size()' to simplify the math used in 'kzalloc()'.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport
  2024-07-30 16:04 [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport Dmitry Antipov
  2024-07-30 18:41 ` Gustavo A. R. Silva
  2024-07-30 19:06 ` Kuniyuki Iwashima
@ 2024-07-31  0:01 ` Jakub Kicinski
  2024-07-31  4:53   ` [PATCH v2] net: core: annotate socks of struct sock_reuseport with __counted_by Dmitry Antipov
  2 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2024-07-31  0:01 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: Paolo Abeni, Kees Cook, netdev, linux-hardening

On Tue, 30 Jul 2024 19:04:49 +0300 Dmitry Antipov wrote:
> -	unsigned int size = sizeof(struct sock_reuseport) +
> -		      sizeof(struct sock *) * max_socks;
> -	struct sock_reuseport *reuse = kzalloc(size, GFP_ATOMIC);
> +	struct sock_reuseport *reuse =
> +		kzalloc(struct_size(reuse, socks, max_socks), GFP_ATOMIC);
>  

please move the function call out of the init. It doesn't fit on a
single line so what's the point..

> -	if (!reuse)
> +	if (unlikely(!reuse))

And why add the unlikely here? Seems unrelated..
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] net: core: annotate socks of struct sock_reuseport with __counted_by
  2024-07-31  0:01 ` Jakub Kicinski
@ 2024-07-31  4:53   ` Dmitry Antipov
  2024-07-31 23:50     ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Antipov @ 2024-07-31  4:53 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Paolo Abeni, Gustavo A . R . Silva, Kuniyuki Iwashima, Kees Cook,
	netdev, linux-hardening, Dmitry Antipov

According to '__reuseport_alloc()', annotate flexible array member
'sock' of 'struct sock_reuseport' with '__counted_by()' and use
convenient 'struct_size()' to simplify the math used in 'kzalloc()'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v2: style (Jakub), title and commit message (Gustavo) adjustments
---
 include/net/sock_reuseport.h | 2 +-
 net/core/sock_reuseport.c    | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
index 6ec140b0a61b..6e4faf3ee76f 100644
--- a/include/net/sock_reuseport.h
+++ b/include/net/sock_reuseport.h
@@ -26,7 +26,7 @@ struct sock_reuseport {
 	unsigned int		bind_inany:1;
 	unsigned int		has_conns:1;
 	struct bpf_prog __rcu	*prog;		/* optional BPF sock selector */
-	struct sock		*socks[];	/* array of sock pointers */
+	struct sock		*socks[] __counted_by(max_socks);
 };
 
 extern int reuseport_alloc(struct sock *sk, bool bind_inany);
diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index 5a165286e4d8..79f2456e27a5 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -173,9 +173,9 @@ static bool __reuseport_detach_closed_sock(struct sock *sk,
 
 static struct sock_reuseport *__reuseport_alloc(unsigned int max_socks)
 {
-	unsigned int size = sizeof(struct sock_reuseport) +
-		      sizeof(struct sock *) * max_socks;
-	struct sock_reuseport *reuse = kzalloc(size, GFP_ATOMIC);
+	struct sock_reuseport *reuse;
+
+	reuse = kzalloc(struct_size(reuse, socks, max_socks), GFP_ATOMIC);
 
 	if (!reuse)
 		return NULL;
-- 
2.45.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] net: core: annotate socks of struct sock_reuseport with __counted_by
  2024-07-31  4:53   ` [PATCH v2] net: core: annotate socks of struct sock_reuseport with __counted_by Dmitry Antipov
@ 2024-07-31 23:50     ` Jakub Kicinski
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2024-07-31 23:50 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Paolo Abeni, Gustavo A . R . Silva, Kuniyuki Iwashima, Kees Cook,
	netdev, linux-hardening

On Wed, 31 Jul 2024 07:53:46 +0300 Dmitry Antipov wrote:
> +	reuse = kzalloc(struct_size(reuse, socks, max_socks), GFP_ATOMIC);
>  
>  	if (!reuse)

another nit -- no empty lines between function call and its error check
:)

Two bits of docs to look at before sending v3:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#resending-after-review
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#changes-requested
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-07-31 23:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-30 16:04 [PATCH] net: core: use __counted_by for trailing VLA of struct sock_reuseport Dmitry Antipov
2024-07-30 18:41 ` Gustavo A. R. Silva
2024-07-30 19:06 ` Kuniyuki Iwashima
2024-07-31  0:01 ` Jakub Kicinski
2024-07-31  4:53   ` [PATCH v2] net: core: annotate socks of struct sock_reuseport with __counted_by Dmitry Antipov
2024-07-31 23:50     ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).