Re: [PATCH 2/5] ceph: Propagate dentry down to inode_change_ok()

[Jan Kara <jack@suse.cz>]

On Mon 19-09-16 14:57:02, Jeff Layton wrote:
> On Mon, 2016-09-19 at 17:30 +0200, Jan Kara wrote:
> > To avoid clearing of capabilities or security related extended
> > attributes too early, inode_change_ok() will need to take dentry instead
> > of inode. ceph_setattr() has the dentry easily available but
> > __ceph_setattr() is also called from ceph_set_acl() where dentry is not
> > easily available. Luckily that call path does not need inode_change_ok()
> > to be called anyway. So reorganize functions a bit so that
> > inode_change_ok() is called only from paths where dentry is available.
> > 
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > �fs/ceph/acl.c���|��5 +++++
> > �fs/ceph/inode.c | 19 +++++++++++--------
> > �2 files changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
> > index 013151d50069..a2cedfde75eb 100644
> > --- a/fs/ceph/acl.c
> > +++ b/fs/ceph/acl.c
> > @@ -127,6 +127,11 @@ int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> > > �			goto out_free;
> > > �	}
> > �
> > > +	if (ceph_snap(inode) != CEPH_NOSNAP) {
> > > +		ret = -EROFS;
> > > +		goto out_free;
> > > +	}
> > +
> 
> So to make sure I understand: What's the expected behavior when someone
> changes the ACL in such a way that the mode gets changed? Should
> security_inode_killpriv be getting called in that case? If so, where
> does that happen, as I don't see notify_change being called in this
> codepath?

No. security_inode_killpriv() is supposed to be called when either contents
of the file changes (truncate, write) or when owner of the file changes. It
is not called for permission changes.

...
> Looks reasonable though:
> 
> Acked-by: Jeff Layton <jlayton@redhat.com>

Thanks for review!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR