Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <168553315664.20663.13753087625689463092-0@git.sr.ht> <20230531191909.95136-1-kuniyu@amazon.com>
In-Reply-To: <20230531191909.95136-1-kuniyu@amazon.com>
From:   Eric Dumazet <edumazet@google.com>
Date:   Wed, 31 May 2023 23:09:02 +0200
Message-ID: <CANn89iK13jkbKXv-rKiUbTqMrk3KjVPGYH_Vv7FtJJ5pTdUAYQ@mail.gmail.com>
Subject: Re: [PATCH linux] net/ipv4: ping_group_range: allow GID from
 2147483648 to 4294967294
To:     Kuniyuki Iwashima <kuniyu@amazon.com>
Cc:     akihirosuda@git.sr.ht, akihiro.suda.cz@hco.ntt.co.jp,
        davem@davemloft.net, kuba@kernel.org, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, pabeni@redhat.com, segoon@openwall.com,
        suda.kyoto@gmail.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, May 31, 2023 at 9:19=E2=80=AFPM Kuniyuki Iwashima <kuniyu@amazon.co=
m> 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 ca=
use
> > -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 i=
t
> > 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 =3D 0   2147483647
> # sysctl -w net.ipv4.ping_group_range=3D"0 4294967295"
> sysctl: setting key "net.ipv4.ping_group_range": Invalid argument
> # sysctl -w net.ipv4.ping_group_range=3D"0 4294967294"
> net.ipv4.ping_group_range =3D 0 4294967294
> # sysctl -a | grep ping
> net.ipv4.ping_group_range =3D 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 mod=
ule */
>  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 =3D 255;
>  static int tcp_syn_retries_min =3D 1;
>  static int tcp_syn_retries_max =3D MAX_TCP_SYNCNT;
>  static int tcp_syn_linear_timeouts_max =3D MAX_TCP_SYNCNT;
> -static int ip_ping_group_range_min[] =3D { 0, 0 };
> -static int ip_ping_group_range_max[] =3D { GID_T_MAX, GID_T_MAX };
> +static unsigned long ip_ping_group_range_min[] =3D { 0, 0 };
> +static unsigned long ip_ping_group_range_max[] =3D { GID_T_MAX, GID_T_MA=
X };
>  static u32 u32_max_div_HZ =3D UINT_MAX / HZ;
>  static int one_day_secs =3D 24 * 3600;
>  static u32 fib_multipath_hash_fields_all_mask __maybe_unused =3D
> @@ -165,8 +165,8 @@ static int ipv4_ping_group_range(struct ctl_table *ta=
ble, int write,
>                                  void *buffer, size_t *lenp, loff_t *ppos=
)
>  {
>         struct user_namespace *user_ns =3D current_user_ns();
> +       unsigned long urange[2];
>         int ret;
> -       gid_t urange[2];
>         kgid_t low, high;
>         struct ctl_table tmp =3D {
>                 .data =3D &urange,
> @@ -179,7 +179,7 @@ static int ipv4_ping_group_range(struct ctl_table *ta=
ble, int write,
>         inet_get_ping_group_range_table(table, &low, &high);
>         urange[0] =3D from_kgid_munged(user_ns, low);
>         urange[1] =3D from_kgid_munged(user_ns, high);
> -       ret =3D proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> +       ret =3D proc_doulongvec_minmax(&tmp, write, buffer, lenp, ppos);
>
>         if (write && ret =3D=3D 0) {
>                 low =3D make_kgid(user_ns, urange[0]);
> ---8<---


Will this work on 32bit build ?