Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 2 Apr 2012 13:10:24 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Rick Macklem <rmacklem@uoguelph.ca>
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:  <201204021310.24420.jhb@freebsd.org>
In-Reply-To: <1615948459.2113806.1333381510230.JavaMail.root@erie.cs.uoguelph.ca>
References:  <1615948459.2113806.1333381510230.JavaMail.root@erie.cs.uoguelph.ca>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204021310.24420.jhb>