jffs2: check jffs2_prealloc_raw_node_refs() result in few other places

[PATCH] jffs2: check jffs2_prealloc_raw_node_refs() result in few other places

Posted by Fedor Pchelkin 8 months, 3 weeks ago

Fuzzing hit another invalid pointer dereference due to the lack of
checking whether jffs2_prealloc_raw_node_refs() completed successfully.
Subsequent logic implies that the node refs have been allocated.

Handle that. The code is ready for propagating the error upwards.

KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 1 PID: 5835 Comm: syz-executor145 Not tainted 5.10.234-syzkaller #0
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
RIP: 0010:jffs2_link_node_ref+0xac/0x690 fs/jffs2/nodelist.c:600
Call Trace:
 jffs2_mark_erased_block fs/jffs2/erase.c:460 [inline]
 jffs2_erase_pending_blocks+0x688/0x1860 fs/jffs2/erase.c:118
 jffs2_garbage_collect_pass+0x638/0x1a00 fs/jffs2/gc.c:253
 jffs2_reserve_space+0x3f4/0xad0 fs/jffs2/nodemgmt.c:167
 jffs2_write_inode_range+0x246/0xb50 fs/jffs2/write.c:362
 jffs2_write_end+0x712/0x1110 fs/jffs2/file.c:302
 generic_perform_write+0x2c2/0x500 mm/filemap.c:3347
 __generic_file_write_iter+0x252/0x610 mm/filemap.c:3465
 generic_file_write_iter+0xdb/0x230 mm/filemap.c:3497
 call_write_iter include/linux/fs.h:2039 [inline]
 do_iter_readv_writev+0x46d/0x750 fs/read_write.c:740
 do_iter_write+0x18c/0x710 fs/read_write.c:866
 vfs_writev+0x1db/0x6a0 fs/read_write.c:939
 do_pwritev fs/read_write.c:1036 [inline]
 __do_sys_pwritev fs/read_write.c:1083 [inline]
 __se_sys_pwritev fs/read_write.c:1078 [inline]
 __x64_sys_pwritev+0x235/0x310 fs/read_write.c:1078
 do_syscall_64+0x30/0x40 arch/x86/entry/common.c:46
 entry_SYSCALL_64_after_hwframe+0x67/0xd1

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Fixes: 2f785402f39b ("[JFFS2] Reduce visibility of raw_node_ref to upper layers of JFFS2 code.")
Fixes: f560928baa60 ("[JFFS2] Allocate node_ref for wasted space when skipping to page boundary")
Cc: stable@vger.kernel.org
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---

Similar to https://lore.kernel.org/linux-mtd/20250307163409.13491-2-a.sadovnikov@ispras.ru/
but touches two remaining places.

 fs/jffs2/erase.c | 4 +++-
 fs/jffs2/scan.c  | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index ef3a1e1b6cb0..fda9f4d6093f 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -425,7 +425,9 @@ static void jffs2_mark_erased_block(struct jffs2_sb_info *c, struct jffs2_eraseb
 			.totlen =	cpu_to_je32(c->cleanmarker_size)
 		};
 
-		jffs2_prealloc_raw_node_refs(c, jeb, 1);
+		ret = jffs2_prealloc_raw_node_refs(c, jeb, 1);
+		if (ret)
+			goto filebad;
 
 		marker.hdr_crc = cpu_to_je32(crc32(0, &marker, sizeof(struct jffs2_unknown_node)-4));
 
diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index 29671e33a171..62879c218d4b 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -256,7 +256,9 @@ int jffs2_scan_medium(struct jffs2_sb_info *c)
 
 		jffs2_dbg(1, "%s(): Skipping %d bytes in nextblock to ensure page alignment\n",
 			  __func__, skip);
-		jffs2_prealloc_raw_node_refs(c, c->nextblock, 1);
+		ret = jffs2_prealloc_raw_node_refs(c, c->nextblock, 1);
+		if (ret)
+			goto out;
 		jffs2_scan_dirty_space(c, c->nextblock, skip);
 	}
 #endif
-- 
2.49.0

Re: [PATCH] jffs2: check jffs2_prealloc_raw_node_refs() result in few other places

Posted by Zhihao Cheng 8 months, 3 weeks ago

