2023-06-01 00:55:37

by ~akihirosuda

[permalink] [raw]
Subject: [PATCH linux v2] net/ipv4: ping_group_range: allow GID from 2147483648 to 4294967294

From: Akihiro Suda <[email protected]>

With this commit, all the GIDs ("0 4294967294") can be written to the
"net.ipv4.ping_group_range" sysctl.

Note that 4294967295 (0xffffffff) is an invalid GID (see gid_valid() in
include/linux/uidgid.h), and an attempt to register this number will cause
-EINVAL.

Prior to this commit, only up to GID 2147483647 could be covered.
Documentation/networking/ip-sysctl.rst had "0 4294967295" as an example
value, but this example was wrong and causing -EINVAL.

v1->v2: Simplified the patch (Thanks to Kuniyuki Iwashima for suggestion)

Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind")
Signed-off-by: Akihiro Suda <[email protected]>
---
Documentation/networking/ip-sysctl.rst | 4 ++--
include/net/ping.h | 6 +-----
net/ipv4/sysctl_net_ipv4.c | 8 ++++----
3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 6ec06a33688a..80b8f73a0244 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -1352,8 +1352,8 @@ ping_group_range - 2 INTEGERS
Restrict ICMP_PROTO datagram sockets to users in the group range.
The default is "1 0", meaning, that nobody (not even root) may
create ping sockets. Setting it to "100 100" would grant permissions
- to the single group. "0 4294967295" would enable it for the world, "100
- 4294967295" would enable it for the users, but not daemons.
+ to the single group. "0 4294967294" would enable it for the world, "100
+ 4294967294" would enable it for the users, but not daemons.

tcp_early_demux - BOOLEAN
Enable early demux for established TCP sockets.
diff --git a/include/net/ping.h b/include/net/ping.h
index 9233ad3de0ad..bc7779262e60 100644
--- a/include/net/ping.h
+++ b/include/net/ping.h
@@ -16,11 +16,7 @@
#define PING_HTABLE_SIZE 64
#define PING_HTABLE_MASK (PING_HTABLE_SIZE-1)

-/*
- * gid_t is either uint or ushort. We want to pass it to
- * proc_dointvec_minmax(), so it must not be larger than MAX_INT
- */
-#define GID_T_MAX (((gid_t)~0U) >> 1)
+#define GID_T_MAX (((gid_t)~0U) - 1)

