Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753211AbdGSBw4 (ORCPT ); Tue, 18 Jul 2017 21:52:56 -0400 Received: from mail.kernel.org ([198.145.29.99]:35712 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752500AbdGSBwy (ORCPT ); Tue, 18 Jul 2017 21:52:54 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5DB5922C91 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=luto@kernel.org MIME-Version: 1.0 In-Reply-To: <1500416736-49829-8-git-send-email-keescook@chromium.org> References: <1500416736-49829-1-git-send-email-keescook@chromium.org> <1500416736-49829-8-git-send-email-keescook@chromium.org> From: Andy Lutomirski Date: Tue, 18 Jul 2017 18:52:31 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds To: Kees Cook Cc: Andrew Morton , Serge Hallyn , Andy Lutomirski , David Howells , "Eric W. Biederman" , John Johansen , Paul Moore , Stephen Smalley , Casey Schaufler , Tetsuo Handa , James Morris , Linus Torvalds , Linux FS Devel , LSM List , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1252 Lines: 32 On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook wrote: > Instead of a separate function, open-code the cap_elevated test, which > lets us entirely remove bprm->cap_effective (to use the local "effective" > variable instead), and more accurately examine euid/egid changes via the > existing local "is_setid". ... > -static int is_secureexec(struct linux_binprm *bprm) > -{ > - const struct cred *cred = bprm->cred; > - kuid_t root_uid = make_kuid(cred->user_ns, 0); > - > - if (!uid_eq(cred->uid, root_uid)) { > - if (bprm->cap_effective) > - return 1; > - if (!cap_issubset(cred->cap_permitted, cred->cap_ambient)) > - return 1; > + bprm->cap_elevated = 0; > + if (is_setid) { > + bprm->cap_elevated = 1; > + } else if (!uid_eq(new->uid, root_uid)) { > + if (effective || > + !cap_issubset(new->cap_permitted, new->cap_ambient)) > + bprm->cap_elevated = 1; > } > > - return (!uid_eq(cred->euid, cred->uid) || > - !gid_eq(cred->egid, cred->gid)); > + return 0; I think this matches the old behavior. IOW it looks right.