linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size
@ 2022-11-15 15:14 George Kennedy
  2022-11-19 21:17 ` Richard Weinberger
  2023-04-17 16:01 ` [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size] Uwe Kleine-König
  0 siblings, 2 replies; 5+ messages in thread
From: George Kennedy @ 2022-11-15 15:14 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr
  Cc: eorge.kennedy, linux-mtd, syzkaller, linux-kernel, harshit.m.mogalapalli

Ensure that the VID header offset + VID header size does not exceed
the allocated area to avoid slab OOB.

BUG: KASAN: slab-out-of-bounds in crc32_body lib/crc32.c:111 [inline]
BUG: KASAN: slab-out-of-bounds in crc32_le_generic lib/crc32.c:179 [inline]
BUG: KASAN: slab-out-of-bounds in crc32_le_base+0x58c/0x626 lib/crc32.c:197
Read of size 4 at addr ffff88802bb36f00 by task syz-executor136/1555

CPU: 2 PID: 1555 Comm: syz-executor136 Tainted: G        W
6.0.0-1868 #1
Hardware name: Red Hat KVM, BIOS 1.13.0-2.module+el8.3.0+7860+a7792d29
04/01/2014
Call Trace:
  <TASK>
  __dump_stack lib/dump_stack.c:88 [inline]
  dump_stack_lvl+0x85/0xad lib/dump_stack.c:106
  print_address_description mm/kasan/report.c:317 [inline]
  print_report.cold.13+0xb6/0x6bb mm/kasan/report.c:433
  kasan_report+0xa7/0x11b mm/kasan/report.c:495
  crc32_body lib/crc32.c:111 [inline]
  crc32_le_generic lib/crc32.c:179 [inline]
  crc32_le_base+0x58c/0x626 lib/crc32.c:197
  ubi_io_write_vid_hdr+0x1b7/0x472 drivers/mtd/ubi/io.c:1067
  create_vtbl+0x4d5/0x9c4 drivers/mtd/ubi/vtbl.c:317
  create_empty_lvol drivers/mtd/ubi/vtbl.c:500 [inline]
  ubi_read_volume_table+0x67b/0x288a drivers/mtd/ubi/vtbl.c:812
  ubi_attach+0xf34/0x1603 drivers/mtd/ubi/attach.c:1601
  ubi_attach_mtd_dev+0x6f3/0x185e drivers/mtd/ubi/build.c:965
  ctrl_cdev_ioctl+0x2db/0x347 drivers/mtd/ubi/cdev.c:1043
  vfs_ioctl fs/ioctl.c:51 [inline]
  __do_sys_ioctl fs/ioctl.c:870 [inline]
  __se_sys_ioctl fs/ioctl.c:856 [inline]
  __x64_sys_ioctl+0x193/0x213 fs/ioctl.c:856
  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
  do_syscall_64+0x3e/0x86 arch/x86/entry/common.c:80
  entry_SYSCALL_64_after_hwframe+0x63/0x0
RIP: 0033:0x7f96d5cf753d
Code:
RSP: 002b:00007fffd72206f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f96d5cf753d
RDX: 0000000020000080 RSI: 0000000040186f40 RDI: 0000000000000003
RBP: 0000000000400cd0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000400be0
R13: 00007fffd72207e0 R14: 0000000000000000 R15: 0000000000000000
  </TASK>

Allocated by task 1555:
  kasan_save_stack+0x20/0x3d mm/kasan/common.c:38
  kasan_set_track mm/kasan/common.c:45 [inline]
  set_alloc_info mm/kasan/common.c:437 [inline]
  ____kasan_kmalloc mm/kasan/common.c:516 [inline]
  __kasan_kmalloc+0x88/0xa3 mm/kasan/common.c:525
  kasan_kmalloc include/linux/kasan.h:234 [inline]
  __kmalloc+0x138/0x257 mm/slub.c:4429
  kmalloc include/linux/slab.h:605 [inline]
  ubi_alloc_vid_buf drivers/mtd/ubi/ubi.h:1093 [inline]
  create_vtbl+0xcc/0x9c4 drivers/mtd/ubi/vtbl.c:295
  create_empty_lvol drivers/mtd/ubi/vtbl.c:500 [inline]
  ubi_read_volume_table+0x67b/0x288a drivers/mtd/ubi/vtbl.c:812
  ubi_attach+0xf34/0x1603 drivers/mtd/ubi/attach.c:1601
  ubi_attach_mtd_dev+0x6f3/0x185e drivers/mtd/ubi/build.c:965
  ctrl_cdev_ioctl+0x2db/0x347 drivers/mtd/ubi/cdev.c:1043
  vfs_ioctl fs/ioctl.c:51 [inline]
  __do_sys_ioctl fs/ioctl.c:870 [inline]
  __se_sys_ioctl fs/ioctl.c:856 [inline]
  __x64_sys_ioctl+0x193/0x213 fs/ioctl.c:856
  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
  do_syscall_64+0x3e/0x86 arch/x86/entry/common.c:80
  entry_SYSCALL_64_after_hwframe+0x63/0x0

The buggy address belongs to the object at ffff88802bb36e00
  which belongs to the cache kmalloc-256 of size 256
The buggy address is located 0 bytes to the right of
  256-byte region [ffff88802bb36e00, ffff88802bb36f00)

The buggy address belongs to the physical page:
page:00000000ea4d1263 refcount:1 mapcount:0 mapping:0000000000000000
index:0x0 pfn:0x2bb36
head:00000000ea4d1263 order:1 compound_mapcount:0 compound_pincount:0
flags: 0xfffffc0010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
raw: 000fffffc0010200 ffffea000066c300 dead000000000003 ffff888100042b40
raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff88802bb36e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff88802bb36e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff88802bb36f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                    ^
  ffff88802bb36f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  ffff88802bb37000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
Reported-by: syzkaller <syzkaller@googlegroups.com>
Signed-off-by: George Kennedy <george.kennedy@oracle.com>
---
  drivers/mtd/ubi/build.c | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index a32050fecabf..53aa4de6b963 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -663,6 +663,12 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
  	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
  	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
  
+	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
+	    ubi->vid_hdr_alsize)) {
+		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
+		return -EINVAL;
+	}
+
  	dbg_gen("min_io_size      %d", ubi->min_io_size);
  	dbg_gen("max_write_size   %d", ubi->max_write_size);
  	dbg_gen("hdrs_min_io_size %d", ubi->hdrs_min_io_size);
-- 
2.31.1



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size
  2022-11-15 15:14 [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size George Kennedy
@ 2022-11-19 21:17 ` Richard Weinberger
  2023-04-17 16:01 ` [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size] Uwe Kleine-König
  1 sibling, 0 replies; 5+ messages in thread
From: Richard Weinberger @ 2022-11-19 21:17 UTC (permalink / raw)
  To: George Kennedy
  Cc: Miquel Raynal, Vignesh Raghavendra, eorge kennedy, linux-mtd,
	syzkaller, linux-kernel, harshit m mogalapalli

----- Ursprüngliche Mail -----
> Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: George Kennedy <george.kennedy@oracle.com>
> ---
>  drivers/mtd/ubi/build.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index a32050fecabf..53aa4de6b963 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -663,6 +663,12 @@ static int io_init(struct ubi_device *ubi, int
> max_beb_per1024)
>  	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
>  	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
>  
> +	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
> +	    ubi->vid_hdr_alsize)) {
> +		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
> +		return -EINVAL;
> +	}
> +

