Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 28 Apr 2020 12:34:56 +0100
From:   Chris Down <chris@chrisdown.name>
To:     Suren Baghdasaryan <surenb@google.com>
Cc:     Johannes Weiner <hannes@cmpxchg.org>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: PSI poll() support for unprivileged users
Message-ID: <20200428113456.GA2170292@chrisdown.name>
References: <20200424153859.GA1481119@chrisdown.name>
 <CAJuCfpGnJBEvQTUeJ_U6+rHmPcMjw_pPL+QFj7Sec5fHZPH67w@mail.gmail.com>
 <CAJuCfpFhsN1=kDK0tU8aWeNt5Dj6U_1rC_dVT-27a4TL_hF0gA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <CAJuCfpFhsN1=kDK0tU8aWeNt5Dj6U_1rC_dVT-27a4TL_hF0gA@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hey Suren,

Suren Baghdasaryan writes:
>> > I'm building a userspace daemon for desktop users which notifies based on
>> > pressure events, and it's particularly janky to ask people to run such a
>> > notifier as root: the notification mechanism is usually tied to the user's
>> > display server auth, and the surrounding environment is generally pretty
>> > important to maintain. In addition to this, just in general this doesn't feel
>> > like the kind of feature that by its nature needs to be restricted to root --
>> > it seems reasonable that there would be unprivileged users which want to use
>> > this, and that not using RT threads would be acceptable in that scenario.
>>
>> For these cases you can provide a userspace privileged daemon that
>> will relay pressure notifications to its unprivileged clients. This is
>> what we do on Android - Android Management Server registers its PSI
>> triggers and then relays low memory notifications to unprivileged
>> apps.
>> Another approach is taken by Android Low Memory Killer Daemon (lmkd)
>> which is an unprivileged process but registers its PSI triggers. The
>> trick is that the init process executes "chmod 0664
>> /proc/pressure/memory" from its init script and further restrictions
>> are enforced by selinux policy granting only LMKD write access to this
>> file.
>>
>> Would any of these options work for you?

Hmm, I think these are reasonable options when you have control over the 
system, but not so great if you don't. For example, I want to get pressure 
notifications for my logind seat, but that doesn't necessarily imply that I 
have administrative access to the machine.

>> > Have you considered making the per-cgroup RT threads optional? If the
>> > processing isn't done in the FIFO kthread for unprivileged users, I think it
>> > should be safe to allow them to write to pressure files (perhaps with some
>> > additional limits or restrictions on things like the interval, as needed).
>>
>> I didn't consider that as I viewed memory condition tracking that
>> consumes kernel resources as being potentially exploitable. RT threads
>> did make that more of an issue but even without them I'm not sure we
>> should allow unprivileged processes to create unlimited numbers of
>> triggers each of which is not really free.

There's precedent for other similar issues like this in the kernel, eg. rates 
for some ICMP packets, where we enforce a static limit in the kernel for 
unprivileged users. I'd imagine we can do something similar here, too.

>Thinking some more about this. LMKD in the above-mentioned usecase is
>not a privileged process but it is granted access to PSI triggers by a
>privileged init process+sepolicy and it needs RT threads to react to
>memory pressure promptly without being preempted. If we allow only the
>privileged users to have RT threads for PSI triggers then that
>requirement would break this scenario and LMKD won't be able to use RT
>threads.

Well, fiddlesticks :-)

If we needed to have both, I don't know what the interface would look like, but 
yes, it sounds overcomplicated. I'll think about it some more.

Thanks,

Chris