/* Compatibility glue so we can support IPv6 when it's compiled as a module */
struct pingv6_ops {
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 40fe70fc2015..bb49d9407c45 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -34,8 +34,8 @@ static int ip_ttl_min = 1;
static int ip_ttl_max = 255;
static int tcp_syn_retries_min = 1;
static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
-static int ip_ping_group_range_min[] = { 0, 0 };
-static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
+static long ip_ping_group_range_min[] = { 0, 0 };
+static long ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
static u32 u32_max_div_HZ = UINT_MAX / HZ;
static int one_day_secs = 24 * 3600;
static u32 fib_multipath_hash_fields_all_mask __maybe_unused =
@@ -165,7 +165,7 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
{
struct user_namespace *user_ns = current_user_ns();
int ret;
- gid_t urange[2];
+ unsigned long urange[2];
kgid_t low, high;
struct ctl_table tmp = {
.data = &urange,
@@ -178,7 +178,7 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
inet_get_ping_group_range_table(table, &low, &high);
urange[0] = from_kgid_munged(user_ns, low);
urange[1] = from_kgid_munged(user_ns, high);
- ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
+ ret = proc_doulongvec_minmax(&tmp, write, buffer, lenp, ppos);

if (write && ret == 0) {
low = make_kgid(user_ns, urange[0]);
--
2.38.4


2023-06-01 01:51:55

by Kuniyuki Iwashima

[permalink] [raw]
Subject: Re: [PATCH linux v2] net/ipv4: ping_group_range: allow GID from 2147483648 to 4294967294

From: ~akihirosuda <[email protected]>
Date: Wed, 31 May 2023 19:42:49 +0900
> From: Akihiro Suda <[email protected]>
>
> With this commit, all the GIDs ("0 4294967294") can be written to the
> "net.ipv4.ping_group_range" sysctl.
>
> Note that 4294967295 (0xffffffff) is an invalid GID (see gid_valid() in
> include/linux/uidgid.h), and an attempt to register this number will cause
> -EINVAL.
>
> Prior to this commit, only up to GID 2147483647 could be covered.
> Documentation/networking/ip-sysctl.rst had "0 4294967295" as an example
> value, but this example was wrong and causing -EINVAL.
>
> v1->v2: Simplified the patch (Thanks to Kuniyuki Iwashima for suggestion)

Changelog should be placed under '---'.

Also could you use 'net' instead of 'linux' in Subject so that
patchwork will be happy ?

https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/


>
> Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind")
> Signed-off-by: Akihiro Suda <[email protected]>
> ---
> Documentation/networking/ip-sysctl.rst | 4 ++--
> include/net/ping.h | 6 +-----
> net/ipv4/sysctl_net_ipv4.c | 8 ++++----
> 3 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index 6ec06a33688a..80b8f73a0244 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -1352,8 +1352,8 @@ ping_group_range - 2 INTEGERS
> Restrict ICMP_PROTO datagram sockets to users in the group range.
> The default is "1 0", meaning, that nobody (not even root) may
> create ping sockets. Setting it to "100 100" would grant permissions
> - to the single group. "0 4294967295" would enable it for the world, "100
> - 4294967295" would enable it for the users, but not daemons.
> + to the single group. "0 4294967294" would enable it for the world, "100
> + 4294967294" would enable it for the users, but not daemons.
>
> tcp_early_demux - BOOLEAN
> Enable early demux for established TCP sockets.
> diff --git a/include/net/ping.h b/include/net/ping.h
> index 9233ad3de0ad..bc7779262e60 100644
> --- a/include/net/ping.h
> +++ b/include/net/ping.h
> @@ -16,11 +16,7 @@
> #define PING_HTABLE_SIZE 64
> #define PING_HTABLE_MASK (PING_HTABLE_SIZE-1)
>
> -/*
> - * gid_t is either uint or ushort. We want to pass it to
> - * proc_dointvec_minmax(), so it must not be larger than MAX_INT
> - */
> -#define GID_T_MAX (((gid_t)~0U) >> 1)
> +#define GID_T_MAX (((gid_t)~0U) - 1)
>
> /* Compatibility glue so we can support IPv6 when it's compiled as a module */
> struct pingv6_ops {
> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> index 40fe70fc2015..bb49d9407c45 100644
> --- a/net/ipv4/sysctl_net_ipv4.c
> +++ b/net/ipv4/sysctl_net_ipv4.c
> @@ -34,8 +34,8 @@ static int ip_ttl_min = 1;
> static int ip_ttl_max = 255;
> static int tcp_syn_retries_min = 1;
> static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
> -static int ip_ping_group_range_min[] = { 0, 0 };
> -static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
> +static long ip_ping_group_range_min[] = { 0, 0 };
> +static long ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };

nit: s/long/unsigned long/

Then, add

Signed-off-by: Kuniyuki Iwashima <[email protected]>

Thanks!


> static u32 u32_max_div_HZ = UINT_MAX / HZ;
> static int one_day_secs = 24 * 3600;
> static u32 fib_multipath_hash_fields_all_mask __maybe_unused =
> @@ -165,7 +165,7 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
> {
> struct user_namespace *user_ns = current_user_ns();
> int ret;
> - gid_t urange[2];
> + unsigned long urange[2];
> kgid_t low, high;
> struct ctl_table tmp = {
> .data = &urange,
> @@ -178,7 +178,7 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
> inet_get_ping_group_range_table(table, &low, &high);
> urange[0] = from_kgid_munged(user_ns, low);
> urange[1] = from_kgid_munged(user_ns, high);
> - ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> + ret = proc_doulongvec_minmax(&tmp, write, buffer, lenp, ppos);
>
> if (write && ret == 0) {
> low = make_kgid(user_ns, urange[0]);
> --
> 2.38.4


2023-06-01 03:40:57

by Akihiro Suda

[permalink] [raw]
Subject: Re: [PATCH linux v2] net/ipv4: ping_group_range: allow GID from 2147483648 to 4294967294

Thanks, submitted v3 with your suggestions
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

2023年6月1日(木) 10:27 Kuniyuki Iwashima <[email protected]>:
>
> From: ~akihirosuda <[email protected]>
> Date: Wed, 31 May 2023 19:42:49 +0900
> > From: Akihiro Suda <[email protected]>
> >
> > With this commit, all the GIDs ("0 4294967294") can be written to the
> > "net.ipv4.ping_group_range" sysctl.
> >
> > Note that 4294967295 (0xffffffff) is an invalid GID (see gid_valid() in
> > include/linux/uidgid.h), and an attempt to register this number will cause
> > -EINVAL.
> >
> > Prior to this commit, only up to GID 2147483647 could be covered.
> > Documentation/networking/ip-sysctl.rst had "0 4294967295" as an example
> > value, but this example was wrong and causing -EINVAL.
> >
> > v1->v2: Simplified the patch (Thanks to Kuniyuki Iwashima for suggestion)
>
> Changelog should be placed under '---'.
>
> Also could you use 'net' instead of 'linux' in Subject so that
> patchwork will be happy ?
>
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>
>
> >
> > Fixes: c319b4d76b9e ("net: ipv4: add IPPROTO_ICMP socket kind")
> > Signed-off-by: Akihiro Suda <[email protected]>
> > ---
> > Documentation/networking/ip-sysctl.rst | 4 ++--
> > include/net/ping.h | 6 +-----
> > net/ipv4/sysctl_net_ipv4.c | 8 ++++----
> > 3 files changed, 7 insertions(+), 11 deletions(-)
> >
> > diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> > index 6ec06a33688a..80b8f73a0244 100644
> > --- a/Documentation/networking/ip-sysctl.rst
> > +++ b/Documentation/networking/ip-sysctl.rst
> > @@ -1352,8 +1352,8 @@ ping_group_range - 2 INTEGERS
> > Restrict ICMP_PROTO datagram sockets to users in the group range.
> > The default is "1 0", meaning, that nobody (not even root) may
> > create ping sockets. Setting it to "100 100" would grant permissions
> > - to the single group. "0 4294967295" would enable it for the world, "100
> > - 4294967295" would enable it for the users, but not daemons.
> > + to the single group. "0 4294967294" would enable it for the world, "100
> > + 4294967294" would enable it for the users, but not daemons.
> >
> > tcp_early_demux - BOOLEAN
> > Enable early demux for established TCP sockets.
> > diff --git a/include/net/ping.h b/include/net/ping.h
> > index 9233ad3de0ad..bc7779262e60 100644
> > --- a/include/net/ping.h
> > +++ b/include/net/ping.h
> > @@ -16,11 +16,7 @@
> > #define PING_HTABLE_SIZE 64
> > #define PING_HTABLE_MASK (PING_HTABLE_SIZE-1)
> >
> > -/*
> > - * gid_t is either uint or ushort. We want to pass it to
> > - * proc_dointvec_minmax(), so it must not be larger than MAX_INT
> > - */
> > -#define GID_T_MAX (((gid_t)~0U) >> 1)
> > +#define GID_T_MAX (((gid_t)~0U) - 1)
> >
> > /* Compatibility glue so we can support IPv6 when it's compiled as a module */
> > struct pingv6_ops {
> > diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> > index 40fe70fc2015..bb49d9407c45 100644
> > --- a/net/ipv4/sysctl_net_ipv4.c
> > +++ b/net/ipv4/sysctl_net_ipv4.c
> > @@ -34,8 +34,8 @@ static int ip_ttl_min = 1;
> > static int ip_ttl_max = 255;
> > static int tcp_syn_retries_min = 1;
> > static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
> > -static int ip_ping_group_range_min[] = { 0, 0 };
> > -static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
> > +static long ip_ping_group_range_min[] = { 0, 0 };
> > +static long ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
>
> nit: s/long/unsigned long/
>
> Then, add
>
> Signed-off-by: Kuniyuki Iwashima <[email protected]>
>
> Thanks!
>
>
> > static u32 u32_max_div_HZ = UINT_MAX / HZ;
> > static int one_day_secs = 24 * 3600;
> > static u32 fib_multipath_hash_fields_all_mask __maybe_unused =
> > @@ -165,7 +165,7 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
> > {
> > struct user_namespace *user_ns = current_user_ns();
> > int ret;
> > - gid_t urange[2];
> > + unsigned long urange[2];
> > kgid_t low, high;
> > struct ctl_table tmp = {
> > .data = &urange,
> > @@ -178,7 +178,7 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
> > inet_get_ping_group_range_table(table, &low, &high);
> > urange[0] = from_kgid_munged(user_ns, low);
> > urange[1] = from_kgid_munged(user_ns, high);
> > - ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> > + ret = proc_doulongvec_minmax(&tmp, write, buffer, lenp, ppos);
> >
> > if (write && ret == 0) {
> > low = make_kgid(user_ns, urange[0]);
> > --
> > 2.38.4
>