linux-kernel - Re: [PATCH v4] sched/rt: fix the case where sched_rt_period

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: <b4d4313342a10afc7cda2574d37fdb38@linux.dev>
Date:   Tue, 07 Jun 2022 01:25:48 +0000
From:   "Yajun Deng" <yajun.deng@...ux.dev>
To:     "Valentin Schneider" <vschneid@...hat.com>, mingo@...hat.com,
        peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] sched/rt: fix the case where sched_rt_period_us is
 negative

Ping.



May 18, 2022 12:08 AM, "Valentin Schneider" <vschneid@...hat.com> wrote:

> On 17/05/22 14:29, Yajun Deng wrote:
> 
>> The proc_dointvec() is for integer, but sysctl_sched_rt_period is a
>> unsigned integer, proc_dointvec() would convert negative number into
>> positive number. So both proc_dointvec() and sched_rt_global_validate()
>> aren't return error even if we set a negative number.
>> 
>> Use proc_dointvec_minmax() instead of proc_dointvec() and use extra1
>> limit the minimum value for sched_rt_period_us/sched_rt_runtime_us.
>> 
>> Make sysctl_sched_rt_period integer to match proc_dointvec_minmax().
> 
> How about:
> 
> While sysctl_sched_rt_runtime is a signed integer and
> sysctl_sched_rt_period is unsigned, both are handled in sched_rt_handler()
> via proc_dointvec(), so negative inputs can be fed into
> sysctl_sched_rt_period. However, per sched-rt-group.rst:
> 
> * sched_rt_period_us takes values from 1 to INT_MAX.
> * sched_rt_runtime_us takes values from -1 to (INT_MAX - 1).
> 
> Use proc_dointvec_minmax() instead of proc_dointvec() and use the .extra1
> parameter to enforce a minimum value.
> 
> Make sysctl_sched_rt_period a signed integer as this matches the expected
> upper boundary and the expected type of proc_dointvec_minmax().
> 
>> v4:
>> - Make sysctl_sched_rt_period integer (Valentin Schneider)
> 
> Even if v3 was bogus, it's good not to skip it in the version log.
> Also, the version logs should be after the "---" marker line:
> 
> Documentation/process/submitting-patches.rt
> """
> Please put this information **after** the ``---`` line which separates
> the changelog from the rest of the patch. The version information is
> not part of the changelog which gets committed to the git tree. It is
> additional information for the reviewers. If it's placed above the
> commit tags, it needs manual interaction to remove it. If it is below
> the separator line, it gets automatically stripped off when applying the
> patch
> """
> 
>> v2:
>> - Remove sched_rr_timeslice_ms related changes (Valentin Schneider)
>> 
>> Fixes: d0b27fa77854 ("sched: rt-group: synchonised bandwidth period")
>> Signed-off-by: Yajun Deng <yajun.deng@...ux.dev>
> 
> Reviewed-by: Valentin Schneider <vschneid@...hat.com>