linux-kernel - Re: [PATCH v2 1/1] net: sysctl: Bound check gc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANn89iJ5CMe+gYpEKJFWu6hBVWNZt9VFCc7ANu3_gk6_88zh=A@mail.gmail.com>
Date: Tue, 28 Jan 2025 14:50:48 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: nicolas.bouchinet@...p-os.org
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>, Joel Granados <j.granados@...sung.com>, 
	"David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Steffen Klassert <steffen.klassert@...unet.com>, Herbert Xu <herbert@...dor.apana.org.au>
Subject: Re: [PATCH v2 1/1] net: sysctl: Bound check gc_thresh sysctls

On Tue, Jan 28, 2025 at 11:39 AM <nicolas.bouchinet@...p-os.org> wrote:
>
> From: Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>
>
> 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@....gouv.fr>

gc_thresh is "unsigned int"

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

Your patch seems not needed to me.