Re: [PATCH v2] ceph: fix posix ACL hooks

[Christoph Hellwig <hch@infradead.org>]

On Tue, Feb 04, 2014 at 11:33:35AM +0000, Steven Whitehouse wrote:
> To diverge from that topic for a moment, this thread has also brought
> together some discussion on another issue which I've been pondering
> recently.... that of whether the inode operations for get/set_xattr
> should take a dentry or not. I had thought that we'd come to the
> conclusion that 9p made it impossible to swap the current dentry
> argument for an inode, and I was about to send a patch for selinux
> support on clustered fs on that basis. However the discussion in this
> thread has made me wonder whether that really is the case or not.... Al,
> can you confirm whether your xattr-experimental patches are still under
> active consideration?

My plan was to work on the 9p and cifs conversions using the
d_find_alias hack we have in ceph right now.  That means the base work
could switch to passed in dentries or in case of 9p the per-inode fids
easily.

> The other question that I have relating to that side of things, is why
> security_inode_permission() is called from __inode_permission() rather
> than from generic_permission() ? Maybe there is a good reason, but I
> can't immediately see what it is at the moment.

Seems like almost everything of the security_* family is called from the
VFS instead of the filesystem.  There's also some very odd other
behaviour in there, e.g. for the xattrs sets are handed to the
filesystem first, and then the xattr layer calls into the security
layer, which for reads the filesystems is never reached at all.

> In response to the question elsewhere about GFS2 calling
> gfs2_permission() after the vfs has already done its checks, that is
> indeed down to needing to ensure that we have the cluster locks when
> this check is called. More importantly to know that things haven't
> changed since the VFS called the same function in case we've raced with
> another node changing the permissions, for example. There are a number
> of cases where we redo vfs level checks for this reason,

Seems like we should be able to grab a cluster lock where we grab
i_mutex in the namespace code to avoid having to redo all these checks.