Date: Mon, 2 Apr 2012 16:38:11 -0400 From: John Baldwin <jhb@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: alc@freebsd.org, freebsd-fs@freebsd.org, Joel Ray Holveck <joelh@juniper.net>, David Wolfskill <david@catwhisker.org> Subject: Re: RFC: which patch is preferred for fix to PR#165923 Message-ID: <201204021638.11817.jhb@freebsd.org> In-Reply-To: <20120402191310.GX2358@deviant.kiev.zoral.com.ua> References: <1615948459.2113806.1333381510230.JavaMail.root@erie.cs.uoguelph.ca> <201204021310.24420.jhb@freebsd.org> <20120402191310.GX2358@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204021638.11817.jhb>