[PATCH] ubi: Reject device with erasesize 0

[PATCH] ubi: Reject device with erasesize 0

[Jan Kara]

In principle MTD device with erasesize 0 can exist and it is possible to
create them e.g. via KVM. If that happens UBI layer currently crashes
with:

ubi7: attaching mtd147
divide error: 0000 [#1] PREEMPT SMP KASAN
CPU: 0 PID: 20023 Comm: syz-executor.0 Not tainted 6.2.0 #6
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.13.0-1ubuntu1.1 04/01/2014
RIP: 0010:mtd_div_by_eb include/linux/mtd/mtd.h:580 [inline]
RIP: 0010:io_init drivers/mtd/ubi/build.c:620 [inline]
RIP: 0010:ubi_attach_mtd_dev+0x77f/0x2fe0 drivers/mtd/ubi/build.c:955
Code: fc ff df 48 c1 ea 03 0f b6 14 02 4c 89 f0 83 e0 07 83 c0 03 38
d0 7c 08 84 d2 0f 85 1f 25 00 00 41 8b 4c 24 10 48 89 d8 31 d2 <48> f7
f1 48 89 c3 e8 b6 f3 1b fc 48 8d 85 40 17 00 00 48 89 c2 48
RSP: 0018:ffffc9000be0fd30 EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffff888047a49d40 RDI: 0000000000000002
RBP: ffff888024e1c000 R08: 0000000000000016 R09: fffff520017c1f47
R10: ffffc9000be0fa37 R11: fffff520017c1f46 R12: ffff88806545a000
R13: 0000000000000000 R14: ffff88806545a010 R15: 0000000000000007
FS:  00007fd45e85c700(0000) GS:ffff88802ca00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f64aeef53a4 CR3: 000000004f39a000 CR4: 0000000000350ef0
Call Trace:
 <TASK>
 ctrl_cdev_ioctl+0x303/0x3a0 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+0x198/0x210 fs/ioctl.c:856
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd

Handle such devices gracefully and just reject attaching UBI to them
instead of crashing.

Reported-by: Yu Hao <yhao016@ucr.edu>
Link: https://lore.kernel.org/all/CA+UBctDsHRpkLG5ppdiuV8Msn4Dx-ZJ2xDrxfa48VMb7ZE+xBA@mail.gmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
---
 drivers/mtd/ubi/build.c | 3 +++
 1 file changed, 3 insertions(+)

It doesn't seem the discussion linked above concluded in a patch. So is
anything wrong with the trivial approach here so that we can close the issue?

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 8b91a55ec0d2..684273e13efb 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -613,6 +613,9 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
 	if (ubi->vid_hdr_offset < 0)
 		return -EINVAL;
 
+	if (ubi->mtd->erasesize == 0)
+		return -EINVAL;
+
 	/*
 	 * Note, in this implementation we support MTD devices with 0x7FFFFFFF
 	 * physical eraseblocks maximum.
-- 
2.35.3


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

Re: [PATCH] ubi: Reject device with erasesize 0

[Zhihao Cheng]

在 2023/8/31 19:11, Jan Kara 写道:
Hi Jan,
> In principle MTD device with erasesize 0 can exist and it is possible to
> create them e.g. via KVM. If that happens UBI layer currently crashes
> with:
> 
> ubi7: attaching mtd147
> divide error: 0000 [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 20023 Comm: syz-executor.0 Not tainted 6.2.0 #6
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:mtd_div_by_eb include/linux/mtd/mtd.h:580 [inline]
> RIP: 0010:io_init drivers/mtd/ubi/build.c:620 [inline]
> RIP: 0010:ubi_attach_mtd_dev+0x77f/0x2fe0 drivers/mtd/ubi/build.c:955
> Code: fc ff df 48 c1 ea 03 0f b6 14 02 4c 89 f0 83 e0 07 83 c0 03 38
> d0 7c 08 84 d2 0f 85 1f 25 00 00 41 8b 4c 24 10 48 89 d8 31 d2 <48> f7
> f1 48 89 c3 e8 b6 f3 1b fc 48 8d 85 40 17 00 00 48 89 c2 48
> RSP: 0018:ffffc9000be0fd30 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffff888047a49d40 RDI: 0000000000000002
> RBP: ffff888024e1c000 R08: 0000000000000016 R09: fffff520017c1f47
> R10: ffffc9000be0fa37 R11: fffff520017c1f46 R12: ffff88806545a000
> R13: 0000000000000000 R14: ffff88806545a010 R15: 0000000000000007
> FS:  00007fd45e85c700(0000) GS:ffff88802ca00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f64aeef53a4 CR3: 000000004f39a000 CR4: 0000000000350ef0
> Call Trace:
>   <TASK>
>   ctrl_cdev_ioctl+0x303/0x3a0 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+0x198/0x210 fs/ioctl.c:856
>   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>   do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
>   entry_SYSCALL_64_after_hwframe+0x63/0xcd
> 
> Handle such devices gracefully and just reject attaching UBI to them
> instead of crashing.
> 
> Reported-by: Yu Hao <yhao016@ucr.edu>
> Link: https://lore.kernel.org/all/CA+UBctDsHRpkLG5ppdiuV8Msn4Dx-ZJ2xDrxfa48VMb7ZE+xBA@mail.gmail.com
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>   drivers/mtd/ubi/build.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> It doesn't seem the discussion linked above concluded in a patch. So is
> anything wrong with the trivial approach here so that we can close the issue?

There was a patch, see 
https://patchwork.ozlabs.org/project/linux-mtd/patch/20230423111041.684297-1-chengzhihao1@huawei.com/


> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 8b91a55ec0d2..684273e13efb 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -613,6 +613,9 @@ static int io_init(struct ubi_device *ubi, int max_beb_per1024)
>   	if (ubi->vid_hdr_offset < 0)
>   		return -EINVAL;
>   
> +	if (ubi->mtd->erasesize == 0)
> +		return -EINVAL;
> +
>   	/*
>   	 * Note, in this implementation we support MTD devices with 0x7FFFFFFF
>   	 * physical eraseblocks maximum.
> 


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

Re: [PATCH] ubi: Reject device with erasesize 0

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>> https://lore.kernel.org/all/CA+UBctDsHRpkLG5ppdiuV8Msn4Dx-ZJ2xDrxfa48VMb7ZE+xBA@mail.gmail.com
>> Signed-off-by: Jan Kara <jack@suse.cz>
>> ---
>>   drivers/mtd/ubi/build.c | 3 +++
>>   1 file changed, 3 insertions(+)
>> 
>> It doesn't seem the discussion linked above concluded in a patch. So is
>> anything wrong with the trivial approach here so that we can close the issue?
> 
> There was a patch, see
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20230423111041.684297-1-chengzhihao1@huawei.com/

Hmm, let me check the patch status of that one.

Thanks,
//richard

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

Re: [PATCH] ubi: Reject device with erasesize 0

[Jan Kara]

On Fri 01-09-23 09:11:26, Zhihao Cheng wrote:
> 在 2023/8/31 19:11, Jan Kara 写道:
> Hi Jan,
> > In principle MTD device with erasesize 0 can exist and it is possible to
> > create them e.g. via KVM. If that happens UBI layer currently crashes
> > with:
> > 
> > ubi7: attaching mtd147
> > divide error: 0000 [#1] PREEMPT SMP KASAN
> > CPU: 0 PID: 20023 Comm: syz-executor.0 Not tainted 6.2.0 #6
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > 1.13.0-1ubuntu1.1 04/01/2014
> > RIP: 0010:mtd_div_by_eb include/linux/mtd/mtd.h:580 [inline]
> > RIP: 0010:io_init drivers/mtd/ubi/build.c:620 [inline]
> > RIP: 0010:ubi_attach_mtd_dev+0x77f/0x2fe0 drivers/mtd/ubi/build.c:955
> > Code: fc ff df 48 c1 ea 03 0f b6 14 02 4c 89 f0 83 e0 07 83 c0 03 38
> > d0 7c 08 84 d2 0f 85 1f 25 00 00 41 8b 4c 24 10 48 89 d8 31 d2 <48> f7
> > f1 48 89 c3 e8 b6 f3 1b fc 48 8d 85 40 17 00 00 48 89 c2 48
> > RSP: 0018:ffffc9000be0fd30 EFLAGS: 00010246
> > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> > RDX: 0000000000000000 RSI: ffff888047a49d40 RDI: 0000000000000002
> > RBP: ffff888024e1c000 R08: 0000000000000016 R09: fffff520017c1f47
> > R10: ffffc9000be0fa37 R11: fffff520017c1f46 R12: ffff88806545a000
> > R13: 0000000000000000 R14: ffff88806545a010 R15: 0000000000000007
> > FS:  00007fd45e85c700(0000) GS:ffff88802ca00000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f64aeef53a4 CR3: 000000004f39a000 CR4: 0000000000350ef0
> > Call Trace:
> >   <TASK>
> >   ctrl_cdev_ioctl+0x303/0x3a0 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+0x198/0x210 fs/ioctl.c:856
> >   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> >   do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
> >   entry_SYSCALL_64_after_hwframe+0x63/0xcd
> > 
> > Handle such devices gracefully and just reject attaching UBI to them
> > instead of crashing.
> > 
> > Reported-by: Yu Hao <yhao016@ucr.edu>
> > Link: https://lore.kernel.org/all/CA+UBctDsHRpkLG5ppdiuV8Msn4Dx-ZJ2xDrxfa48VMb7ZE+xBA@mail.gmail.com
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >   drivers/mtd/ubi/build.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > It doesn't seem the discussion linked above concluded in a patch. So is
> > anything wrong with the trivial approach here so that we can close the issue?
> 
> There was a patch, see
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20230423111041.684297-1-chengzhihao1@huawei.com/

Ah, OK, I wasn't able to find it through lore search. Thanks for the
pointer! Hopefully Richard will merge it now.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

Re: [PATCH] ubi: Reject device with erasesize 0

[Tudor Ambarus]

Hi, Richard,

On 8/31/23 12:11, Jan Kara wrote:
> In principle MTD device with erasesize 0 can exist and it is possible to create them e.g. via KVM. If that happens UBI layer currently crashes

mtd devices are organized in erase blocks, having a mtd with erase
size zero looks suspicious to me. Do you know if there are any mtd
devices with erasesize of zero?

Cheers,
ta

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

Re: [PATCH] ubi: Reject device with erasesize 0

[Jan Kara]

On Fri 01-09-23 10:25:11, Tudor Ambarus wrote:
> Hi, Richard,
> 
> On 8/31/23 12:11, Jan Kara wrote:
> > In principle MTD device with erasesize 0 can exist and it is possible to create them e.g. via KVM. If that happens UBI layer currently crashes
> 
> mtd devices are organized in erase blocks, having a mtd with erase
> size zero looks suspicious to me. Do you know if there are any mtd
> devices with erasesize of zero?

Please see the discussion I've referenced from the patch. I agree such
devices are strange but apparently devices reporting erasesize==0 do exist
and can be created in virtualized environment. IMO we don't need to work
with them but we shouldn't crash a kernel if they are attached...

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

Re: [PATCH] ubi: Reject device with erasesize 0

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Tudor Ambarus" <tudor.ambarus@linaro.org>
> An: "Jan Kara" <jack@suse.cz>, "richard" <richard@nod.at>, "Miquel Raynal" <miquel.raynal@bootlin.com>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "chengzhihao1" <chengzhihao1@huawei.com>, "Yu Hao" <yhao016@ucr.edu>
> Gesendet: Freitag, 1. September 2023 11:25:11
> Betreff: Re: [PATCH] ubi: Reject device with erasesize 0

> Hi, Richard,
> 
> On 8/31/23 12:11, Jan Kara wrote:
>> In principle MTD device with erasesize 0 can exist and it is possible to create
>> them e.g. via KVM. If that happens UBI layer currently crashes
> 
> mtd devices are organized in erase blocks, having a mtd with erase
> size zero looks suspicious to me. Do you know if there are any mtd
> devices with erasesize of zero?

Yes. There are. So, the fix is fine.

Thanks,
//richard

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

Re: [PATCH] ubi: Reject device with erasesize 0

[Tudor Ambarus]

On 9/1/23 10:37, Jan Kara wrote:
> On Fri 01-09-23 10:25:11, Tudor Ambarus wrote:
>> Hi, Richard,
>>
>> On 8/31/23 12:11, Jan Kara wrote:
>>> In principle MTD device with erasesize 0 can exist and it is possible to create them e.g. via KVM. If that happens UBI layer currently crashes
>>
>> mtd devices are organized in erase blocks, having a mtd with erase
>> size zero looks suspicious to me. Do you know if there are any mtd
>> devices with erasesize of zero?
> 
> Please see the discussion I've referenced from the patch. I agree such
> devices are strange but apparently devices reporting erasesize==0 do exist
> and can be created in virtualized environment. IMO we don't need to work
> with them but we shouldn't crash a kernel if they are attached...
> 
> 								Honza
> 

Thanks, Jan. I agree we shouldn't crash the kernel. I was wondering
whether on the long term we should aim to not allow the creation of
mtd devices with erasesize zero, or/and to not accept devices with
erasesize of zero under mtd. Was curious about the why, I don't have
anything against the patch. I guess I'll answer my questions myself
if I ever stumble on this.

Cheers,
ta

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