| From foo@baz Tue Mar 12 05:46:41 PDT 2019 |
| From: Gao Xiang <gaoxiang25@huawei.com> |
| Date: Mon, 11 Mar 2019 14:08:57 +0800 |
| Subject: staging: erofs: fix race of initializing xattrs of a inode at the same time |
| To: <stable@vger.kernel.org> |
| Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, LKML <linux-kernel@vger.kernel.org>, <linux-erofs@lists.ozlabs.org>, Chao Yu <yuchao0@huawei.com>, Chao Yu <chao@kernel.org>, Miao Xie <miaoxie@huawei.com>, Fang Wei <fangwei1@huawei.com>, Gao Xiang <gaoxiang25@huawei.com> |
| Message-ID: <20190311060858.28654-4-gaoxiang25@huawei.com> |
| |
| From: Gao Xiang <gaoxiang25@huawei.com> |
| |
| commit 62dc45979f3f8cb0ea67302a93bff686f0c46c5a upstream. |
| |
| In real scenario, there could be several threads accessing xattrs |
| of the same xattr-uninitialized inode, and init_inode_xattrs() |
| almost at the same time. |
| |
| That's actually an unexpected behavior, this patch closes the race. |
| |
| Fixes: b17500a0fdba ("staging: erofs: introduce xattr & acl support") |
| Cc: <stable@vger.kernel.org> # 4.19+ |
| Reviewed-by: Chao Yu <yuchao0@huawei.com> |
| Signed-off-by: Gao Xiang <gaoxiang25@huawei.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/staging/erofs/internal.h | 11 +++++++--- |
| drivers/staging/erofs/xattr.c | 41 +++++++++++++++++++++++++++------------ |
| 2 files changed, 37 insertions(+), 15 deletions(-) |
| |
| --- a/drivers/staging/erofs/internal.h |
| +++ b/drivers/staging/erofs/internal.h |
| @@ -327,12 +327,17 @@ static inline erofs_off_t iloc(struct er |
| return blknr_to_addr(sbi->meta_blkaddr) + (nid << sbi->islotbits); |
| } |
| |
| -#define inode_set_inited_xattr(inode) (EROFS_V(inode)->flags |= 1) |
| -#define inode_has_inited_xattr(inode) (EROFS_V(inode)->flags & 1) |
| +/* atomic flag definitions */ |
| +#define EROFS_V_EA_INITED_BIT 0 |
| + |
| +/* bitlock definitions (arranged in reverse order) */ |
| +#define EROFS_V_BL_XATTR_BIT (BITS_PER_LONG - 1) |
| |
| struct erofs_vnode { |
| erofs_nid_t nid; |
| - unsigned int flags; |
| + |
| + /* atomic flags (including bitlocks) */ |
| + unsigned long flags; |
| |
| unsigned char data_mapping_mode; |
| /* inline size in bytes */ |
| --- a/drivers/staging/erofs/xattr.c |
| +++ b/drivers/staging/erofs/xattr.c |
| @@ -44,17 +44,24 @@ static inline void xattr_iter_end_final( |
| |
| static int init_inode_xattrs(struct inode *inode) |
| { |
| + struct erofs_vnode *const vi = EROFS_V(inode); |
| struct xattr_iter it; |
| unsigned i; |
| struct erofs_xattr_ibody_header *ih; |
| struct erofs_sb_info *sbi; |
| - struct erofs_vnode *vi; |
| bool atomic_map; |
| + int ret = 0; |
| |
| - if (likely(inode_has_inited_xattr(inode))) |
| + /* the most case is that xattrs of this inode are initialized. */ |
| + if (test_bit(EROFS_V_EA_INITED_BIT, &vi->flags)) |
| return 0; |
| |
| - vi = EROFS_V(inode); |
| + if (wait_on_bit_lock(&vi->flags, EROFS_V_BL_XATTR_BIT, TASK_KILLABLE)) |
| + return -ERESTARTSYS; |
| + |
| + /* someone has initialized xattrs for us? */ |
| + if (test_bit(EROFS_V_EA_INITED_BIT, &vi->flags)) |
| + goto out_unlock; |
| |
| /* |
| * bypass all xattr operations if ->xattr_isize is not greater than |
| @@ -67,13 +74,16 @@ static int init_inode_xattrs(struct inod |
| if (vi->xattr_isize == sizeof(struct erofs_xattr_ibody_header)) { |
| errln("xattr_isize %d of nid %llu is not supported yet", |
| vi->xattr_isize, vi->nid); |
| - return -ENOTSUPP; |
| + ret = -ENOTSUPP; |
| + goto out_unlock; |
| } else if (vi->xattr_isize < sizeof(struct erofs_xattr_ibody_header)) { |
| if (unlikely(vi->xattr_isize)) { |
| DBG_BUGON(1); |
| - return -EIO; /* xattr ondisk layout error */ |
| + ret = -EIO; |
| + goto out_unlock; /* xattr ondisk layout error */ |
| } |
| - return -ENOATTR; |
| + ret = -ENOATTR; |
| + goto out_unlock; |
| } |
| |
| sbi = EROFS_I_SB(inode); |
| @@ -81,8 +91,10 @@ static int init_inode_xattrs(struct inod |
| it.ofs = erofs_blkoff(iloc(sbi, vi->nid) + vi->inode_isize); |
| |
| it.page = erofs_get_inline_page(inode, it.blkaddr); |
| - if (IS_ERR(it.page)) |
| - return PTR_ERR(it.page); |
| + if (IS_ERR(it.page)) { |
| + ret = PTR_ERR(it.page); |
| + goto out_unlock; |
| + } |
| |
| /* read in shared xattr array (non-atomic, see kmalloc below) */ |
| it.kaddr = kmap(it.page); |
| @@ -95,7 +107,8 @@ static int init_inode_xattrs(struct inod |
| sizeof(uint), GFP_KERNEL); |
| if (!vi->xattr_shared_xattrs) { |
| xattr_iter_end(&it, atomic_map); |
| - return -ENOMEM; |
| + ret = -ENOMEM; |
| + goto out_unlock; |
| } |
| |
| /* let's skip ibody header */ |
| @@ -112,7 +125,8 @@ static int init_inode_xattrs(struct inod |
| if (IS_ERR(it.page)) { |
| kfree(vi->xattr_shared_xattrs); |
| vi->xattr_shared_xattrs = NULL; |
| - return PTR_ERR(it.page); |
| + ret = PTR_ERR(it.page); |
| + goto out_unlock; |
| } |
| |
| it.kaddr = kmap_atomic(it.page); |
| @@ -125,8 +139,11 @@ static int init_inode_xattrs(struct inod |
| } |
| xattr_iter_end(&it, atomic_map); |
| |
| - inode_set_inited_xattr(inode); |
| - return 0; |
| + set_bit(EROFS_V_EA_INITED_BIT, &vi->flags); |
| + |
| +out_unlock: |
| + clear_and_wake_up_bit(EROFS_V_BL_XATTR_BIT, &vi->flags); |
| + return ret; |
| } |
| |
| struct xattr_iter_handlers { |