From owner-freebsd-fs@FreeBSD.ORG Mon Apr 2 20:53:44 2012 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D48AA106564A; Mon, 2 Apr 2012 20:53:44 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 96C6C8FC14; Mon, 2 Apr 2012 20:53:44 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id F3BD1B945; Mon, 2 Apr 2012 16:53:43 -0400 (EDT) From: John Baldwin To: Konstantin Belousov Date: Mon, 2 Apr 2012 16:38:11 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p10; KDE/4.5.5; amd64; ; ) References: <1615948459.2113806.1333381510230.JavaMail.root@erie.cs.uoguelph.ca> <201204021310.24420.jhb@freebsd.org> <20120402191310.GX2358@deviant.kiev.zoral.com.ua> In-Reply-To: <20120402191310.GX2358@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201204021638.11817.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 02 Apr 2012 16:53:44 -0400 (EDT) Cc: alc@freebsd.org, freebsd-fs@freebsd.org, Joel Ray Holveck , David Wolfskill Subject: Re: RFC: which patch is preferred for fix to PR#165923 X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Apr 2012 20:53:44 -0000 On Monday, April 02, 2012 3:13:10 pm Konstantin Belousov wrote: > On Mon, Apr 02, 2012 at 01:10:24PM -0400, John Baldwin wrote: > > On Monday, April 02, 2012 11:45:10 am Rick Macklem wrote: > > > > > However, there seems to be some disagreement as to how a patch to > > > > > use the mmap() credentials should be done. > > > > > > > > > > putpages-a.patch - This one captures the credentials during the > > > > > mmap() > > > > > call by using the property that only mmap() calls > > > > > vnode_pager_alloc() > > > > > with "prot != 0" and references them in a new field in > > > > > vm_object_t. > > > > > > > > > > putpages-b.patch - This one adds a new VOP_PUTPAGES_NOTIFY() call, > > > > > which > > > > > is done by mmap(). For all file systems except NFS clients, it is > > > > > a > > > > > null op. For the NFS clients, they reference the credentials in a > > > > > new field of the NFS specific part of the vnode. > > > > > > > > > > - I don't have a patch for the third one, but I believe Joel was > > > > > proposing > > > > > something similar to putpages-a.patch, except that the > > > > > credentials are > > > > > passed as an extra argument to VOP_PUTPAGES(), instead of the NFS > > > > > client's > > > > > VOP_PUTPAGES() looking in the vnode's vm_object_t for the cred. > > > > > > > > I think what I would prefer for the longterm is option 3) (and > > > > possibly doing > > > > the same for VOP_GETPAGES(). Also, I would call the new field in > > > > vm_object > > > > something like 'writecred'. For MFC purposes you could use > > > > putpages-a.patch > > > > instead and only make the change to the VOPs in HEAD. > > > > > > > > However, I wonder if you can't manage this all at the vnode layer? If > > > > all > > > > you need is a credential that is allowed to write, can't you cache the > > > > credential used for a successful VOP_ACCESS() check with write support > > > > in the > > > > nfsnode and just use that for write RPCs? The same thing would work > > > > for > > > > reads, yes? I know for NFSv4 you already maintain a cache of > > > > credentials > > > > from open()'s that you use for other RPC's already, so this would seem > > > > to be > > > > similar to that case. This would also let you do the "list of > > > > credentials" > > > > idea if needed and even let you add new write-capable credentials to > > > > the list > > > > via VOP_ACCESS() calls after the file was mmap()'d or open()'d. > > > > > > > I think there is a question w.r.t. should you use credentials that weren't > > > ones that mmap'd the file with PROT_WRITE. > > > > But you already are potentially (in that you might mismatch permissions > > for a given WRITE since the page might have been dirtied by a different > > user than the credential we use). You have to open() the file with write > > permissions before you mmap() it, so all mmap()'s are going to do a > > VOP_ACCESS() prior, so if you just cache whatever credential last worked > > for write access to VOP_ACCESS() that would more or less work, and be > > about as reliable while not requiring any changes outside of the NFS > > client itself. > > > > > For example, the extreme case of this would be to just VOP_GETATTR() the > > > file and use credentials with uid == va_uid to do the writes, if there are > > > EACCES failures. This assumes that the client "should always be able to > > > write the file, if it was mmap'd". This trick hasn't been used for non-mmap'd > > > writes, presumably because it was felt that the per-wrire RPC access check > > > was a good thing that the client shouldn't be circumventing. > > > > Well, we always have a credential for normal writes, with mmap() we don't. > > Presumably the open() will have to check for write permissions, so even > > what you suggest of just using the uid of the file might be fine. > > I am highly biased toward fs-specific solution. I do not like an idea > to add a credentials reference to the vm_object for the sake of some > filesystems. Esp. because the policy of which credential is right might > be very much depend on the filesystem. > > Adding notification VOP at the time of mmap() is fine for me, but it probably > indeed use the credentials of the file descriptor and not the current process. > E.g., if file descriptor was passed using unix domain socket, then > curthread->td_cred is wrong thing to use. So cached credentials used for > open() is probably indeed closest approximation. Yes, a new VOP would want to use the file's credential, not the current thread. > Fours option is to not change anything at all, e.g. my opinion is that > having root uid mapped to non-root is mostly configuration error. Actually, mapping root to nobody is a very common practice, common enough that we can't ignore it. -- John Baldwin