linux-kernel - Re: [PATCH] sched/uclamp: Avoid setting cpu.uclamp.min bigger than cpu.uclamp.max

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YLeF/556Wbvx1Ssc@google.com>
Date:   Wed, 2 Jun 2021 13:22:07 +0000
From:   Quentin Perret <qperret@...gle.com>
To:     Xuewen Yan <xuewen.yan94@...il.com>
Cc:     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, linux-kernel@...r.kernel.org,
        zhang.lyra@...il.com, xuewyan@...mail.com,
        patrick.bellasi@...bug.net, tj@...nel.org
Subject: Re: [PATCH] sched/uclamp: Avoid setting cpu.uclamp.min bigger than
 cpu.uclamp.max

+CC Patrick and Tejun

On Wednesday 02 Jun 2021 at 20:38:03 (+0800), Xuewen Yan wrote:
> From: Xuewen Yan <xuewen.yan@...soc.com>
> 
> When setting cpu.uclamp.min/max in cgroup, there is no validating
> like uclamp_validate() in __sched_setscheduler(). It may cause the
> cpu.uclamp.min is bigger than cpu.uclamp.max.

ISTR this was intentional. We also allow child groups to ask for
whatever clamps they want, but that is always limited by the parent, and
reflected in the 'effective' values, as per the cgroup delegation model.

> Although there is protection in cpu_util_update_eff():
> “eff[UCLAMP_MIN] = min(eff[UCLAMP_MIN], eff[UCLAMP_MAX])”, it's better
> not to let it happen.
> 
> Judging the uclamp value before setting uclamp_min/max, avoid
> the cpu.uclamp.min is bigger than cpu.uclamp.max.
> 
> Signed-off-by: Xuewen Yan <xuewen.yan@...soc.com>
> ---
>  kernel/sched/core.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 5226cc26a095..520a2da40dc9 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8867,6 +8867,30 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
>  	rcu_read_lock();
>  
>  	tg = css_tg(of_css(of));
> +
> +	switch (clamp_id) {
> +	case UCLAMP_MIN: {
> +		unsigned int uc_req_max = tg->uclamp_req[UCLAMP_MAX].value;
> +
> +		if (req.util > uc_req_max) {
> +			nbytes = -EINVAL;
> +			goto unlock;
> +		}
> +		break;
> +	}
> +	case UCLAMP_MAX: {
> +		unsigned int uc_req_min = tg->uclamp_req[UCLAMP_MIN].value;
> +
> +		if (req.util < uc_req_min) {
> +			nbytes = -EINVAL;
> +			goto unlock;
> +		}
> +		break;
> +	}
> +	default:
> +		nbytes = -EINVAL;
> +		goto unlock;
> +	}
>  	if (tg->uclamp_req[clamp_id].value != req.util)
>  		uclamp_se_set(&tg->uclamp_req[clamp_id], req.util, false);
>  
> @@ -8878,7 +8902,7 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
>  
>  	/* Update effective clamps to track the most restrictive value */
>  	cpu_util_update_eff(of_css(of));
> -
> +unlock:
>  	rcu_read_unlock();
>  	mutex_unlock(&uclamp_mutex);
>  
> -- 
> 2.25.1
>