From patchwork Fri Feb 17 16:21:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ondrej Mosnacek X-Patchwork-Id: 58704 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp981529wrn; Fri, 17 Feb 2023 08:29:31 -0800 (PST) X-Google-Smtp-Source: AK7set8/CSi7AAepUyOeFDglwFixybWq7uoC71WBcZdvUtZpZRYnCQmAOj5d0poOprNKm/HSLD7r X-Received: by 2002:a05:6402:14:b0:4aa:a4f1:3edb with SMTP id d20-20020a056402001400b004aaa4f13edbmr1251201edu.29.1676651370851; Fri, 17 Feb 2023 08:29:30 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1676651370; cv=none; d=google.com; s=arc-20160816; b=YAF2al6CbGejVozZsg7xgQ79Pue7Rn3/d/rKp2WqsPbvPxeiDRXSMfb1OqGSQQQYKZ Vaf0z/P9sCVdMWOOyWm1TH8szcI6i/nehVU/8wcPj8aK01Wqe4Y8WigVykyqmXUDJLrB AuEYyoifN6WGP2ekVLeztslPfHJs6T0q+SCYad1rdHgEWCwp9SSFW8B/C5Nl/2Q13cZO aQV+6WiQhVhYUr0jrbYu9rbitjekSEiZ9TQ7WcBO1xasep7+YnNdN0Xb/3AqDPYtuTmk /A86lS+gmKnDaG5ky2KYfn50nZ6hbg7/SrvJRN71bPLgxQ4OBxnXjLpUejZDXXkcDPDb zfrg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=FVuEhhYScOgrQqnypKq2VFgUAU8z41nhpw8bLy+N1Yw=; b=fz9ZhsifxpQBvWRHz0cTc9hI0Peimgba3vnPoK+PKby9uaQY6iv2s+8A39ONYzhsxn 1sUfW2fSYhkszoQQb3sacbbGtTersU0q5yBgGef14k+p87Rf9mIQlbcDuKmD5zLtdZY0 wUk0IsYjGS2A39H9D5EtkgcwNe0XzvNEcPqixgxlDFXe1Cxy7OH+lRcs47sp7AaB3zGS c1Pvz4xTupC7H3aiTuzRagZcXBdUxjt8I65sNQ+Id8pHxpCZICI8PbkoXoLy40Am8/DH KRhaedTd1aEvWAasivgL3e9odKfEFzSDfqMuk58mNYTt2onZjh75+S8VFNXsoueV/uWz mFoQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Sq1ijBG+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id f21-20020a056402069500b004acbe796d63si5428586edy.463.2023.02.17.08.29.08; Fri, 17 Feb 2023 08:29:30 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Sq1ijBG+; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229885AbjBQQYs (ORCPT + 99 others); Fri, 17 Feb 2023 11:24:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59456 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229793AbjBQQYo (ORCPT ); Fri, 17 Feb 2023 11:24:44 -0500 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 669D55FC66 for ; Fri, 17 Feb 2023 08:23:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676650921; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=FVuEhhYScOgrQqnypKq2VFgUAU8z41nhpw8bLy+N1Yw=; b=Sq1ijBG+h2InL9r41VlVfBav/nez7RuRzj4LEOg1XRUIxFgIHsojOj6WHj5go9OMQArb0s AdCV3RWL/eyD0hVbcZ0nHHnaHVSpH8R5TKPoGPJpRL8XkFubmMc/F3PrFUzMsv99SUWtRH oX9JHdccwYa3pGV0AAviliYqKVlQ+wc= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-557-Cysw7ch9Msmj8kLbW92ryw-1; Fri, 17 Feb 2023 11:21:59 -0500 X-MC-Unique: Cysw7ch9Msmj8kLbW92ryw-1 Received: by mail-ed1-f71.google.com with SMTP id cy15-20020a0564021c8f00b004aaa054d189so2166441edb.11 for ; Fri, 17 Feb 2023 08:21:59 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=FVuEhhYScOgrQqnypKq2VFgUAU8z41nhpw8bLy+N1Yw=; b=kEw828Yl6l7/gqHDnzokvHNvuCOEPP1jVbGWESUR463mQvUXFrCtoM2b2J7D9L4yCi JyPbRmzoDIDNppFbFlqsAygk1w9+fgOqaUNx4O/qzYIvACLlRJnaquz/IDaF6+XZlFkW 5ZD2ldqyucSvm7hJ52BNxZAFpn43MmDUkY8Ujf3GHFwv65l79CyBdFoxg+xmDl+zMX53 ae/Nob/JUHQXsoPZGflcUd80LZckT961Npt4XbDx19BnqVDJ0uQRKTBG1HnxwEqi0OdM XDdu2i4IgjyIFcYMemP/S+yWMI4gq4Gh/vfpScBFKKXCGe+YV8H9l9ey0BBd54l5BTEv xcjw== X-Gm-Message-State: AO0yUKV3JOgZ03XqRtUEqOg+d2wngfkw/95PxiAnng1aEZ5C8Y3Z07WG I1+F1x9xfKzvBcRy4yLK+y3U16ezfvtq8Gh2w9uI/UeJo8j2OnqHmgt9FLyGM9EVaSVYfYSmvUf 0waS0W3R9V/MFQuBG95cj5H5I X-Received: by 2002:a17:907:a04f:b0:88b:23bb:e61f with SMTP id gz15-20020a170907a04f00b0088b23bbe61fmr600698ejc.25.1676650918323; Fri, 17 Feb 2023 08:21:58 -0800 (PST) X-Received: by 2002:a17:907:a04f:b0:88b:23bb:e61f with SMTP id gz15-20020a170907a04f00b0088b23bbe61fmr600677ejc.25.1676650918056; Fri, 17 Feb 2023 08:21:58 -0800 (PST) Received: from localhost.localdomain ([2a02:8308:b104:2c00:2e8:ec99:5760:fb52]) by smtp.gmail.com with ESMTPSA id g23-20020a170906539700b008b128106fc7sm2293812ejo.46.2023.02.17.08.21.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Feb 2023 08:21:57 -0800 (PST) From: Ondrej Mosnacek To: "Eric W. Biederman" , Andrew Morton Cc: linux-security-module@vger.kernel.org, selinux@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] kernel/sys.c: fix and improve control flow in __sys_setres[ug]id() Date: Fri, 17 Feb 2023 17:21:54 +0100 Message-Id: <20230217162154.837549-1-omosnace@redhat.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757903971921177003?= X-GMAIL-MSGID: =?utf-8?q?1758096387515487383?= Linux Security Modules (LSMs) that implement the "capable" hook will usually emit an access denial message to the audit log whenever they "block" the current task from using the given capability based on their security policy. The occurrence of a denial is used as an indication that the given task has attempted an operation that requires the given access permission, so the callers of functions that perform LSM permission checks must take care to avoid calling them too early (before it is decided if the permission is actually needed to perform the requested operation). The __sys_setres[ug]id() functions violate this convention by first calling ns_capable_setid() and only then checking if the operation requires the capability or not. It means that any caller that has the capability granted by DAC (task's capability set) but not by MAC (LSMs) will generate a "denied" audit record, even if is doing an operation for which the capability is not required. Fix this by reordering the checks such that ns_capable_setid() is checked last and -EPERM is returned immediately if it returns false. While there, also do two small optimizations: * move the capability check before prepare_creds() and * bail out early in case of a no-op. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Ondrej Mosnacek --- v2: improve commit message kernel/sys.c | 69 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/kernel/sys.c b/kernel/sys.c index 5fd54bf0e8867..6fd88686cd06f 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -664,6 +664,7 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid) struct cred *new; int retval; kuid_t kruid, keuid, ksuid; + bool ruid_new, euid_new, suid_new; kruid = make_kuid(ns, ruid); keuid = make_kuid(ns, euid); @@ -678,25 +679,29 @@ long __sys_setresuid(uid_t ruid, uid_t euid, uid_t suid) if ((suid != (uid_t) -1) && !uid_valid(ksuid)) return -EINVAL; + old = current_cred(); + + /* check for no-op */ + if ((ruid == (uid_t) -1 || uid_eq(kruid, old->uid)) && + (euid == (uid_t) -1 || (uid_eq(keuid, old->euid) && + uid_eq(keuid, old->fsuid))) && + (suid == (uid_t) -1 || uid_eq(ksuid, old->suid))) + return 0; + + ruid_new = ruid != (uid_t) -1 && !uid_eq(kruid, old->uid) && + !uid_eq(kruid, old->euid) && !uid_eq(kruid, old->suid); + euid_new = euid != (uid_t) -1 && !uid_eq(keuid, old->uid) && + !uid_eq(keuid, old->euid) && !uid_eq(keuid, old->suid); + suid_new = suid != (uid_t) -1 && !uid_eq(ksuid, old->uid) && + !uid_eq(ksuid, old->euid) && !uid_eq(ksuid, old->suid); + if ((ruid_new || euid_new || suid_new) && + !ns_capable_setid(old->user_ns, CAP_SETUID)) + return -EPERM; + new = prepare_creds(); if (!new) return -ENOMEM; - old = current_cred(); - - retval = -EPERM; - if (!ns_capable_setid(old->user_ns, CAP_SETUID)) { - if (ruid != (uid_t) -1 && !uid_eq(kruid, old->uid) && - !uid_eq(kruid, old->euid) && !uid_eq(kruid, old->suid)) - goto error; - if (euid != (uid_t) -1 && !uid_eq(keuid, old->uid) && - !uid_eq(keuid, old->euid) && !uid_eq(keuid, old->suid)) - goto error; - if (suid != (uid_t) -1 && !uid_eq(ksuid, old->uid) && - !uid_eq(ksuid, old->euid) && !uid_eq(ksuid, old->suid)) - goto error; - } - if (ruid != (uid_t) -1) { new->uid = kruid; if (!uid_eq(kruid, old->uid)) { @@ -761,6 +766,7 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) struct cred *new; int retval; kgid_t krgid, kegid, ksgid; + bool rgid_new, egid_new, sgid_new; krgid = make_kgid(ns, rgid); kegid = make_kgid(ns, egid); @@ -773,23 +779,28 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid) if ((sgid != (gid_t) -1) && !gid_valid(ksgid)) return -EINVAL; + old = current_cred(); + + /* check for no-op */ + if ((rgid == (gid_t) -1 || gid_eq(krgid, old->gid)) && + (egid == (gid_t) -1 || (gid_eq(kegid, old->egid) && + gid_eq(kegid, old->fsgid))) && + (sgid == (gid_t) -1 || gid_eq(ksgid, old->sgid))) + return 0; + + rgid_new = rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) && + !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid); + egid_new = egid != (gid_t) -1 && !gid_eq(kegid, old->gid) && + !gid_eq(kegid, old->egid) && !gid_eq(kegid, old->sgid); + sgid_new = sgid != (gid_t) -1 && !gid_eq(ksgid, old->gid) && + !gid_eq(ksgid, old->egid) && !gid_eq(ksgid, old->sgid); + if ((rgid_new || egid_new || sgid_new) && + !ns_capable_setid(old->user_ns, CAP_SETGID)) + return -EPERM; + new = prepare_creds(); if (!new) return -ENOMEM; - old = current_cred(); - - retval = -EPERM; - if (!ns_capable_setid(old->user_ns, CAP_SETGID)) { - if (rgid != (gid_t) -1 && !gid_eq(krgid, old->gid) && - !gid_eq(krgid, old->egid) && !gid_eq(krgid, old->sgid)) - goto error; - if (egid != (gid_t) -1 && !gid_eq(kegid, old->gid) && - !gid_eq(kegid, old->egid) && !gid_eq(kegid, old->sgid)) - goto error; - if (sgid != (gid_t) -1 && !gid_eq(ksgid, old->gid) && - !gid_eq(ksgid, old->egid) && !gid_eq(ksgid, old->sgid)) - goto error; - } if (rgid != (gid_t) -1) new->gid = krgid;