From 5bd9628b784cc5e38e1c7ebb680bbd6ee741230e Mon Sep 17 00:00:00 2001 From: Sun Ke Date: Mon, 15 Aug 2022 11:48:29 +0800 Subject: [PATCH 1/3] erofs: fix error return code in erofs_fscache_{meta_,}read_folio If erofs_fscache_alloc_request fail and then goto out, it will return 0. it should return a negative error code instead of 0. Fixes: d435d53228dd ("erofs: change to use asynchronous io for fscache readpage/readahead") Signed-off-by: Sun Ke Reviewed-by: Jingbo Xu Reviewed-by: Gao Xiang Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20220815034829.3940803-1-sunke32@huawei.com Signed-off-by: Gao Xiang --- fs/erofs/fscache.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/erofs/fscache.c b/fs/erofs/fscache.c index 8e01d89c3319..b5fd9d71e67f 100644 --- a/fs/erofs/fscache.c +++ b/fs/erofs/fscache.c @@ -222,8 +222,10 @@ static int erofs_fscache_meta_read_folio(struct file *data, struct folio *folio) rreq = erofs_fscache_alloc_request(folio_mapping(folio), folio_pos(folio), folio_size(folio)); - if (IS_ERR(rreq)) + if (IS_ERR(rreq)) { + ret = PTR_ERR(rreq); goto out; + } return erofs_fscache_read_folios_async(mdev.m_fscache->cookie, rreq, mdev.m_pa); @@ -301,8 +303,10 @@ static int erofs_fscache_read_folio(struct file *file, struct folio *folio) rreq = erofs_fscache_alloc_request(folio_mapping(folio), folio_pos(folio), folio_size(folio)); - if (IS_ERR(rreq)) + if (IS_ERR(rreq)) { + ret = PTR_ERR(rreq); goto out_unlock; + } pstart = mdev.m_pa + (pos - map.m_la); return erofs_fscache_read_folios_async(mdev.m_fscache->cookie, From ea0b7b0d59e81a9c1bc03f8696bb04851bc2ad22 Mon Sep 17 00:00:00 2001 From: Yue Hu Date: Fri, 12 Aug 2022 14:01:50 +0800 Subject: [PATCH 2/3] erofs: avoid the potentially wrong m_plen for big pcluster Actually, 'compressedlcs' stores compressed block count rather than lcluster count. Therefore, the number of bits for shifting the count should be 'LOG_BLOCK_SIZE' rather than 'lclusterbits' although current lcluster size is 4K. The value of 'm_plen' will be wrong once we enable the non 4K-sized lcluster. Signed-off-by: Yue Hu Reviewed-by: Gao Xiang Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20220812060150.8510-1-huyue2@coolpad.com Signed-off-by: Gao Xiang --- fs/erofs/zmap.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/erofs/zmap.c b/fs/erofs/zmap.c index 572f0b8151ba..d58549ca1df9 100644 --- a/fs/erofs/zmap.c +++ b/fs/erofs/zmap.c @@ -141,7 +141,7 @@ struct z_erofs_maprecorder { u8 type, headtype; u16 clusterofs; u16 delta[2]; - erofs_blk_t pblk, compressedlcs; + erofs_blk_t pblk, compressedblks; erofs_off_t nextpackoff; }; @@ -192,7 +192,7 @@ static int legacy_load_cluster_from_disk(struct z_erofs_maprecorder *m, DBG_BUGON(1); return -EFSCORRUPTED; } - m->compressedlcs = m->delta[0] & + m->compressedblks = m->delta[0] & ~Z_EROFS_VLE_DI_D0_CBLKCNT; m->delta[0] = 1; } @@ -293,7 +293,7 @@ static int unpack_compacted_index(struct z_erofs_maprecorder *m, DBG_BUGON(1); return -EFSCORRUPTED; } - m->compressedlcs = lo & ~Z_EROFS_VLE_DI_D0_CBLKCNT; + m->compressedblks = lo & ~Z_EROFS_VLE_DI_D0_CBLKCNT; m->delta[0] = 1; return 0; } else if (i + 1 != (int)vcnt) { @@ -497,7 +497,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m, return 0; } lcn = m->lcn + 1; - if (m->compressedlcs) + if (m->compressedblks) goto out; err = z_erofs_load_cluster_from_disk(m, lcn, false); @@ -506,7 +506,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m, /* * If the 1st NONHEAD lcluster has already been handled initially w/o - * valid compressedlcs, which means at least it mustn't be CBLKCNT, or + * valid compressedblks, which means at least it mustn't be CBLKCNT, or * an internal implemenatation error is detected. * * The following code can also handle it properly anyway, but let's @@ -523,12 +523,12 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m, * if the 1st NONHEAD lcluster is actually PLAIN or HEAD type * rather than CBLKCNT, it's a 1 lcluster-sized pcluster. */ - m->compressedlcs = 1; + m->compressedblks = 1 << (lclusterbits - LOG_BLOCK_SIZE); break; case Z_EROFS_VLE_CLUSTER_TYPE_NONHEAD: if (m->delta[0] != 1) goto err_bonus_cblkcnt; - if (m->compressedlcs) + if (m->compressedblks) break; fallthrough; default: @@ -539,7 +539,7 @@ static int z_erofs_get_extent_compressedlen(struct z_erofs_maprecorder *m, return -EFSCORRUPTED; } out: - map->m_plen = (u64)m->compressedlcs << lclusterbits; + map->m_plen = (u64)m->compressedblks << LOG_BLOCK_SIZE; return 0; err_bonus_cblkcnt: erofs_err(m->inode->i_sb, From 2f44013e39984c127c6efedf70e6b5f4e9dcf315 Mon Sep 17 00:00:00 2001 From: Gao Xiang Date: Fri, 2 Sep 2022 12:57:10 +0800 Subject: [PATCH 3/3] erofs: fix pcluster use-after-free on UP platforms During stress testing with CONFIG_SMP disabled, KASAN reports as below: ================================================================== BUG: KASAN: use-after-free in __mutex_lock+0xe5/0xc30 Read of size 8 at addr ffff8881094223f8 by task stress/7789 CPU: 0 PID: 7789 Comm: stress Not tainted 6.0.0-rc1-00002-g0d53d2e882f9 #3 Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011 Call Trace: .. __mutex_lock+0xe5/0xc30 .. z_erofs_do_read_page+0x8ce/0x1560 .. z_erofs_readahead+0x31c/0x580 .. Freed by task 7787 kasan_save_stack+0x1e/0x40 kasan_set_track+0x20/0x30 kasan_set_free_info+0x20/0x40 __kasan_slab_free+0x10c/0x190 kmem_cache_free+0xed/0x380 rcu_core+0x3d5/0xc90 __do_softirq+0x12d/0x389 Last potentially related work creation: kasan_save_stack+0x1e/0x40 __kasan_record_aux_stack+0x97/0xb0 call_rcu+0x3d/0x3f0 erofs_shrink_workstation+0x11f/0x210 erofs_shrink_scan+0xdc/0x170 shrink_slab.constprop.0+0x296/0x530 drop_slab+0x1c/0x70 drop_caches_sysctl_handler+0x70/0x80 proc_sys_call_handler+0x20a/0x2f0 vfs_write+0x555/0x6c0 ksys_write+0xbe/0x160 do_syscall_64+0x3b/0x90 The root cause is that erofs_workgroup_unfreeze() doesn't reset to orig_val thus it causes a race that the pcluster reuses unexpectedly before freeing. Since UP platforms are quite rare now, such path becomes unnecessary. Let's drop such specific-designed path directly instead. Fixes: 73f5c66df3e2 ("staging: erofs: fix `erofs_workgroup_{try_to_freeze, unfreeze}'") Reviewed-by: Yue Hu Reviewed-by: Chao Yu Link: https://lore.kernel.org/r/20220902045710.109530-1-hsiangkao@linux.alibaba.com Signed-off-by: Gao Xiang --- fs/erofs/internal.h | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index cfee49d33b95..a01cc82795a2 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h @@ -195,7 +195,6 @@ struct erofs_workgroup { atomic_t refcount; }; -#if defined(CONFIG_SMP) static inline bool erofs_workgroup_try_to_freeze(struct erofs_workgroup *grp, int val) { @@ -224,34 +223,6 @@ static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp) return atomic_cond_read_relaxed(&grp->refcount, VAL != EROFS_LOCKED_MAGIC); } -#else -static inline bool erofs_workgroup_try_to_freeze(struct erofs_workgroup *grp, - int val) -{ - preempt_disable(); - /* no need to spin on UP platforms, let's just disable preemption. */ - if (val != atomic_read(&grp->refcount)) { - preempt_enable(); - return false; - } - return true; -} - -static inline void erofs_workgroup_unfreeze(struct erofs_workgroup *grp, - int orig_val) -{ - preempt_enable(); -} - -static inline int erofs_wait_on_workgroup_freezed(struct erofs_workgroup *grp) -{ - int v = atomic_read(&grp->refcount); - - /* workgroup is never freezed on uniprocessor systems */ - DBG_BUGON(v == EROFS_LOCKED_MAGIC); - return v; -} -#endif /* !CONFIG_SMP */ #endif /* !CONFIG_EROFS_FS_ZIP */ /* we strictly follow PAGE_SIZE and no buffer head yet */