linux-kernel - Re: [PATCH] sched/core: Fix wrong burst unit in cgroup2 cpu.max write

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: <xm267dfonouc.fsf@google.com>
Date:   Fri, 10 Sep 2021 13:43:39 -0700
From:   Benjamin Segall <bsegall@...gle.com>
To:     Kailun Qin <kailun.qin@...el.com>
Cc:     tj@...nel.org, linux-kernel@...r.kernel.org,
        cgroups@...r.kernel.org, changhuaixin@...ux.alibaba.com,
        mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org
Subject: Re: [PATCH] sched/core: Fix wrong burst unit in cgroup2 cpu.max write

Kailun Qin <kailun.qin@...el.com> writes:

> In cpu_max_write(), as the eventual tg_set_cfs_bandwidth() operates on
> the burst in nsec which is input from tg_get_cfs_burst() in usec, it
> should be converted into nsec accordingly.
>
> If not, this may cause a write into cgroup2 cpu.max to unexpectedly
> change an already set cpu.max.burst.
>
> This patch addresses the above issue.
>
> Signed-off-by: Kailun Qin <kailun.qin@...el.com>

Oh, huh, cpu_period_quota_parse is confusing and changes the units in
period. (It might make more sense to make all that interaction clearer
somehow, but this is probably fine)

It's probably also better to just use tg->cfs_bandwidth.burst directly
rather than do an unnecessary div+multiply.

For whichever version:

Reviewed-by: Ben Segall <bsegall@...gle.com>

> ---
>  kernel/sched/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index c4462c454ab9..fc9fcc56149f 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -10711,7 +10711,7 @@ static ssize_t cpu_max_write(struct kernfs_open_file *of,
>  {
>  	struct task_group *tg = css_tg(of_css(of));
>  	u64 period = tg_get_cfs_period(tg);
> -	u64 burst = tg_get_cfs_burst(tg);
> +	u64 burst = (u64)tg_get_cfs_burst(tg) * NSEC_PER_USEC;
>  	u64 quota;
>  	int ret;