Thanks for fixing this, applied to the fixes branch.
That said, something broke whitespaces in your patch, I applied it manually.
Context lines before and after contain an extra space and caused both
git am and patch to give up.

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size]
  2022-11-15 15:14 [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size George Kennedy
  2022-11-19 21:17 ` Richard Weinberger
@ 2023-04-17 16:01 ` Uwe Kleine-König
  2023-04-17 16:05   ` Uwe Kleine-König
  2023-04-17 16:07   ` Richard Weinberger
  1 sibling, 2 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2023-04-17 16:01 UTC (permalink / raw)
  To: George Kennedy
  Cc: richard, miquel.raynal, vigneshr, eorge.kennedy, linux-mtd,
	syzkaller, linux-kernel, harshit.m.mogalapalli, kernel, stable


[-- Attachment #1.1: Type: text/plain, Size: 7816 bytes --]

Hello,

On Tue, Nov 15, 2022 at 10:14:44AM -0500, George Kennedy wrote:
> Ensure that the VID header offset + VID header size does not exceed
> the allocated area to avoid slab OOB.
> 
> BUG: KASAN: slab-out-of-bounds in crc32_body lib/crc32.c:111 [inline]
> BUG: KASAN: slab-out-of-bounds in crc32_le_generic lib/crc32.c:179 [inline]
> BUG: KASAN: slab-out-of-bounds in crc32_le_base+0x58c/0x626 lib/crc32.c:197
> Read of size 4 at addr ffff88802bb36f00 by task syz-executor136/1555
> 
> CPU: 2 PID: 1555 Comm: syz-executor136 Tainted: G        W
> 6.0.0-1868 #1
> Hardware name: Red Hat KVM, BIOS 1.13.0-2.module+el8.3.0+7860+a7792d29
> 04/01/2014
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0x85/0xad lib/dump_stack.c:106
>  print_address_description mm/kasan/report.c:317 [inline]
>  print_report.cold.13+0xb6/0x6bb mm/kasan/report.c:433
>  kasan_report+0xa7/0x11b mm/kasan/report.c:495
>  crc32_body lib/crc32.c:111 [inline]
>  crc32_le_generic lib/crc32.c:179 [inline]
>  crc32_le_base+0x58c/0x626 lib/crc32.c:197
>  ubi_io_write_vid_hdr+0x1b7/0x472 drivers/mtd/ubi/io.c:1067
>  create_vtbl+0x4d5/0x9c4 drivers/mtd/ubi/vtbl.c:317
>  create_empty_lvol drivers/mtd/ubi/vtbl.c:500 [inline]
>  ubi_read_volume_table+0x67b/0x288a drivers/mtd/ubi/vtbl.c:812
>  ubi_attach+0xf34/0x1603 drivers/mtd/ubi/attach.c:1601
>  ubi_attach_mtd_dev+0x6f3/0x185e drivers/mtd/ubi/build.c:965
>  ctrl_cdev_ioctl+0x2db/0x347 drivers/mtd/ubi/cdev.c:1043
>  vfs_ioctl fs/ioctl.c:51 [inline]
>  __do_sys_ioctl fs/ioctl.c:870 [inline]
>  __se_sys_ioctl fs/ioctl.c:856 [inline]
>  __x64_sys_ioctl+0x193/0x213 fs/ioctl.c:856
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x3e/0x86 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x63/0x0
> RIP: 0033:0x7f96d5cf753d
> Code:
> RSP: 002b:00007fffd72206f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f96d5cf753d
> RDX: 0000000020000080 RSI: 0000000040186f40 RDI: 0000000000000003
> RBP: 0000000000400cd0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000400be0
> R13: 00007fffd72207e0 R14: 0000000000000000 R15: 0000000000000000
>  </TASK>
> 
> Allocated by task 1555:
>  kasan_save_stack+0x20/0x3d mm/kasan/common.c:38
>  kasan_set_track mm/kasan/common.c:45 [inline]
>  set_alloc_info mm/kasan/common.c:437 [inline]
>  ____kasan_kmalloc mm/kasan/common.c:516 [inline]
>  __kasan_kmalloc+0x88/0xa3 mm/kasan/common.c:525
>  kasan_kmalloc include/linux/kasan.h:234 [inline]
>  __kmalloc+0x138/0x257 mm/slub.c:4429
>  kmalloc include/linux/slab.h:605 [inline]
>  ubi_alloc_vid_buf drivers/mtd/ubi/ubi.h:1093 [inline]
>  create_vtbl+0xcc/0x9c4 drivers/mtd/ubi/vtbl.c:295
>  create_empty_lvol drivers/mtd/ubi/vtbl.c:500 [inline]
>  ubi_read_volume_table+0x67b/0x288a drivers/mtd/ubi/vtbl.c:812
>  ubi_attach+0xf34/0x1603 drivers/mtd/ubi/attach.c:1601
>  ubi_attach_mtd_dev+0x6f3/0x185e drivers/mtd/ubi/build.c:965
>  ctrl_cdev_ioctl+0x2db/0x347 drivers/mtd/ubi/cdev.c:1043
>  vfs_ioctl fs/ioctl.c:51 [inline]
>  __do_sys_ioctl fs/ioctl.c:870 [inline]
>  __se_sys_ioctl fs/ioctl.c:856 [inline]
>  __x64_sys_ioctl+0x193/0x213 fs/ioctl.c:856
>  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>  do_syscall_64+0x3e/0x86 arch/x86/entry/common.c:80
>  entry_SYSCALL_64_after_hwframe+0x63/0x0
> 
> The buggy address belongs to the object at ffff88802bb36e00
>  which belongs to the cache kmalloc-256 of size 256
> The buggy address is located 0 bytes to the right of
>  256-byte region [ffff88802bb36e00, ffff88802bb36f00)
> 
> The buggy address belongs to the physical page:
> page:00000000ea4d1263 refcount:1 mapcount:0 mapping:0000000000000000
> index:0x0 pfn:0x2bb36
> head:00000000ea4d1263 order:1 compound_mapcount:0 compound_pincount:0
> flags: 0xfffffc0010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
> raw: 000fffffc0010200 ffffea000066c300 dead000000000003 ffff888100042b40
> raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
>  ffff88802bb36e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffff88802bb36e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > ffff88802bb36f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>                    ^
>  ffff88802bb36f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  ffff88802bb37000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
> 
> Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: George Kennedy <george.kennedy@oracle.com>
> ---
>  drivers/mtd/ubi/build.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index a32050fecabf..53aa4de6b963 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -663,6 +663,12 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
>  	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
>  	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
> +	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
> +	    ubi->vid_hdr_alsize)) {
> +		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
> +		return -EINVAL;
> +	}
> +

