Re: [PATCH v2 1/1] net: sysctl: Bound check gc_thresh sysctls

[Eric Dumazet]

On Tue, Jan 28, 2025 at 11:39 AM <nicolas.bouchinet@xxxxxxxxxxx> wrote:
>
> From: Nicolas Bouchinet <nicolas.bouchinet@xxxxxxxxxxx>
>
> ipv4, ipv6 and xfrm6 gc_thresh sysctls were authorized to be written any
> negative values, which would be stored in an unsigned int backing data
> (the gc_thresh variable in the dst_ops struct) since the proc_handler
> was proc_dointvec.
>
> It seems to be used to disables garbage collection of
> `net/ipv4/route/gc_thresh` since commit: 4ff3885262d0 ("ipv4: Delete
> routing cache."). gc_thresh variable being set to `~0`.

Simply reflecting this sysctl was kept to avoid user scripts to fail.

Kernel ignores its value.

>
> To clarify the sysctl interface, the proc_handler has thus been updated
> to proc_dointvec_minmax and writings have between limited between
> SYSCTL_NEG_ONE and SYSCTL_INT_MAX.
>
> With this patch applied, sysctl writes outside the defined in the bound
> will thus lead to a write error :
>
> ```
> echo "-2" > /proc/sys/net/ipv4/route/gc_thresh
> bash: echo: write error: Invalid argument
> ```
>
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@xxxxxxxxxxx>

gc_thresh is "unsigned int"

Why would we allow setting it to 0xFFFFFFFF but not 0xCF012345 ?

Your patch seems not needed to me.