Date: Mon, 2 Apr 2012 22:02:55 -0400 (EDT) From: Rick Macklem <rmacklem@uoguelph.ca> To: John Baldwin <jhb@freebsd.org> 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: <394500062.2166073.1333418575016.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <201204021310.24420.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. > Taking a reference to the cred passed to the most recent VOP_OPEN() with FWRITE for the vnode and keeping that in the nfs specific vnode sounds ok to me. The only issue is that the NFS client will do this for many vnodes that never get mmap'd, but that just means more credentials may be allocated for longer. (As I understand it, the crfree() can't be done until the NFS VOP_INACTIVE()/VOP_RECLAIM(), since writing to mmap'd files can happen after the file descriptor is closed.) This would avoid any new/modified VOPs and any changes to vm_object_t. Thanks for the comments. Please keep them coming, rick > > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?394500062.2166073.1333418575016.JavaMail.root>