This patch is in mainline as 1b42b1a36fc946f0d7088425b90d491b4257ca3e,
and backported to various stable releases.

For me this breaks

	ubiattach -m 0 -O 2048

I think the check

	ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->vid_hdr_alsize

is wrong. Without -O passed to ubiattach (and dynamic debug enabled) I
get:

[ 5294.936762] UBI DBG gen (pid 9619): sizeof(struct ubi_ainf_peb) 56
[ 5294.936769] UBI DBG gen (pid 9619): sizeof(struct ubi_wl_entry) 32
[ 5294.936774] UBI DBG gen (pid 9619): min_io_size      2048
[ 5294.936779] UBI DBG gen (pid 9619): max_write_size   2048
[ 5294.936783] UBI DBG gen (pid 9619): hdrs_min_io_size 512
[ 5294.936787] UBI DBG gen (pid 9619): ec_hdr_alsize    512
[ 5294.936791] UBI DBG gen (pid 9619): vid_hdr_alsize   512
[ 5294.936796] UBI DBG gen (pid 9619): vid_hdr_offset   512
[ 5294.936800] UBI DBG gen (pid 9619): vid_hdr_aloffset 512
[ 5294.936804] UBI DBG gen (pid 9619): vid_hdr_shift    0
[ 5294.936808] UBI DBG gen (pid 9619): leb_start        2048
[ 5294.936812] UBI DBG gen (pid 9619): max_erroneous    409

So the check would only pass for vid_hdr_offset <= 512 -
UBI_VID_HDR_SIZE; note that even specifying the default value 512 (i.e.

	ubiattach -m 0 -O 512

) fails the check.

A less strong check would be:

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 0904eb40c95f..69c28a862430 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -666,8 +666,8 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
 	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
 
