From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AF805C433FE for ; Wed, 24 Nov 2021 17:33:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234872AbhKXRgZ (ORCPT ); Wed, 24 Nov 2021 12:36:25 -0500 Received: from mail.hallyn.com ([178.63.66.53]:40444 "EHLO mail.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233311AbhKXRgX (ORCPT ); Wed, 24 Nov 2021 12:36:23 -0500 Received: by mail.hallyn.com (Postfix, from userid 1001) id 12E4DD1E; Wed, 24 Nov 2021 11:33:11 -0600 (CST) Date: Wed, 24 Nov 2021 11:33:11 -0600 From: "Serge E. Hallyn" To: Greg Kroah-Hartman , serge@hallyn.com Cc: Jari Ruusu , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Alistair Delva , Khazhismel Kumykov , Bart Van Assche , Serge Hallyn , Jens Axboe , Paul Moore , selinux@vger.kernel.org, linux-security-module@vger.kernel.org, kernel-team@android.com Subject: Re: [PATCH 5.10 130/154] block: Check ADMIN before NICE for IOPRIO_CLASS_RT Message-ID: <20211124173310.GA12039@mail.hallyn.com> References: <20211124115702.361983534@linuxfoundation.org> <20211124115706.507376250@linuxfoundation.org> <619E4ABA.DC78AA58@users.sourceforge.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 24, 2021 at 04:31:22PM +0100, Greg Kroah-Hartman wrote: > On Wed, Nov 24, 2021 at 04:22:50PM +0200, Jari Ruusu wrote: > > Greg Kroah-Hartman wrote: > > > From: Alistair Delva > > > > > > commit 94c4b4fd25e6c3763941bdec3ad54f2204afa992 upstream. > > [SNIP] > > > --- a/block/ioprio.c > > > +++ b/block/ioprio.c > > > @@ -69,7 +69,14 @@ int ioprio_check_cap(int ioprio) > > > > > > switch (class) { > > > case IOPRIO_CLASS_RT: > > > - if (!capable(CAP_SYS_NICE) && !capable(CAP_SYS_ADMIN)) > > > + /* > > > + * Originally this only checked for CAP_SYS_ADMIN, > > > + * which was implicitly allowed for pid 0 by security > > > + * modules such as SELinux. Make sure we check > > > + * CAP_SYS_ADMIN first to avoid a denial/avc for > > > + * possibly missing CAP_SYS_NICE permission. > > > + */ > > > + if (!capable(CAP_SYS_ADMIN) && !capable(CAP_SYS_NICE)) > > > return -EPERM; > > > fallthrough; > > > /* rt has prio field too */ > > > > What exactly is above patch trying to fix? > > It does not change control flow at all, and added comment is misleading. > > See the thread on the mailing list for what it does and why it is > needed. > > It does change the result when selinux is enabled. > > thanks, > > greg k-h The case where we create a newer more fine grained capability which is a sub-cap of a broader capability like CAP_SYS_ADMIN is analogous. See check_syslog_permissions() for instance. So I think a helper like int capable_either_or(int cap1, int cap2) { if (has_capability_noaudit(current, cap1)) return 0; return capable(cap2); } might be worthwhile.