linux-kernel - [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-next>] [day] [month] [year] [list]

Message-ID: <20250128103821.29745-1-nicolas.bouchinet@clip-os.org>
Date: Tue, 28 Jan 2025 11:38:17 +0100
From: nicolas.bouchinet@...p-os.org
To: netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>,
	Joel Granados <j.granados@...sung.com>,
	"David S. Miller" <davem@...emloft.net>,
	David Ahern <dsahern@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	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: [PATCH v2 1/1] net: sysctl: Bound check gc_thresh sysctls

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`.

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>

---

Changes since v1:
https://lore.kernel.org/all/20250127142014.37834-1-nicolas.bouchinet@clip-os.org/

* Detatched the patch from the patchset
* Updated the boundcheck between SYSCTL_NEG_ONE and SYSCTL_INT_MAX.
* Reworded the commit message to make it more clear.

---
 net/ipv4/route.c        | 4 +++-
 net/ipv6/route.c        | 4 +++-
 net/ipv6/xfrm6_policy.c | 4 +++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 0fbec35096186..96641ae15049a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -3453,7 +3453,9 @@ static struct ctl_table ipv4_route_table[] = {
 		.data		= &ipv4_dst_ops.gc_thresh,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_NEG_ONE,
+		.extra2		= SYSCTL_INT_MAX,
 	},
 	{
 		.procname	= "max_size",
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 67ff16c047180..3fc7f336dfa04 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -6379,7 +6379,9 @@ static struct ctl_table ipv6_route_table_template[] = {
 		.data		=	&ip6_dst_ops_template.gc_thresh,
 		.maxlen		=	sizeof(int),
 		.mode		=	0644,
-		.proc_handler	=	proc_dointvec,
+		.proc_handler	=	proc_dointvec_minmax,
+		.extra1		=	SYSCTL_NEG_ONE,
+		.extra2		=	SYSCTL_INT_MAX,
 	},
 	{
 		.procname	=	"flush",
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 1f19b6f14484c..1e212d5341839 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -189,7 +189,9 @@ static struct ctl_table xfrm6_policy_table[] = {
 		.data		= &init_net.xfrm.xfrm6_dst_ops.gc_thresh,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler   = proc_dointvec,
+		.proc_handler   = proc_dointvec_minmax,
+		.extra1		= SYSCTL_NEG_ONE,
+		.extra2		= SYSCTL_INT_MAX,
 	},
 };
 
-- 
2.48.1