From 0162a70d8e25ed06db99c7abb8630f9b71aaba98 Mon Sep 17 00:00:00 2001 From: Christian Heusel Date: Sun, 11 Feb 2024 01:39:04 +0100 Subject: [PATCH 1/4] jffs2: print symbolic error name instead of error code Utilize the %pe print specifier to get the symbolic error name as a string (i.e "-ENOMEM") in the log message instead of the error code to increase its readablility. This change was suggested in https://lore.kernel.org/all/92972476-0b1f-4d0a-9951-af3fc8bc6e65@suswa.mountain/ Signed-off-by: Christian Heusel Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/jffs2/background.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c index 6da92ecaf66d..bb0ee1a59e71 100644 --- a/fs/jffs2/background.c +++ b/fs/jffs2/background.c @@ -44,8 +44,8 @@ int jffs2_start_garbage_collect_thread(struct jffs2_sb_info *c) tsk = kthread_run(jffs2_garbage_collect_thread, c, "jffs2_gcd_mtd%d", c->mtd->index); if (IS_ERR(tsk)) { - pr_warn("fork failed for JFFS2 garbage collect thread: %ld\n", - -PTR_ERR(tsk)); + pr_warn("fork failed for JFFS2 garbage collect thread: %pe\n", + tsk); complete(&c->gc_thread_exit); ret = PTR_ERR(tsk); } else { From 2e0a808224028457040caa52fe883740013adac8 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Tue, 5 Dec 2023 09:32:22 -0800 Subject: [PATCH 2/4] jffs2: nodemgmt: fix kernel-doc comments Update the end of one sentence where a comment was truncated. (dwmw2) Fix a bunch of kernel-doc warnings: nodemgmt.c:72: warning: Function parameter or member 'sumsize' not described in 'jffs2_do_reserve_space' nodemgmt.c:72: warning: expecting prototype for jffs2_reserve_space(). Prototype was for jffs2_do_reserve_space() instead nodemgmt.c:76: warning: Function parameter or member 'sumsize' not described in 'jffs2_reserve_space' nodemgmt.c:76: warning: No description found for return value of 'jffs2_reserve_space' nodemgmt.c:503: warning: Function parameter or member 'ofs' not described in 'jffs2_add_physical_node_ref' nodemgmt.c:503: warning: Function parameter or member 'ic' not described in 'jffs2_add_physical_node_ref' nodemgmt.c:503: warning: Excess function parameter 'new' description in 'jffs2_add_physical_node_ref' nodemgmt.c:503: warning: No description found for return value of 'jffs2_add_physical_node_ref' Signed-off-by: Randy Dunlap Cc: David Woodhouse Cc: Richard Weinberger Cc: linux-mtd@lists.infradead.org Reviewed-by: Zhihao Cheng Reviewed-by: Jeff Johnson Signed-off-by: Richard Weinberger --- fs/jffs2/nodemgmt.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c index a7bbe879cfc3..bbab2bdc71b6 100644 --- a/fs/jffs2/nodemgmt.c +++ b/fs/jffs2/nodemgmt.c @@ -49,28 +49,31 @@ static int jffs2_rp_can_write(struct jffs2_sb_info *c) return 0; } +static int jffs2_do_reserve_space(struct jffs2_sb_info *c, uint32_t minsize, + uint32_t *len, uint32_t sumsize); + /** * jffs2_reserve_space - request physical space to write nodes to flash * @c: superblock info * @minsize: Minimum acceptable size of allocation * @len: Returned value of allocation length * @prio: Allocation type - ALLOC_{NORMAL,DELETION} + * @sumsize: summary size requested or JFFS2_SUMMARY_NOSUM_SIZE for no summary * - * Requests a block of physical space on the flash. Returns zero for success - * and puts 'len' into the appropriate place, or returns -ENOSPC or other - * error if appropriate. Doesn't return len since that's + * Requests a block of physical space on the flash. * - * If it returns zero, jffs2_reserve_space() also downs the per-filesystem + * Returns: %0 for success and puts 'len' into the appropriate place, + * or returns -ENOSPC or other error if appropriate. + * Doesn't return len since that's already returned in @len. + * + * If it returns %0, jffs2_reserve_space() also downs the per-filesystem * allocation semaphore, to prevent more than one allocation from being - * active at any time. The semaphore is later released by jffs2_commit_allocation() + * active at any time. The semaphore is later released by jffs2_commit_allocation(). * * jffs2_reserve_space() may trigger garbage collection in order to make room * for the requested allocation. */ -static int jffs2_do_reserve_space(struct jffs2_sb_info *c, uint32_t minsize, - uint32_t *len, uint32_t sumsize); - int jffs2_reserve_space(struct jffs2_sb_info *c, uint32_t minsize, uint32_t *len, int prio, uint32_t sumsize) { @@ -488,13 +491,16 @@ static int jffs2_do_reserve_space(struct jffs2_sb_info *c, uint32_t minsize, /** * jffs2_add_physical_node_ref - add a physical node reference to the list * @c: superblock info - * @new: new node reference to add + * @ofs: offset in the block * @len: length of this physical node + * @ic: inode cache pointer * * Should only be used to report nodes for which space has been allocated * by jffs2_reserve_space. * * Must be called with the alloc_sem held. + * + * Returns: pointer to new node on success or -errno code on error */ struct jffs2_raw_node_ref *jffs2_add_physical_node_ref(struct jffs2_sb_info *c, From 7096fae56f82d00bf9f1217e6267811cc553350c Mon Sep 17 00:00:00 2001 From: Kunwu Chan Date: Mon, 5 Feb 2024 15:51:44 +0800 Subject: [PATCH 3/4] jffs2: Simplify the allocation of slab caches Use the new KMEM_CACHE() macro instead of direct kmem_cache_create to simplify the creation of SLAB caches. And change cache name from 'jffs2_tmp_dnode' to 'jffs2_tmp_dnode_info'. Signed-off-by: Kunwu Chan Reviewed-by: Zhihao Cheng Signed-off-by: Richard Weinberger --- fs/jffs2/malloc.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c index ce1189793288..411de8b361b2 100644 --- a/fs/jffs2/malloc.c +++ b/fs/jffs2/malloc.c @@ -33,27 +33,19 @@ static struct kmem_cache *xattr_ref_cache; int __init jffs2_create_slab_caches(void) { - full_dnode_slab = kmem_cache_create("jffs2_full_dnode", - sizeof(struct jffs2_full_dnode), - 0, 0, NULL); + full_dnode_slab = KMEM_CACHE(jffs2_full_dnode, 0); if (!full_dnode_slab) goto err; - raw_dirent_slab = kmem_cache_create("jffs2_raw_dirent", - sizeof(struct jffs2_raw_dirent), - 0, SLAB_HWCACHE_ALIGN, NULL); + raw_dirent_slab = KMEM_CACHE(jffs2_raw_dirent, SLAB_HWCACHE_ALIGN); if (!raw_dirent_slab) goto err; - raw_inode_slab = kmem_cache_create("jffs2_raw_inode", - sizeof(struct jffs2_raw_inode), - 0, SLAB_HWCACHE_ALIGN, NULL); + raw_inode_slab = KMEM_CACHE(jffs2_raw_inode, SLAB_HWCACHE_ALIGN); if (!raw_inode_slab) goto err; - tmp_dnode_info_slab = kmem_cache_create("jffs2_tmp_dnode", - sizeof(struct jffs2_tmp_dnode_info), - 0, 0, NULL); + tmp_dnode_info_slab = KMEM_CACHE(jffs2_tmp_dnode_info, 0); if (!tmp_dnode_info_slab) goto err; @@ -63,28 +55,20 @@ int __init jffs2_create_slab_caches(void) if (!raw_node_ref_slab) goto err; - node_frag_slab = kmem_cache_create("jffs2_node_frag", - sizeof(struct jffs2_node_frag), - 0, 0, NULL); + node_frag_slab = KMEM_CACHE(jffs2_node_frag, 0); if (!node_frag_slab) goto err; - inode_cache_slab = kmem_cache_create("jffs2_inode_cache", - sizeof(struct jffs2_inode_cache), - 0, 0, NULL); + inode_cache_slab = KMEM_CACHE(jffs2_inode_cache, 0); if (!inode_cache_slab) goto err; #ifdef CONFIG_JFFS2_FS_XATTR - xattr_datum_cache = kmem_cache_create("jffs2_xattr_datum", - sizeof(struct jffs2_xattr_datum), - 0, 0, NULL); + xattr_datum_cache = KMEM_CACHE(jffs2_xattr_datum, 0); if (!xattr_datum_cache) goto err; - xattr_ref_cache = kmem_cache_create("jffs2_xattr_ref", - sizeof(struct jffs2_xattr_ref), - 0, 0, NULL); + xattr_ref_cache = KMEM_CACHE(jffs2_xattr_ref, 0); if (!xattr_ref_cache) goto err; #endif From af9a8730ddb6a4b2edd779ccc0aceb994d616830 Mon Sep 17 00:00:00 2001 From: Wang Yong Date: Tue, 7 May 2024 15:00:46 +0800 Subject: [PATCH 4/4] jffs2: Fix potential illegal address access in jffs2_free_inode During the stress testing of the jffs2 file system,the following abnormal printouts were found: [ 2430.649000] Unable to handle kernel paging request at virtual address 0069696969696948 [ 2430.649622] Mem abort info: [ 2430.649829] ESR = 0x96000004 [ 2430.650115] EC = 0x25: DABT (current EL), IL = 32 bits [ 2430.650564] SET = 0, FnV = 0 [ 2430.650795] EA = 0, S1PTW = 0 [ 2430.651032] FSC = 0x04: level 0 translation fault [ 2430.651446] Data abort info: [ 2430.651683] ISV = 0, ISS = 0x00000004 [ 2430.652001] CM = 0, WnR = 0 [ 2430.652558] [0069696969696948] address between user and kernel address ranges [ 2430.653265] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 2430.654512] CPU: 2 PID: 20919 Comm: cat Not tainted 5.15.25-g512f31242bf6 #33 [ 2430.655008] Hardware name: linux,dummy-virt (DT) [ 2430.655517] pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 2430.656142] pc : kfree+0x78/0x348 [ 2430.656630] lr : jffs2_free_inode+0x24/0x48 [ 2430.657051] sp : ffff800009eebd10 [ 2430.657355] x29: ffff800009eebd10 x28: 0000000000000001 x27: 0000000000000000 [ 2430.658327] x26: ffff000038f09d80 x25: 0080000000000000 x24: ffff800009d38000 [ 2430.658919] x23: 5a5a5a5a5a5a5a5a x22: ffff000038f09d80 x21: ffff8000084f0d14 [ 2430.659434] x20: ffff0000bf9a6ac0 x19: 0169696969696940 x18: 0000000000000000 [ 2430.659969] x17: ffff8000b6506000 x16: ffff800009eec000 x15: 0000000000004000 [ 2430.660637] x14: 0000000000000000 x13: 00000001000820a1 x12: 00000000000d1b19 [ 2430.661345] x11: 0004000800000000 x10: 0000000000000001 x9 : ffff8000084f0d14 [ 2430.662025] x8 : ffff0000bf9a6b40 x7 : ffff0000bf9a6b48 x6 : 0000000003470302 [ 2430.662695] x5 : ffff00002e41dcc0 x4 : ffff0000bf9aa3b0 x3 : 0000000003470342 [ 2430.663486] x2 : 0000000000000000 x1 : ffff8000084f0d14 x0 : fffffc0000000000 [ 2430.664217] Call trace: [ 2430.664528] kfree+0x78/0x348 [ 2430.664855] jffs2_free_inode+0x24/0x48 [ 2430.665233] i_callback+0x24/0x50 [ 2430.665528] rcu_do_batch+0x1ac/0x448 [ 2430.665892] rcu_core+0x28c/0x3c8 [ 2430.666151] rcu_core_si+0x18/0x28 [ 2430.666473] __do_softirq+0x138/0x3cc [ 2430.666781] irq_exit+0xf0/0x110 [ 2430.667065] handle_domain_irq+0x6c/0x98 [ 2430.667447] gic_handle_irq+0xac/0xe8 [ 2430.667739] call_on_irq_stack+0x28/0x54 The parameter passed to kfree was 5a5a5a5a, which corresponds to the target field of the jffs_inode_info structure. It was found that all variables in the jffs_inode_info structure were 5a5a5a5a, except for the first member sem. It is suspected that these variables are not initialized because they were set to 5a5a5a5a during memory testing, which is meant to detect uninitialized memory.The sem variable is initialized in the function jffs2_i_init_once, while other members are initialized in the function jffs2_init_inode_info. The function jffs2_init_inode_info is called after iget_locked, but in the iget_locked function, the destroy_inode process is triggered, which releases the inode and consequently, the target member of the inode is not initialized.In concurrent high pressure scenarios, iget_locked may enter the destroy_inode branch as described in the code. Since the destroy_inode functionality of jffs2 only releases the target, the fix method is to set target to NULL in jffs2_i_init_once. Signed-off-by: Wang Yong Reviewed-by: Lu Zhongjun Reviewed-by: Yang Tao Cc: Xu Xin Cc: Yang Yang Signed-off-by: Richard Weinberger --- fs/jffs2/super.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c index aede1be4dc0c..4545f885c41e 100644 --- a/fs/jffs2/super.c +++ b/fs/jffs2/super.c @@ -58,6 +58,7 @@ static void jffs2_i_init_once(void *foo) struct jffs2_inode_info *f = foo; mutex_init(&f->sem); + f->target = NULL; inode_init_once(&f->vfs_inode); }