From owner-freebsd-fs@FreeBSD.ORG Mon Apr 2 17:13:11 2012 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8E83F10657AE; Mon, 2 Apr 2012 17:13:11 +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 621CD8FC1A; Mon, 2 Apr 2012 17:13:11 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id BABEEB95A; Mon, 2 Apr 2012 13:13:10 -0400 (EDT) From: John Baldwin To: Rick Macklem Date: Mon, 2 Apr 2012 13:10:24 -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> In-Reply-To: <1615948459.2113806.1333381510230.JavaMail.root@erie.cs.uoguelph.ca> MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201204021310.24420.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Mon, 02 Apr 2012 13:13:10 -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 17:13:11 -0000 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. -- John Baldwin