On Thu, Nov 14, 2024 at 05:25:51PM +0100, nicolas.bouchinet@xxxxxxxxxxx wrote:From my observations, vdso_enabled is a unsigned int. If one wants to
From: Nicolas Bouchinet <nicolas.bouchinet@xxxxxxxxxxx>It would be interesting to know what happens when you do a
Commit 3b3376f222e3 ("sysctl.c: fix underflow value setting risk in
vm_table") fixes underflow value setting risk in vm_table but misses
vdso_enabled sysctl.
vdso_enabled sysctl is initialized with .extra1 value as SYSCTL_ZERO to
avoid negative value writes but the proc_handler is proc_dointvec and not
proc_dointvec_minmax and thus do not uses .extra1 and .extra2.
The following command thus works :
`# echo -1 > /proc/sys/vm/vdso_enabled`
# echo (INT_MAX + 1) > /proc/sys/vm/vdso_enabled
This is the reasons why I'm interested in such a test:
1. Both proc_dointvec and proc_dointvec_minmax (calls proc_dointvec) have a
overflow check where they will return -EINVAL if what is given by the user is
greater than (unsiged long)INT_MAX; this will evaluate can evaluate to true
or false depending on the architecture where we are running.
2. I noticed that vdso_enabled is an unsigned long. And so the expectation is
that the range is 0 to ULONG_MAX, which in some cases (depending on the arch)
would not be the case.
If proc_dointvec or its derivative is used, as you said, range is bounded
So my question is: What is the expected range for this value? Because you might
not be getting the whole range in the cases where int is 32 bit and long is 64
bit.
This patch properly sets the proc_handler to proc_dointvec_minmax.Any reason why extra2 is not defined. I know that it was not defined before, but
Fixes: 3b3376f222e3 ("sysctl.c: fix underflow value setting risk in vm_table")
Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@xxxxxxxxxxx>
---
kernel/sysctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 79e6cb1d5c48f..37b1c1a760985 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2194,7 +2194,7 @@ static struct ctl_table vm_table[] = {
.maxlen = sizeof(vdso_enabled),
#endif
.mode = 0644,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_dointvec_minmax,
.extra1 = SYSCTL_ZERO,
this does not mean that it will not have an upper limit. The way that I read the
situation is that this will be bounded by the overflow check done in
proc_dointvec and will have an upper limit of INT_MAX.
Thanks again for your review,
Please correct me if I have read the situation incorrectly.
Best