On Wed, Nov 24, 2021 at 07:15:35PM +0100, Greg Kroah-Hartman wrote:

On Wed, Nov 24, 2021 at 11:33:11AM -0600, Serge E. Hallyn wrote:

quoted

On Wed, Nov 24, 2021 at 04:31:22PM +0100, Greg Kroah-Hartman wrote:

quoted

On Wed, Nov 24, 2021 at 04:22:50PM +0200, Jari Ruusu wrote:

quoted

Greg Kroah-Hartman wrote:

quoted

From: Alistair Delva <redacted>

commit 94c4b4fd25e6c3763941bdec3ad54f2204afa992 upstream.

 [SNIP]

quoted

--- 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.

Sure, feel free to work on that and submit it, but for now, this change
is needed.

Sorry I misread the subject and thought this was just a resubmission.

FWIW I had acked an earlier version of this.

-serge