RFC: which patch is preferred for fix to PR#165923

[Rick Macklem]

First, a little background w.r.t. an offlist discussion of
PR# 165923. A critical part of the problem reported is the
credentials used to to the Write RPCs in the NFS client's
VOP_PUTPAGES(). Currently the code simply uses the credentials
of the thread that called VOP_PUTPAGES(), which is often incorrect,
especially when the caller is "root" and the NFS server is doing
"root squashing" (ie mapping root->nobody or similar).

Although there is no correct answer w.r.t. what credentials should
be used, there seemed to be rough consensus that the credentials of
the process that did the mmap() call would be best. (If there are
multiple writers, you could use either the 1st or most recent mmap(),
but since all writers had write permissions at open(), I don't think
it matters much which you use, but others can comment on this.)

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've attached the first 2, in case anyone wants to look at them.

Here's my take on the advantages/disadvantages of each patch:
        Advantages                          Disadvantages
patch-a simple, doesn't require VOP
        changes and, therefore, can
        be MFC'd easily                     saves a largely NFS specific item
                                            in vm_object_t
                                            depends on the fact that only
                                            mmap() calls vnode_pager_alloc() with
                                            prot != 0
patch-b basically has the advantages/disadvantages of patch1 reversed, although
        it is pretty simple, too

patch3  although it sounds cleaner to pass "cred" as an extra argument to
        VOP_PUTPAGES(), that makes it iffy (I'll let Joel discuss this) to MFC

Sorry this was so long winded, but I figured it needed some explanation. I am
hoping others have opinions w.r.t. which patch is more appropriate and that it
comes to a rough consensus, so I'll know which one to prepare for a commit to
head (and possible MFC). Hopefully Joel and Kostik will post, explaining their
preferences. (Alternately, if they give me permission, I can re-post their previous
offlist comments.)

Thanks in advance for any comments on this, rick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: putpages-a.patch
Type: text/x-patch
Size: 4630 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20120402/8728c4ea/putpages-a.bin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: putpages-b.patch
Type: text/x-patch
Size: 7181 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20120402/8728c4ea/putpages-b.bin