Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
MIME-Version: 1.0
References: <20231026164114.2488682-1-hannes@cmpxchg.org>
In-Reply-To: <20231026164114.2488682-1-hannes@cmpxchg.org>
From:   Luca Boccassi <bluca@debian.org>
Date:   Thu, 26 Oct 2023 17:49:36 +0100
Message-ID: <CAMw=ZnQ56cm4Txgy5EhGYvR+Jt4s-KVgoA9_65HKWVMOXp7a9A@mail.gmail.com>
Subject: Re: [PATCH] sched: psi: fix unprivileged polling against cgroups
To:     Johannes Weiner <hannes@cmpxchg.org>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Suren Baghdasaryan <surenb@google.com>,
        Domenico Cerasuolo <cerasuolodomenico@gmail.com>,
        linux-kernel@vger.kernel.org, stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 26 Oct 2023 at 17:41, Johannes Weiner <hannes@cmpxchg.org> 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@vger.kernel.org # 6.5+
> Reported-by: Luca Boccassi <bluca@debian.org>
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Acked-by: Luca Boccassi <bluca@debian.org>

Thank you very much for the quick fix - this was reported originally
on the systemd bug tracker by Daniel Black (I do not have an email
address):

https://github.com/systemd/systemd/issues/29723

It is very important for systemd services to be able to do this
without capabilities, as using capabilities means in turn user
namespaces cannot be used (PrivateUsers=yes in systemd parlance).

Kind regards,
Luca Boccassi