From owner-freebsd-hackers@FreeBSD.ORG Fri Nov 1 20:27:37 2013 Return-Path: Delivered-To: hackers@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 ESMTP id C1DDC918 for ; Fri, 1 Nov 2013 20:27:37 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wi0-x234.google.com (mail-wi0-x234.google.com [IPv6:2a00:1450:400c:c05::234]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 5D8CC265E for ; Fri, 1 Nov 2013 20:27:37 +0000 (UTC) Received: by mail-wi0-f180.google.com with SMTP id ey11so1600444wid.1 for ; Fri, 01 Nov 2013 13:27:35 -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=GgcrQVBYIJvq2NNOPcEf30V8b9DTBgQ70jeSfUcVQ+Y=; b=NGW4tP556roz07vkkz6HkcOzHquh3jrTSRNJQRAYkVMuDt8r9Z0uPokL75piwq/dFw UjV7JF9AVPnst7x0l8QZzozJkc8x4FXwOIt/Xg1tb29ICJJirX64ZOjYwkP9EOkbhG+p NwTOghQMtpw6ITOEV/hxb1TAl4OI/XMTrAiA0Xq3xOiQGMevDXS5oFK9PSPRTi683Hou 8g72XkoAISmm++CCRN0c6IRPBhs8Hu6SDATo6wJUiTIgsc8Ov34Dj4PJARxwfFsNnlcn HZYmkSFFRRg8S/KHGsjwmTvJEVyrOI80IZCSROjKs5SF7BXmTf71CuX+adeHih2md8sS acmw== X-Received: by 10.180.160.240 with SMTP id xn16mr3710254wib.62.1383337655044; Fri, 01 Nov 2013 13:27:35 -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 b7sm10760494wiz.8.2013.11.01.13.27.33 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 01 Nov 2013 13:27:34 -0700 (PDT) Date: Fri, 1 Nov 2013 21:27:29 +0100 From: Mateusz Guzik To: Vijay Singh Subject: Re: regd. thread ucred update in kern_accessat() Message-ID: <20131101202729.GA19342@dft-labs.eu> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: hackers@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Nov 2013 20:27:37 -0000 On Fri, Nov 01, 2013 at 11:31:05AM -0700, Vijay Singh wrote: > Hi hackers. In kern_accessat() [I'm looking at 8.2 code], for the case > where flags doesn't have AT_EACCESS set, we make a local copy of the thread > ucred. This is then passed in to vn_access(). My question is why we update > td->td_ucred with this temporary ucred? > /me takes nitpicky hat on For starters you should have checked newer tree - maybe this code would be gone and you would get an answer from respective commit. /me takes the hat off > tmpcred = crdup(cred); > tmpcred->cr_uid = cred->cr_ruid; > tmpcred->cr_groups[0] = cred->cr_rgid; > ==> td->td_ucred = tmpcred; > > At this point p->p_ucred != td->td_ucred. Couldn't this cause issues? Can you elaborate what you have in mind? This is fine. In fact, when you do setuid() thread credentials are updated only on syscall boundary. In worst case syscall that is being performed at the moment is using old credentials which is ok - whatever you wanted to do with it you can do before calling setuid() :) Other case would be when you drop some privs and suddenly something is allowed to ptrace you. Even if that was the case, already executing syscall cannot be altered and credentials are changed for another one. > Wouldn't it just suffice to use the tempcred as an argument to vn_access() > and leave the thread's ucred intact? > Before vn_access you can find namei call and it uses thread credentials. Specifically ndp->ni_cnd.cn_thread->td_ucred, as populated by NDINIT_ATRIGHTS. Maybe creating another macro which allows one to supply credentials would be ok, but one would have to go first through the code from namei and vn_access down the stack and check for td_ucred users (this should be fine, but just in case). -- Mateusz Guzik