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;
Subject: Re: [PATCH] psi: allow unprivileged users with CAP_SYS_RESOURCE to
 write psi files
To:     "Eric W. Biederman" <ebiederm@xmission.com>,
        Kees Cook <keescook@chromium.org>
Cc:     Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org
References: <20210401033156.7262-1-johunt@akamai.com>
 <m15z16r583.fsf@fess.ebiederm.org> <202104010039.A134EC56@keescook>
 <m1v995q4l1.fsf@fess.ebiederm.org>
From:   Josh Hunt <johunt@akamai.com>
Message-ID: <0fd5d9b0-4c9e-2dfe-a8fe-1d1def8a6122@akamai.com>
Date:   Thu, 1 Apr 2021 11:10:09 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <m1v995q4l1.fsf@fess.ebiederm.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 4/1/21 10:47 AM, Eric W. Biederman wrote:
> Kees Cook <keescook@chromium.org> writes:
> 
>> On Wed, Mar 31, 2021 at 11:36:28PM -0500, Eric W. Biederman wrote:
>>> Josh Hunt <johunt@akamai.com> writes:
>>>
>>>> Currently only root can write files under /proc/pressure. Relax this to
>>>> allow tasks running as unprivileged users with CAP_SYS_RESOURCE to be
>>>> able to write to these files.
>>>
>>> The test for CAP_SYS_RESOURCE really needs to be in open rather
>>> than in write.
>>>
>>> Otherwise a suid root executable could have stdout redirected
>>> into these files.
>>
>> Right. Or check against f_cred. (See uses of kallsyms_show_value())
>> https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/security/credentials.html*open-file-credentials__;Iw!!GjvTz_vk!B_aeVyHMG20VNUGx001EFKpeYlahLQHye7oS8sokXuZOhVDTtF_deDl71a_KYA$
> 
> We really want to limit checking against f_cred to those cases where we
> break userspace by checking in open.  AKA the cases where we made the
> mistake of putting the permission check in the wrong place and now can't
> fix it.
> 
> Since this change is change the permissions that open uses already I
> don't see any reason we can't perform a proper check in open.
> 
> Eric
> 

Thank you for the feedback. I will spin a v2 doing the check in open.

Josh