Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752800AbdGSDWH (ORCPT ); Tue, 18 Jul 2017 23:22:07 -0400 Received: from h2.hallyn.com ([78.46.35.8]:55820 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751969AbdGSDWF (ORCPT ); Tue, 18 Jul 2017 23:22:05 -0400 Date: Tue, 18 Jul 2017 22:22:05 -0500 From: "Serge E. Hallyn" To: Kees Cook Cc: Andrew Morton , David Howells , "Eric W. Biederman" , John Johansen , "Serge E. Hallyn" , Paul Moore , Stephen Smalley , Casey Schaufler , Tetsuo Handa , James Morris , Andy Lutomirski , Linus Torvalds , linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec Message-ID: <20170719032205.GA23598@mail.hallyn.com> References: <1500416736-49829-1-git-send-email-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1500416736-49829-1-git-send-email-keescook@chromium.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2959 Lines: 65 On Tue, Jul 18, 2017 at 03:25:21PM -0700, Kees Cook wrote: > This series has grown... :P > > As discussed with Linus and Andy, we need to reset the stack rlimit > before we do memory layouts when execing a privilege-gaining (e.g. > setuid) program. To do this, we need to know the results of the > bprm_secureexec hook before memory layouts. As it turns out, this > can be made _mostly_ trivial by collapsing bprm_secureexec into > bprm_set_creds. > > The LSMs using bprm_secureexec nearly always save state between > bprm_set_creds and bprm_secureexec. In the face of multiple calls to > bprm_set_creds (via prepare_binprm() calls from binfmt_script, etc), > all LSMs except commoncap only pay attention to the first call, so > that aligns well with collapsing bprm_secureexec into bprm_set_creds. > The commoncaps, though, needs to check the _last_ bprm_set_creds, so > this series just swaps one bprm flag for another (cap_effective is no > longer needed to save state between bprm_set_creds and bprm_secureexec, > but we do need to keep a separate state, so we add the cap_elevated flag). > > Once secureexec is available to setup_new_exec() before the memory > layout, we can add an rlimit sanity-check for setuid execs. (With no > need to clean up since we're past the point of no return.) > > Along the way, this fixes comments, renames a variable, and consolidates > dumpability and pdeath_signal clearing, which includes some commit log > archeology to examine the subtle differences between what we had and > what we need. > > I'd appreciate some extra eyes on this to make sure this isn't broken > in some special way. Looking at the diffstat, even after all my long > comments, this is a net reduction in lines. :) > > Given this crosses a bunch of areas, I think this is likely best to > go via the -mm tree, which is where nearly all of my prior exec work > has lived too. > > Thanks! > > -Kees > ---------------------------------------------------------------- > Kees Cook (15): > binfmt: Introduce secureexec flag > exec: Rename bprm->cred_prepared to called_set_creds > apparmor: Refactor to remove bprm_secureexec hook > selinux: Refactor to remove bprm_secureexec hook > smack: Refactor to remove bprm_secureexec hook > commoncap: Refactor to remove bprm_secureexec hook > commoncap: Move cap_elevated calculation into bprm_set_creds > LSM: drop bprm_secureexec hook > exec: Correct comments about "point of no return" > exec: Use secureexec for setting dumpability > exec: Use secureexec for clearing pdeath_signal > smack: Remove redundant pdeath_signal clearing > exec: Consolidate dumpability logic > exec: Use sane stack rlimit under secureexec > exec: Consolidate pdeath_signal clearing Thanks, the set looks good to me, Acked-by: Serge Hallyn Have you had a chance to run the ltp caps tests against this? -serge