[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-netdev
Subject:    Re: [PATCH linux] net/ipv4: ping_group_range: allow GID from 2147483648 to 4294967294
From:       Kuniyuki Iwashima <kuniyu () amazon ! com>
Date:       2023-05-31 22:59:47
Message-ID: 20230531225947.38239-1-kuniyu () amazon ! com
[Download RAW message or body]

From: Eric Dumazet <edumazet@google.com>
Date: Wed, 31 May 2023 23:09:02 +0200
> On Wed, May 31, 2023 at 9:19 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> > 
> > From: ~akihirosuda <akihirosuda@git.sr.ht>
> > Date: Wed, 31 May 2023 19:42:49 +0900
> > > From: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
> > > 
> > > 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.
> > > 
> > > In the implementation, proc_dointvec_minmax is no longer used because it
> > > does not support numbers from 2147483648 to 4294967294.
> > 
> > Good catch.
> > 
> > I think we can use proc_doulongvec_minmax() instead of open coding.
> > 
> > With the diff below:
> > 
> > ---8<---
> > # sysctl -a | grep ping
> > net.ipv4.ping_group_range = 0   2147483647
> > # sysctl -w net.ipv4.ping_group_range="0 4294967295"
> > sysctl: setting key "net.ipv4.ping_group_range": Invalid argument
> > # sysctl -w net.ipv4.ping_group_range="0 4294967294"
> > net.ipv4.ping_group_range = 0 4294967294
> > # sysctl -a | grep ping
> > net.ipv4.ping_group_range = 0   4294967294
> > ---8<---
> > 
> > ---8<---
> > diff --git a/include/net/ping.h b/include/net/ping.h
> > index 9233ad3de0ad..9b401b9a9d35 100644
> > --- a/include/net/ping.h
> > +++ b/include/net/ping.h
> > @@ -20,7 +20,7 @@
> > * 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)
> > 
> > /* 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 6ae3345a3bdf..11d401958673 100644
> > --- a/net/ipv4/sysctl_net_ipv4.c
> > +++ b/net/ipv4/sysctl_net_ipv4.c
> > @@ -35,8 +35,8 @@ static int ip_ttl_max = 255;
> > static int tcp_syn_retries_min = 1;
> > static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
> > static int tcp_syn_linear_timeouts_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 unsigned long ip_ping_group_range_min[] = { 0, 0 };
> > +static unsigned 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,8 +165,8 @@ static int ipv4_ping_group_range(struct ctl_table *table, int \
> > write, void *buffer, size_t *lenp, loff_t *ppos)
> > {
> > struct user_namespace *user_ns = current_user_ns();
> > +       unsigned long urange[2];
> > int ret;
> > -       gid_t urange[2];
> > kgid_t low, high;
> > struct ctl_table tmp = {
> > .data = &urange,
> > @@ -179,7 +179,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]);
> > ---8<---
> 
> 
> Will this work on 32bit build ?

It worked at least on my i686 build and qemu.

---8<---
# uname -a
Linux (none) 6.4.0-rc3-00648-g75455b906d82-dirty #76 SMP PREEMPT_DYNAMIC Wed May 31 \
21:30:31 UTC 2023 i686 GNU/Linux # sysctl -a | grep ping
net.ipv4.ping_group_range = 1	0
# sysctl -w net.ipv4.ping_group_range="0 4294967295"
sysctl: setting key "net.ipv4.ping_group_range": Invalid argument
# sysctl -w net.ipv4.ping_group_range="0 4294967294"
net.ipv4.ping_group_range = 0 4294967294
# sysctl -a | grep ping
net.ipv4.ping_group_range = 0	4294967294
---8<---


[prev in list] [next in list] [prev in thread] [next in thread]