On Fri, Aug 10, 2018 at 01:00:27PM -0400, Bruce Fields wrote:
> On Fri, Aug 10, 2018 at 11:29:33AM +1000, NeilBrown wrote:
> > On Mon, Mar 21 2016, Nelson Elhage wrote:
> >
> > > That's correct. The other detail that seems to be important is that
> > > the user making the call must be different from the user owning the
> > > file. We've also been using user remapping on the server, so that
> > > non-xattr calls succeed in that configuration.
> > >
> > > The reproducer James added in the bugzilla is:
> > >
> > > (on machine with IP address 10.1.1.1)
> > > sudo mkdir /nfs_test
> > > sudo useradd -u 10000 test_user
> > > sudo chown test_user /nfs_test
> > > echo "/nfs_test 10.1.1.2(rw,all_squash,anonuid=10000)" | sudo tee -a
> > > /etc/exports
> > > sudo exportfs -a
> > >
> > > (on machine with IP address 10.1.1.2)
> > > sudo mkdir /nfs_test
> > > sudo mount -t nfs -o vers=3,noacl 10.1.1.1:/nfs_test /nfs_test
> > > touch /nfs_test/foo
> > > install -m 755 /nfs_test/foo /nfs_test/bar
> >
> > Did anything ever happen about this?
> > I have a customer with a similar problem (in 4.4) but I cannot see any
> > evidence of fixes landing in mainline.
> >
> > Problem happens with you have uid mapping on the server
> > (e.g. anonuid=10000 as above) and a user with a different uid on the
> > client attempts setacl on a file with that user.
> > As anon is mapped to the owner of the file, setacl should be allowed.
> > However set_posix_acl() calls inode_owner_or_capable() which checks if
> > the client-side uid matches the visible inode->i_uid - they don't.
> >
> > Testing i_uid on the client is always incorrect for permission checking
> > with NFS - the client should always ask the server, either with ACCESS
> > or, in this case, by simply attempting the operation.
> >
> > Any suggestions how best to fix this?
> > - We could move the responsibility for permission checking into
> > i_op->set_acl, but that would be a large change and might make it too
> > easy for other filesystems to get it wrong.
> > - we could have some sort of flag asking set_posix_acl(), but that's
> > rather clumsy.... maybe if i_op->set_acl_check_perm use that without
> > testing ownership first??
> > - we could copy
> > posic_acl_xattr_{get,set,list} into nfs together with functions
> > they call, modify set_posix_acl() to not test ownership,
> > and provide a local 'struct xattr_handler' structure for NFS.
> >
> > I don't really like any of those suggestions. Can someone else do any
> > better?
>
> Do we have important callers of inode_owner_or_capable() in the vfs (as
> opposed to in individual filesystems), and do any of them pose a similar
> problem for network filesystems?
do_linkat()->may_linkat() looks kinda suspicious to me. Or what about
the O_NOATIME check in map_open()? Just engaging in dumb grepping
here....
--b.
