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]
Message-ID: <20231026170150.GA2161924@cmpxchg.org>
Date:   Thu, 26 Oct 2023 13:01:50 -0400
From:   Johannes Weiner <hannes@...xchg.org>
To:     Suren Baghdasaryan <surenb@...gle.com>
Cc:     Luca Boccassi <bluca@...ian.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Domenico Cerasuolo <cerasuolodomenico@...il.com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] sched: psi: fix unprivileged polling against cgroups

On Thu, Oct 26, 2023 at 09:55:23AM -0700, Suren Baghdasaryan wrote:
> On Thu, Oct 26, 2023 at 9:49 AM Luca Boccassi <bluca@...ian.org> wrote:
> >
> > On Thu, 26 Oct 2023 at 17:41, Johannes Weiner <hannes@...xchg.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.
> 
> Ah, I see the problem. In our discussion
> https://lore.kernel.org/all/ZADj4YX4uftK%2FFrh@cmpxchg.org/ we decided
> to have the check in open() to fail early but we never considered
> unprivileged processes which only poll and never create any triggers.
> Makes sense.

Yeah, the two patches just ended up clashing. We made that open()
decision before unprivileged polling was merged, then ended up merging
it before the window patch.

Thanks!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