From owner-svn-src-head@FreeBSD.ORG Mon Jun 23 13:17:04 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id C11C144E; Mon, 23 Jun 2014 13:17:04 +0000 (UTC) Received: from mail-wi0-x231.google.com (mail-wi0-x231.google.com [IPv6:2a00:1450:400c:c05::231]) (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 D08D526B6; Mon, 23 Jun 2014 13:17:03 +0000 (UTC) Received: by mail-wi0-f177.google.com with SMTP id r20so4237168wiv.16 for ; Mon, 23 Jun 2014 06:16:58 -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=P0sQ58MLt9wWZyAzPzBqoE+wfvV472Y4WOCt2kjuYyk=; b=dug2gRw9XkBFnftPiUhmPCSRFOZEGtZrCQ/SHP6z6voov/nd6elQqj988IpKbf1sBu aUsKnlQJLdVyQrrWKYpPxJEYJIaXdaJLstQRU5eVLM7nSQwEbytohMnX5qV5E049ScT2 tGST23ENnZ5wzJtS433LLC4ADi2rm7m6E0gI/OlvzKTxEJETT4Un7GUo5W+evpD0ZVqE imbyxewMEL66kVs5n0LDERpzGrXCVEM3gbNcq2dvNLsBXZ2QhThrZOYugMr9ANqi7VSL ULG3wMuLYX8TG3yGtmRq8D5IEjiu8pd9rjBq564WH2sUvo4P6l8SH2l2Dxt/g5jGRQ80 ZsFw== X-Received: by 10.180.12.33 with SMTP id v1mr26478749wib.0.1403529418311; Mon, 23 Jun 2014 06:16:58 -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 f6sm33336476wiy.19.2014.06.23.06.16.57 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 23 Jun 2014 06:16:57 -0700 (PDT) Date: Mon, 23 Jun 2014 15:16:53 +0200 From: Mateusz Guzik To: Konstantin Belousov Subject: Re: svn commit: r267760 - head/sys/kern Message-ID: <20140623131653.GC27040@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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20140623081823.GG93733@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-head@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Jun 2014 13:17:04 -0000 On Mon, Jun 23, 2014 at 11:18:23AM +0300, Konstantin Belousov wrote: > On Mon, Jun 23, 2014 at 10:05:01AM +0200, Mateusz Guzik wrote: > > On Mon, Jun 23, 2014 at 10:25:19AM +0300, Konstantin Belousov wrote: > > > On Mon, Jun 23, 2014 at 09:06:52AM +0200, Mateusz Guzik wrote: > > > > The table is modified in these functions and is reachable from the rest > > > > of the kernel (can be found by e.g. sysctl_kern_proc_filedesc), thus > > > > XLOCK is needed to ensure consistency for readers. It can also be > > > > altered by mountcheckdirs, although not in a way which disrupts any of > > > > these functions. > > > I would think that such cases should be avoided by testing for P_INEXEC, > > > but I do not insist on this. > > > > > > > proc lock has to be dropped before filedesc lock is taken and I don't > > see any way to prevent the proc from transitioning to P_INEXEC afterwards. > > Since sysctl_kern_proc_filedesc et al can take a long time it does not > > seem feasible to pursue this. > > After a second look this problem has to be dealt with. 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. 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. 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. -- Mateusz Guzik