kernel/sched/rt.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
The validation of the value written to sched_rt_period_us was broken
because:
- the sysclt_sched_rt_period is declared as unsigned int
- parsed by proc_do_intvec()
- the range is asserted after the value parsed by proc_do_intvec()
Because of this negative values written to the file were written into a
unsigned integer that were later on interpreted as large positive
integers which did passed the check:
if (sysclt_sched_rt_period <= 0)
return EINVAL;
This commit fixes the parsing by setting explicit range for both
perid_us and runtime_us into the sched_rt_sysctls table and processes
the values with proc_dointvec_minmax() instead.
Alternatively if we wanted to use full range of unsigned int for the
period value we would have to split the proc_handler and use
proc_douintvec() for it however even the
Documentation/scheduller/sched-rt-group.rst describes the range as 1 to
INT_MAX.
As far as I can tell the only problem this causes is that the sysctl
file allows writing negative values which when read back may confuse
userspace.
There is also a LTP test being submitted for these sysctl files at:
http://patchwork.ozlabs.org/project/ltp/patch/20230901144433.2526-1-chrubis@suse.cz/
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
kernel/sched/rt.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 0597ba0f85ff..aed3d55de2dd 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -37,6 +37,8 @@ static struct ctl_table sched_rt_sysctls[] = {
.maxlen = sizeof(unsigned int),
.mode = 0644,
.proc_handler = sched_rt_handler,
+ .extra1 = SYSCTL_ONE,
+ .extra2 = SYSCTL_INT_MAX,
},
{
.procname = "sched_rt_runtime_us",
@@ -44,6 +46,8 @@ static struct ctl_table sched_rt_sysctls[] = {
.maxlen = sizeof(int),
.mode = 0644,
.proc_handler = sched_rt_handler,
+ .extra1 = SYSCTL_NEG_ONE,
+ .extra2 = SYSCTL_INT_MAX,
},
{
.procname = "sched_rr_timeslice_ms",
@@ -2985,9 +2989,6 @@ static int sched_rt_global_constraints(void)
#ifdef CONFIG_SYSCTL
static int sched_rt_global_validate(void)
{
- if (sysctl_sched_rt_period <= 0)
- return -EINVAL;
-
if ((sysctl_sched_rt_runtime != RUNTIME_INF) &&
((sysctl_sched_rt_runtime > sysctl_sched_rt_period) ||
((u64)sysctl_sched_rt_runtime *
@@ -3018,7 +3019,7 @@ static int sched_rt_handler(struct ctl_table *table, int write, void *buffer,
old_period = sysctl_sched_rt_period;
old_runtime = sysctl_sched_rt_runtime;
- ret = proc_dointvec(table, write, buffer, lenp, ppos);
+ ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
if (!ret && write) {
ret = sched_rt_global_validate();
--
2.41.0> The validation of the value written to sched_rt_period_us was broken
> because:
> - the sysclt_sched_rt_period is declared as unsigned int
> - parsed by proc_do_intvec()
> - the range is asserted after the value parsed by proc_do_intvec()
> Because of this negative values written to the file were written into a
> unsigned integer that were later on interpreted as large positive
> integers which did passed the check:
> if (sysclt_sched_rt_period <= 0)
> return EINVAL;
> This commit fixes the parsing by setting explicit range for both
> perid_us and runtime_us into the sched_rt_sysctls table and processes
> the values with proc_dointvec_minmax() instead.
> Alternatively if we wanted to use full range of unsigned int for the
> period value we would have to split the proc_handler and use
> proc_douintvec() for it however even the
> Documentation/scheduller/sched-rt-group.rst describes the range as 1 to
> INT_MAX.
> As far as I can tell the only problem this causes is that the sysctl
> file allows writing negative values which when read back may confuse
> userspace.
> There is also a LTP test being submitted for these sysctl files at:
> http://patchwork.ozlabs.org/project/ltp/patch/20230901144433.2526-1-chrubis@suse.cz/
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> kernel/sched/rt.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 0597ba0f85ff..aed3d55de2dd 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -37,6 +37,8 @@ static struct ctl_table sched_rt_sysctls[] = {
> .maxlen = sizeof(unsigned int),
> .mode = 0644,
> .proc_handler = sched_rt_handler,
> + .extra1 = SYSCTL_ONE,
> + .extra2 = SYSCTL_INT_MAX,
> },
> {
> .procname = "sched_rt_runtime_us",
> @@ -44,6 +46,8 @@ static struct ctl_table sched_rt_sysctls[] = {
> .maxlen = sizeof(int),
> .mode = 0644,
> .proc_handler = sched_rt_handler,
> + .extra1 = SYSCTL_NEG_ONE,
> + .extra2 = SYSCTL_INT_MAX,
Documentation/scheduller/sched-rt-group.rst [1] specifies this as values from -1 to
(INT_MAX - 1), I guess due int range. Looking into proc_dointvec_minmax() [2]
even INT_MAX would pass the check. I suppose we can do nothing about that,
because there is no value in sysctl_vals[] which would represent INT_MAX - 1.
And you specify in LTP test range: from -1 to INT_MAX.
But even much shorter value than INT_MAX fails:
$ echo 1234567 > /proc/sys/kernel/sched_rt_runtime_us
sh: echo: write error: Invalid argument
Ranges in /proc/sys/kernel/sched_rt_period_us works as expected.
[1] https://www.kernel.org/doc/html/latest/scheduler/sched-rt-group.html#system-wide-settings
[2] https://elixir.bootlin.com/linux/latest/source/kernel/sysctl.c#L843
> },
> {
> .procname = "sched_rr_timeslice_ms",
> @@ -2985,9 +2989,6 @@ static int sched_rt_global_constraints(void)
> #ifdef CONFIG_SYSCTL
> static int sched_rt_global_validate(void)
> {
> - if (sysctl_sched_rt_period <= 0)
> - return -EINVAL;
> -
> if ((sysctl_sched_rt_runtime != RUNTIME_INF) &&
> ((sysctl_sched_rt_runtime > sysctl_sched_rt_period) ||
> ((u64)sysctl_sched_rt_runtime *
> @@ -3018,7 +3019,7 @@ static int sched_rt_handler(struct ctl_table *table, int write, void *buffer,
> old_period = sysctl_sched_rt_period;
> old_runtime = sysctl_sched_rt_runtime;
> - ret = proc_dointvec(table, write, buffer, lenp, ppos);
> + ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> if (!ret && write) {
> ret = sched_rt_global_validate();
Hi! > Documentation/scheduller/sched-rt-group.rst [1] specifies this as values from -1 to > (INT_MAX - 1), I guess due int range. Looking into proc_dointvec_minmax() [2] > even INT_MAX would pass the check. I suppose we can do nothing about that, > because there is no value in sysctl_vals[] which would represent INT_MAX - 1. > > And you specify in LTP test range: from -1 to INT_MAX. > > But even much shorter value than INT_MAX fails: > > $ echo 1234567 > /proc/sys/kernel/sched_rt_runtime_us > sh: echo: write error: Invalid argument That is because runtime_us must be < period_us by definition, since runtime_us defines how much time is allocated from the period_us. I guess that this is not described good enough in the kernel docs. -- Cyril Hrubis chrubis@suse.cz
* Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > Documentation/scheduller/sched-rt-group.rst [1] specifies this as values from -1 to > > (INT_MAX - 1), I guess due int range. Looking into proc_dointvec_minmax() [2] > > even INT_MAX would pass the check. I suppose we can do nothing about that, > > because there is no value in sysctl_vals[] which would represent INT_MAX - 1. > > > > And you specify in LTP test range: from -1 to INT_MAX. > > > > But even much shorter value than INT_MAX fails: > > > > $ echo 1234567 > /proc/sys/kernel/sched_rt_runtime_us > > sh: echo: write error: Invalid argument > > That is because runtime_us must be < period_us by definition, since > runtime_us defines how much time is allocated from the period_us. I > guess that this is not described good enough in the kernel docs. Mind adding a second patch to your series, clarifying Documentation/scheduler/sched-rt-group.rst? Thanks, Ingo
Hi! > > > Documentation/scheduller/sched-rt-group.rst [1] specifies this as values from -1 to > > > (INT_MAX - 1), I guess due int range. Looking into proc_dointvec_minmax() [2] > > > even INT_MAX would pass the check. I suppose we can do nothing about that, > > > because there is no value in sysctl_vals[] which would represent INT_MAX - 1. > > > > > > And you specify in LTP test range: from -1 to INT_MAX. > > > > > > But even much shorter value than INT_MAX fails: > > > > > > $ echo 1234567 > /proc/sys/kernel/sched_rt_runtime_us > > > sh: echo: write error: Invalid argument > > > > That is because runtime_us must be < period_us by definition, since > > runtime_us defines how much time is allocated from the period_us. I > > guess that this is not described good enough in the kernel docs. > > Mind adding a second patch to your series, clarifying > Documentation/scheduler/sched-rt-group.rst? Yes, that is the plan. I've been working on LTP release for past two weeks so this has been postponed. I will get to it hopefully next week. -- Cyril Hrubis chrubis@suse.cz
> Hi! > > Documentation/scheduller/sched-rt-group.rst [1] specifies this as values from -1 to > > (INT_MAX - 1), I guess due int range. Looking into proc_dointvec_minmax() [2] > > even INT_MAX would pass the check. I suppose we can do nothing about that, > > because there is no value in sysctl_vals[] which would represent INT_MAX - 1. > > And you specify in LTP test range: from -1 to INT_MAX. > > But even much shorter value than INT_MAX fails: > > $ echo 1234567 > /proc/sys/kernel/sched_rt_runtime_us > > sh: echo: write error: Invalid argument > That is because runtime_us must be < period_us by definition, since > runtime_us defines how much time is allocated from the period_us. I > guess that this is not described good enough in the kernel docs. Thanks for clarification. +1 for docs update. Kind regards, Petr
© 2016 - 2025 Red Hat, Inc.