* [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).