From owner-svn-src-all@FreeBSD.ORG Fri Jul 11 02:43:57 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 29DB6D1A; Fri, 11 Jul 2014 02:43:57 +0000 (UTC) Received: from mail-we0-x22a.google.com (mail-we0-x22a.google.com [IPv6:2a00:1450:400c:c03::22a]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 45AE82250; Fri, 11 Jul 2014 02:43:56 +0000 (UTC) Received: by mail-we0-f170.google.com with SMTP id w62so110024wes.29 for ; Thu, 10 Jul 2014 19:43:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=wGTKw3lFJW88BRy7Cx3wGXciPdtVaQXB+K3Q6ALAukc=; b=E/+MEgnufa0caQ71QPv8S2y3wiUCS8l+QCef+HNpLz5fhXfLJaEdxbjSha/qS9WCfk mCMIqkjEPcigWU/QDJMYAqWDmj5UMUZuZPLdx6ndXc5KqKybGdUJYB+86xc1sc0iq3bh uui8ZdZ8RB6ck46MMMABJZKANMHIy/sigkMvAOx4XrytnbRerjXw+rq5Sg0s3g2XhuS2 eY5vfrV/GYDnwt0mkAV/GtOLcTRZxTsDq/5gUlzW72vjb7gpMpyAoYMKL/WZA7ZWSUWl ABGAMz1h973IKVaQ8DQsZFhcilf3oo36WG4tQyflktpfZiX5/KfTOXambGdBqweqEO6m P9fg== X-Received: by 10.194.92.196 with SMTP id co4mr61889228wjb.4.1405046634269; Thu, 10 Jul 2014 19:43:54 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id lb3sm2090544wjc.30.2014.07.10.19.43.53 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 10 Jul 2014 19:43:53 -0700 (PDT) Date: Fri, 11 Jul 2014 04:43:51 +0200 From: Mateusz Guzik To: Konstantin Belousov Subject: Re: svn commit: r267760 - head/sys/kern Message-ID: <20140711024351.GA18214@dft-labs.eu> References: <201406230128.s5N1SIYK097224@svn.freebsd.org> <20140623064044.GD93733@kib.kiev.ua> <20140623070652.GA27040@dft-labs.eu> <20140623072519.GE93733@kib.kiev.ua> <20140623080501.GB27040@dft-labs.eu> <20140623081823.GG93733@kib.kiev.ua> <20140623131653.GC27040@dft-labs.eu> <20140623163523.GK93733@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140623163523.GK93733@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Jul 2014 02:43:57 -0000 On Mon, Jun 23, 2014 at 07:35:23PM +0300, Konstantin Belousov wrote: > On Mon, Jun 23, 2014 at 03:16:53PM +0200, Mateusz Guzik wrote: > > If traversal while transition to P_INEXEC is allowed, execve dealing > > with a setuid binary is problematic. This is more of hypothetical nature, > > but with sufficienly long delay it could finish the syscall and start > > opening some files, which paths would now be visible for an unprivileged > > reader. > > > > That said, I propose adding a counter to struct proc which would which > > would block execve. It would be quite similar to p_lock. > I thought about this too. In fact, I considered using PHOLD for this. > > > > > iow execve would: > > > > PROC_LOCK(p); > > p->p_flag |= P_INEXEC; > > while (p->p_execlock > 0) > > msleep(&p->p_execlock, &p->p_mtx, PWAIT, "execlock", 0); > > PROC_UNLOCK(p); > > > > And it would be mandatory for external fdp consumers to grab the counter. > > > > I'm tempted to add P_GETPIN which would both increase p_lock and p_execlock, > > that way the process is guaranteed not to exit and not to execve even > > after proc lock is dropped. > See above about PHOLD. > > > > > There is a separate question if p_execlock should be renamed and > > extended to also block any kind of credential changes. > > > > Then the guarantee is even stronger since we know that credentials we > > checked against are not going to change for the duration of our > > operations, but it is unclear if we need this. > > If doing separate execlock/p_lock, I think that it could be possible > to use per-process sx lock instead of hand-rolling the counter. The > accessors would lock sx shared, while kern_execve would take it in > exclusive mode. Both patches need some cleaning up. The name 'keeplock' is no exactly the best either. In both cases the same mechanism blocks both exec and exit, this can be split if needed (p_lock would still cover exit, p_something would cover exec). Here is a version with sx lock: http://people.freebsd.org/~mjg/patches/exec-exit-hold-wait.patch I'm not really happy with this. Reading foreign fdt is very rare and this adds lock + unlock for every exec and exit. On the other hand mere counter version is rather simple: http://people.freebsd.org/~mjg/patches/exec-exit-hold-nolock.patch I don't have strong opinion here, but prefer the latter. -- Mateusz Guzik