-	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
-	    ubi->vid_hdr_alsize)) {
+	if (ubi->vid_hdr_offset &&
+	    ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->peb_size) {
 		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
 		return -EINVAL;
 	}

But I'm unsure if this would be too lax?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size]
  2023-04-17 16:01 ` [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size] Uwe Kleine-König
@ 2023-04-17 16:05   ` Uwe Kleine-König
  2023-04-17 16:07   ` Richard Weinberger
  1 sibling, 0 replies; 5+ messages in thread
From: Uwe Kleine-König @ 2023-04-17 16:05 UTC (permalink / raw)
  To: George Kennedy
  Cc: vigneshr, eorge.kennedy, richard, linux-kernel, stable,
	syzkaller, linux-mtd, harshit.m.mogalapalli, miquel.raynal,
	kernel


[-- Attachment #1.1: Type: text/plain, Size: 8370 bytes --]

Hello,

On Mon, Apr 17, 2023 at 06:01:02PM +0200, Uwe Kleine-König wrote:
> On Tue, Nov 15, 2022 at 10:14:44AM -0500, George Kennedy wrote:
> > Ensure that the VID header offset + VID header size does not exceed
> > the allocated area to avoid slab OOB.
> > 
> > BUG: KASAN: slab-out-of-bounds in crc32_body lib/crc32.c:111 [inline]
> > BUG: KASAN: slab-out-of-bounds in crc32_le_generic lib/crc32.c:179 [inline]
> > BUG: KASAN: slab-out-of-bounds in crc32_le_base+0x58c/0x626 lib/crc32.c:197
> > Read of size 4 at addr ffff88802bb36f00 by task syz-executor136/1555
> > 
> > CPU: 2 PID: 1555 Comm: syz-executor136 Tainted: G        W
> > 6.0.0-1868 #1
> > Hardware name: Red Hat KVM, BIOS 1.13.0-2.module+el8.3.0+7860+a7792d29
> > 04/01/2014
> > Call Trace:
> >  <TASK>
> >  __dump_stack lib/dump_stack.c:88 [inline]
> >  dump_stack_lvl+0x85/0xad lib/dump_stack.c:106
> >  print_address_description mm/kasan/report.c:317 [inline]
> >  print_report.cold.13+0xb6/0x6bb mm/kasan/report.c:433
> >  kasan_report+0xa7/0x11b mm/kasan/report.c:495
> >  crc32_body lib/crc32.c:111 [inline]
> >  crc32_le_generic lib/crc32.c:179 [inline]
> >  crc32_le_base+0x58c/0x626 lib/crc32.c:197
> >  ubi_io_write_vid_hdr+0x1b7/0x472 drivers/mtd/ubi/io.c:1067
> >  create_vtbl+0x4d5/0x9c4 drivers/mtd/ubi/vtbl.c:317
> >  create_empty_lvol drivers/mtd/ubi/vtbl.c:500 [inline]
> >  ubi_read_volume_table+0x67b/0x288a drivers/mtd/ubi/vtbl.c:812
> >  ubi_attach+0xf34/0x1603 drivers/mtd/ubi/attach.c:1601
> >  ubi_attach_mtd_dev+0x6f3/0x185e drivers/mtd/ubi/build.c:965
> >  ctrl_cdev_ioctl+0x2db/0x347 drivers/mtd/ubi/cdev.c:1043
> >  vfs_ioctl fs/ioctl.c:51 [inline]
> >  __do_sys_ioctl fs/ioctl.c:870 [inline]
> >  __se_sys_ioctl fs/ioctl.c:856 [inline]
> >  __x64_sys_ioctl+0x193/0x213 fs/ioctl.c:856
> >  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> >  do_syscall_64+0x3e/0x86 arch/x86/entry/common.c:80
> >  entry_SYSCALL_64_after_hwframe+0x63/0x0
> > RIP: 0033:0x7f96d5cf753d
> > Code:
> > RSP: 002b:00007fffd72206f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f96d5cf753d
> > RDX: 0000000020000080 RSI: 0000000040186f40 RDI: 0000000000000003
> > RBP: 0000000000400cd0 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000400be0
> > R13: 00007fffd72207e0 R14: 0000000000000000 R15: 0000000000000000
> >  </TASK>
> > 
> > Allocated by task 1555:
> >  kasan_save_stack+0x20/0x3d mm/kasan/common.c:38
> >  kasan_set_track mm/kasan/common.c:45 [inline]
> >  set_alloc_info mm/kasan/common.c:437 [inline]
> >  ____kasan_kmalloc mm/kasan/common.c:516 [inline]
> >  __kasan_kmalloc+0x88/0xa3 mm/kasan/common.c:525
> >  kasan_kmalloc include/linux/kasan.h:234 [inline]
> >  __kmalloc+0x138/0x257 mm/slub.c:4429
> >  kmalloc include/linux/slab.h:605 [inline]
> >  ubi_alloc_vid_buf drivers/mtd/ubi/ubi.h:1093 [inline]
> >  create_vtbl+0xcc/0x9c4 drivers/mtd/ubi/vtbl.c:295
> >  create_empty_lvol drivers/mtd/ubi/vtbl.c:500 [inline]
> >  ubi_read_volume_table+0x67b/0x288a drivers/mtd/ubi/vtbl.c:812
> >  ubi_attach+0xf34/0x1603 drivers/mtd/ubi/attach.c:1601
> >  ubi_attach_mtd_dev+0x6f3/0x185e drivers/mtd/ubi/build.c:965
> >  ctrl_cdev_ioctl+0x2db/0x347 drivers/mtd/ubi/cdev.c:1043
> >  vfs_ioctl fs/ioctl.c:51 [inline]
> >  __do_sys_ioctl fs/ioctl.c:870 [inline]
> >  __se_sys_ioctl fs/ioctl.c:856 [inline]
> >  __x64_sys_ioctl+0x193/0x213 fs/ioctl.c:856
> >  do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> >  do_syscall_64+0x3e/0x86 arch/x86/entry/common.c:80
> >  entry_SYSCALL_64_after_hwframe+0x63/0x0
> > 
> > The buggy address belongs to the object at ffff88802bb36e00
> >  which belongs to the cache kmalloc-256 of size 256
> > The buggy address is located 0 bytes to the right of
> >  256-byte region [ffff88802bb36e00, ffff88802bb36f00)
> > 
> > The buggy address belongs to the physical page:
> > page:00000000ea4d1263 refcount:1 mapcount:0 mapping:0000000000000000
> > index:0x0 pfn:0x2bb36
> > head:00000000ea4d1263 order:1 compound_mapcount:0 compound_pincount:0
> > flags: 0xfffffc0010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
> > raw: 000fffffc0010200 ffffea000066c300 dead000000000003 ffff888100042b40
> > raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
> > page dumped because: kasan: bad access detected
> > 
> > Memory state around the buggy address:
> >  ffff88802bb36e00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >  ffff88802bb36e80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > ffff88802bb36f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >                    ^
> >  ffff88802bb36f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> >  ffff88802bb37000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > ==================================================================
> > 
> > Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
> > Reported-by: syzkaller <syzkaller@googlegroups.com>
> > Signed-off-by: George Kennedy <george.kennedy@oracle.com>
> > ---
> >  drivers/mtd/ubi/build.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> > index a32050fecabf..53aa4de6b963 100644
> > --- a/drivers/mtd/ubi/build.c
> > +++ b/drivers/mtd/ubi/build.c
> > @@ -663,6 +663,12 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
> >  	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
> >  	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
> > +	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
> > +	    ubi->vid_hdr_alsize)) {
> > +		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
> > +		return -EINVAL;
> > +	}
> > +
> 
> This patch is in mainline as 1b42b1a36fc946f0d7088425b90d491b4257ca3e,
> and backported to various stable releases.
> 
> For me this breaks
> 
> 	ubiattach -m 0 -O 2048
> 
> I think the check
> 
> 	ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->vid_hdr_alsize
> 
> is wrong. Without -O passed to ubiattach (and dynamic debug enabled) I
> get:
> 
> [ 5294.936762] UBI DBG gen (pid 9619): sizeof(struct ubi_ainf_peb) 56
> [ 5294.936769] UBI DBG gen (pid 9619): sizeof(struct ubi_wl_entry) 32
> [ 5294.936774] UBI DBG gen (pid 9619): min_io_size      2048
> [ 5294.936779] UBI DBG gen (pid 9619): max_write_size   2048
> [ 5294.936783] UBI DBG gen (pid 9619): hdrs_min_io_size 512
> [ 5294.936787] UBI DBG gen (pid 9619): ec_hdr_alsize    512
> [ 5294.936791] UBI DBG gen (pid 9619): vid_hdr_alsize   512
> [ 5294.936796] UBI DBG gen (pid 9619): vid_hdr_offset   512
> [ 5294.936800] UBI DBG gen (pid 9619): vid_hdr_aloffset 512
> [ 5294.936804] UBI DBG gen (pid 9619): vid_hdr_shift    0
> [ 5294.936808] UBI DBG gen (pid 9619): leb_start        2048
> [ 5294.936812] UBI DBG gen (pid 9619): max_erroneous    409
> 
> So the check would only pass for vid_hdr_offset <= 512 -
> UBI_VID_HDR_SIZE; note that even specifying the default value 512 (i.e.
> 
> 	ubiattach -m 0 -O 512
> 
> ) fails the check.
> 
> A less strong check would be:
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 0904eb40c95f..69c28a862430 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -666,8 +666,8 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
>  	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
>  	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
>  
> -	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
> -	    ubi->vid_hdr_alsize)) {
> +	if (ubi->vid_hdr_offset &&
> +	    ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->peb_size) {
>  		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
>  		return -EINVAL;
>  	}
> 
> But I'm unsure if this would be too lax?!

I just learned in irc that

	1e020e1b96af ("ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size")

is supposed to fix that.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size]
  2023-04-17 16:01 ` [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size] Uwe Kleine-König
  2023-04-17 16:05   ` Uwe Kleine-König
@ 2023-04-17 16:07   ` Richard Weinberger
  1 sibling, 0 replies; 5+ messages in thread
From: Richard Weinberger @ 2023-04-17 16:07 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: George Kennedy, Miquel Raynal, Vignesh Raghavendra,
	eorge kennedy, linux-mtd, syzkaller, linux-kernel,
	harshit m mogalapalli, kernel, stable

Uwe,

----- Ursprüngliche Mail -----
> Von: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
> This patch is in mainline as 1b42b1a36fc946f0d7088425b90d491b4257ca3e,
> and backported to various stable releases.
> 
> For me this breaks
> 
>	ubiattach -m 0 -O 2048
> 
> I think the check
> 
>	ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->vid_hdr_alsize
> 
> is wrong. Without -O passed to ubiattach (and dynamic debug enabled) I
> get:
> 
> [ 5294.936762] UBI DBG gen (pid 9619): sizeof(struct ubi_ainf_peb) 56
> [ 5294.936769] UBI DBG gen (pid 9619): sizeof(struct ubi_wl_entry) 32
> [ 5294.936774] UBI DBG gen (pid 9619): min_io_size      2048
> [ 5294.936779] UBI DBG gen (pid 9619): max_write_size   2048
> [ 5294.936783] UBI DBG gen (pid 9619): hdrs_min_io_size 512
> [ 5294.936787] UBI DBG gen (pid 9619): ec_hdr_alsize    512
> [ 5294.936791] UBI DBG gen (pid 9619): vid_hdr_alsize   512
> [ 5294.936796] UBI DBG gen (pid 9619): vid_hdr_offset   512
> [ 5294.936800] UBI DBG gen (pid 9619): vid_hdr_aloffset 512
> [ 5294.936804] UBI DBG gen (pid 9619): vid_hdr_shift    0
> [ 5294.936808] UBI DBG gen (pid 9619): leb_start        2048
> [ 5294.936812] UBI DBG gen (pid 9619): max_erroneous    409
> 
> So the check would only pass for vid_hdr_offset <= 512 -
> UBI_VID_HDR_SIZE; note that even specifying the default value 512 (i.e.
> 
>	ubiattach -m 0 -O 512
> 
> ) fails the check.
> 
> A less strong check would be:
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 0904eb40c95f..69c28a862430 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -666,8 +666,8 @@ static int io_init(struct ubi_device *ubi, int
> max_beb_per1024)
> 	ubi->ec_hdr_alsize = ALIGN(UBI_EC_HDR_SIZE, ubi->hdrs_min_io_size);
> 	ubi->vid_hdr_alsize = ALIGN(UBI_VID_HDR_SIZE, ubi->hdrs_min_io_size);
> 
> -	if (ubi->vid_hdr_offset && ((ubi->vid_hdr_offset + UBI_VID_HDR_SIZE) >
> -	    ubi->vid_hdr_alsize)) {
> +	if (ubi->vid_hdr_offset &&
> +	    ubi->vid_hdr_offset + UBI_VID_HDR_SIZE > ubi->peb_size) {
> 		ubi_err(ubi, "VID header offset %d too large.", ubi->vid_hdr_offset);
> 		return -EINVAL;
> 	}
> 
> But I'm unsure if this would be too lax?!

As written on IRC, 1e020e1b96af ("ubi: Fix failure attaching when vid_hdr offset equals to (sub)page size") is supposed to fix that
and on it's way into stable.

Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-17 16:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-15 15:14 [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size George Kennedy
2022-11-19 21:17 ` Richard Weinberger
2023-04-17 16:01 ` [Regression] Cannot overwrite VID header offset any more [Was: [PATCH] ubi: ensure that VID header offset + VID header size <= alloc, size] Uwe Kleine-König
2023-04-17 16:05   ` Uwe Kleine-König
2023-04-17 16:07   ` Richard Weinberger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).