From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yan, Zheng" Subject: [PATCH] ceph: fix recursively call between ceph_set_acl and __ceph_setattr Date: Wed, 19 Apr 2017 11:33:47 +0800 Message-ID: <20170419033347.45857-1-zyan@redhat.com> Return-path: Received: from mx1.redhat.com ([209.132.183.28]:34886 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759370AbdDSDdy (ORCPT ); Tue, 18 Apr 2017 23:33:54 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 28EBC31F3E2 for ; Wed, 19 Apr 2017 03:33:54 +0000 (UTC) Sender: ceph-devel-owner@vger.kernel.org List-ID: To: ceph-devel@vger.kernel.org Cc: jlayton@redhat.com, "Yan, Zheng" ceph_set_acl() calls __ceph_setattr() if the setacl operation needs to modify inode's i_mode. __ceph_setattr() updates inode's i_mode, then calls posix_acl_chmod(). The problem is that __ceph_setattr() calls posix_acl_chmod() before sending the setattr request. The get_acl() call in posix_acl_chmod() can trigger a getxattr request. The reply of the getxattr request can restore inode's i_mode to it's old value. The set_acl() call in posix_acl_chmod() sees old value of inode's i_mode, so it calls __ceph_setattr() again. Link: http://tracker.ceph.com/issues/19688 Reported-by: Jerry Lee Signed-off-by: "Yan, Zheng" --- fs/ceph/acl.c | 4 ++-- fs/ceph/inode.c | 24 +++++++++++------------- fs/ceph/super.h | 3 ++- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c index 987044b..bc985e0 100644 --- a/fs/ceph/acl.c +++ b/fs/ceph/acl.c @@ -133,7 +133,7 @@ int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type) if (new_mode != old_mode) { newattrs.ia_mode = new_mode; newattrs.ia_valid = ATTR_MODE; - ret = __ceph_setattr(inode, &newattrs); + ret = __ceph_setattr(inode, &newattrs, false); if (ret) goto out_free; } @@ -143,7 +143,7 @@ int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type) if (new_mode != old_mode) { newattrs.ia_mode = old_mode; newattrs.ia_valid = ATTR_MODE; - __ceph_setattr(inode, &newattrs); + __ceph_setattr(inode, &newattrs, false); } goto out_free; } diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index efee88c..4aa23c6 100644 --- a/fs/ceph/inode.c +++ b/fs/ceph/inode.c @@ -1885,7 +1885,7 @@ static const struct inode_operations ceph_symlink_iops = { .listxattr = ceph_listxattr, }; -int __ceph_setattr(struct inode *inode, struct iattr *attr) +int __ceph_setattr(struct inode *inode, struct iattr *attr, bool update_acl) { struct ceph_inode_info *ci = ceph_inode(inode); const unsigned int ia_valid = attr->ia_valid; @@ -2078,11 +2078,6 @@ int __ceph_setattr(struct inode *inode, struct iattr *attr) if (inode_dirty_flags) __mark_inode_dirty(inode, inode_dirty_flags); - if (ia_valid & ATTR_MODE) { - err = posix_acl_chmod(inode, attr->ia_mode); - if (err) - goto out_put; - } if (mask) { req->r_inode = inode; @@ -2096,13 +2091,16 @@ int __ceph_setattr(struct inode *inode, struct iattr *attr) ceph_cap_string(dirtied), mask); ceph_mdsc_put_request(req); - if (mask & CEPH_SETATTR_SIZE) - __ceph_do_pending_vmtruncate(inode); - ceph_free_cap_flush(prealloc_cf); - return err; -out_put: - ceph_mdsc_put_request(req); ceph_free_cap_flush(prealloc_cf); + + if (err >= 0) { + if (update_acl && (ia_valid & ATTR_MODE)) { + err = posix_acl_chmod(inode, attr->ia_mode); + } + if (mask & CEPH_SETATTR_SIZE) { + __ceph_do_pending_vmtruncate(inode); + } + } return err; } @@ -2121,7 +2119,7 @@ int ceph_setattr(struct dentry *dentry, struct iattr *attr) if (err != 0) return err; - return __ceph_setattr(inode, attr); + return __ceph_setattr(inode, attr, true); } /* diff --git a/fs/ceph/super.h b/fs/ceph/super.h index 7334ee8..bc7769c 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -809,7 +809,8 @@ static inline int ceph_do_getattr(struct inode *inode, int mask, bool force) return __ceph_do_getattr(inode, NULL, mask, force); } extern int ceph_permission(struct inode *inode, int mask); -extern int __ceph_setattr(struct inode *inode, struct iattr *attr); +extern int __ceph_setattr(struct inode *inode, struct iattr *attr, + bool update_acl); extern int ceph_setattr(struct dentry *dentry, struct iattr *attr); extern int ceph_getattr(const struct path *path, struct kstat *stat, u32 request_mask, unsigned int flags); -- 2.9.3