在 2025/3/26 0:32, Fedor Pchelkin 写道:
> Fuzzing hit another invalid pointer dereference due to the lack of
> checking whether jffs2_prealloc_raw_node_refs() completed successfully.
> Subsequent logic implies that the node refs have been allocated.
> 
> Handle that. The code is ready for propagating the error upwards.
> 
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> CPU: 1 PID: 5835 Comm: syz-executor145 Not tainted 5.10.234-syzkaller #0
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
> RIP: 0010:jffs2_link_node_ref+0xac/0x690 fs/jffs2/nodelist.c:600
> Call Trace:
>   jffs2_mark_erased_block fs/jffs2/erase.c:460 [inline]
>   jffs2_erase_pending_blocks+0x688/0x1860 fs/jffs2/erase.c:118
>   jffs2_garbage_collect_pass+0x638/0x1a00 fs/jffs2/gc.c:253
>   jffs2_reserve_space+0x3f4/0xad0 fs/jffs2/nodemgmt.c:167
>   jffs2_write_inode_range+0x246/0xb50 fs/jffs2/write.c:362
>   jffs2_write_end+0x712/0x1110 fs/jffs2/file.c:302
>   generic_perform_write+0x2c2/0x500 mm/filemap.c:3347
>   __generic_file_write_iter+0x252/0x610 mm/filemap.c:3465
>   generic_file_write_iter+0xdb/0x230 mm/filemap.c:3497
>   call_write_iter include/linux/fs.h:2039 [inline]
>   do_iter_readv_writev+0x46d/0x750 fs/read_write.c:740
>   do_iter_write+0x18c/0x710 fs/read_write.c:866
>   vfs_writev+0x1db/0x6a0 fs/read_write.c:939
>   do_pwritev fs/read_write.c:1036 [inline]
>   __do_sys_pwritev fs/read_write.c:1083 [inline]
>   __se_sys_pwritev fs/read_write.c:1078 [inline]
>   __x64_sys_pwritev+0x235/0x310 fs/read_write.c:1078
>   do_syscall_64+0x30/0x40 arch/x86/entry/common.c:46
>   entry_SYSCALL_64_after_hwframe+0x67/0xd1
> 
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Fixes: 2f785402f39b ("[JFFS2] Reduce visibility of raw_node_ref to upper layers of JFFS2 code.")
> Fixes: f560928baa60 ("[JFFS2] Allocate node_ref for wasted space when skipping to page boundary")
> Cc: stable@vger.kernel.org
> Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> ---
> 
> Similar to https://lore.kernel.org/linux-mtd/20250307163409.13491-2-a.sadovnikov@ispras.ru/
> but touches two remaining places.
> 
>   fs/jffs2/erase.c | 4 +++-
>   fs/jffs2/scan.c  | 4 +++-
>   2 files changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
> index ef3a1e1b6cb0..fda9f4d6093f 100644
> --- a/fs/jffs2/erase.c
> +++ b/fs/jffs2/erase.c
> @@ -425,7 +425,9 @@ static void jffs2_mark_erased_block(struct jffs2_sb_info *c, struct jffs2_eraseb
>   			.totlen =	cpu_to_je32(c->cleanmarker_size)
>   		};
>   
> -		jffs2_prealloc_raw_node_refs(c, jeb, 1);
> +		ret = jffs2_prealloc_raw_node_refs(c, jeb, 1);
> +		if (ret)
> +			goto filebad;
>   
>   		marker.hdr_crc = cpu_to_je32(crc32(0, &marker, sizeof(struct jffs2_unknown_node)-4));
>   
> diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
> index 29671e33a171..62879c218d4b 100644
> --- a/fs/jffs2/scan.c
> +++ b/fs/jffs2/scan.c
> @@ -256,7 +256,9 @@ int jffs2_scan_medium(struct jffs2_sb_info *c)
>   
>   		jffs2_dbg(1, "%s(): Skipping %d bytes in nextblock to ensure page alignment\n",
>   			  __func__, skip);
> -		jffs2_prealloc_raw_node_refs(c, c->nextblock, 1);
> +		ret = jffs2_prealloc_raw_node_refs(c, c->nextblock, 1);
> +		if (ret)
> +			goto out;
>   		jffs2_scan_dirty_space(c, c->nextblock, skip);
>   	}
>   #endif
>