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

[Eric Dumazet]

On Wed, May 31, 2023 at 9:19 PM Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> wrote:
>
> From: ~akihirosuda <akihirosuda@xxxxxxxxx>
> Date: Wed, 31 May 2023 19:42:49 +0900
> > From: Akihiro Suda <akihiro.suda.cz@xxxxxxxxxxxxx>
> >
> > 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 ?