linux-kernel - Re: [PATCH] sched: psi: fix unprivileged polling against cgroups

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 31 Oct 2023 21:05:26 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Johannes Weiner <hannes@...xchg.org>, Tejun Heo <tj@...nel.org>
Cc:     Suren Baghdasaryan <surenb@...gle.com>,
        Domenico Cerasuolo <cerasuolodomenico@...il.com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Luca Boccassi <bluca@...ian.org>
Subject: Re: [PATCH] sched: psi: fix unprivileged polling against cgroups


+cc tj because cgroup

On Thu, Oct 26, 2023 at 12:41:14PM -0400, Johannes Weiner wrote:
> 519fabc7aaba ("psi: remove 500ms min window size limitation for
> triggers") breaks unprivileged psi polling on cgroups.
> 
> Historically, we had a privilege check for polling in the open() of a
> pressure file in /proc, but were erroneously missing it for the open()
> of cgroup pressure files.
> 
> When unprivileged polling was introduced in d82caa273565 ("sched/psi:
> Allow unprivileged polling of N*2s period"), it needed to filter
> privileges depending on the exact polling parameters, and as such
> moved the CAP_SYS_RESOURCE check from the proc open() callback to
> psi_trigger_create(). Both the proc files as well as cgroup files go
> through this during write(). This implicitly added the missing check
> for privileges required for HT polling for cgroups.
> 
> When 519fabc7aaba ("psi: remove 500ms min window size limitation for
> triggers") followed right after to remove further restrictions on the
> RT polling window, it incorrectly assumed the cgroup privilege check
> was still missing and added it to the cgroup open(), mirroring what we
> used to do for proc files in the past.
> 
> As a result, unprivileged poll requests that would be supported now
> get rejected when opening the cgroup pressure file for writing.
> 
> Remove the cgroup open() check. psi_trigger_create() handles it.
> 
> Fixes: 519fabc7aaba ("psi: remove 500ms min window size limitation for triggers")
> Cc: stable@...r.kernel.org # 6.5+
> Reported-by: Luca Boccassi <bluca@...ian.org>
> Signed-off-by: Johannes Weiner <hannes@...xchg.org>

Since merge window is upon is, I've queued this with the intent to stick
into sched/urgent after rc1.

> ---
>  kernel/cgroup/cgroup.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index f11488b18ceb..2069ee98da60 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -3879,14 +3879,6 @@ static __poll_t cgroup_pressure_poll(struct kernfs_open_file *of,
>  	return psi_trigger_poll(&ctx->psi.trigger, of->file, pt);
>  }
>  
> -static int cgroup_pressure_open(struct kernfs_open_file *of)
> -{
> -	if (of->file->f_mode & FMODE_WRITE && !capable(CAP_SYS_RESOURCE))
> -		return -EPERM;
> -
> -	return 0;
> -}
> -
>  static void cgroup_pressure_release(struct kernfs_open_file *of)
>  {
>  	struct cgroup_file_ctx *ctx = of->priv;
> @@ -5287,7 +5279,6 @@ static struct cftype cgroup_psi_files[] = {
>  	{
>  		.name = "io.pressure",
>  		.file_offset = offsetof(struct cgroup, psi_files[PSI_IO]),
> -		.open = cgroup_pressure_open,
>  		.seq_show = cgroup_io_pressure_show,
>  		.write = cgroup_io_pressure_write,
>  		.poll = cgroup_pressure_poll,
> @@ -5296,7 +5287,6 @@ static struct cftype cgroup_psi_files[] = {
>  	{
>  		.name = "memory.pressure",
>  		.file_offset = offsetof(struct cgroup, psi_files[PSI_MEM]),
> -		.open = cgroup_pressure_open,
>  		.seq_show = cgroup_memory_pressure_show,
>  		.write = cgroup_memory_pressure_write,
>  		.poll = cgroup_pressure_poll,
> @@ -5305,7 +5295,6 @@ static struct cftype cgroup_psi_files[] = {
>  	{
>  		.name = "cpu.pressure",
>  		.file_offset = offsetof(struct cgroup, psi_files[PSI_CPU]),
> -		.open = cgroup_pressure_open,
>  		.seq_show = cgroup_cpu_pressure_show,
>  		.write = cgroup_cpu_pressure_write,
>  		.poll = cgroup_pressure_poll,
> @@ -5315,7 +5304,6 @@ static struct cftype cgroup_psi_files[] = {
>  	{
>  		.name = "irq.pressure",
>  		.file_offset = offsetof(struct cgroup, psi_files[PSI_IRQ]),
> -		.open = cgroup_pressure_open,
>  		.seq_show = cgroup_irq_pressure_show,
>  		.write = cgroup_irq_pressure_write,
>  		.poll = cgroup_pressure_poll,
> -- 
> 2.42.0
>