2023-10-27 01:16:14

by ZhaoLong Wang

[permalink] [raw]
Subject: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

If both flt.ko and gluebi.ko are loaded, the notiier of ftl
triggers NULL pointer dereference when trying to access
‘gluebi->desc’ in gluebi_read().

ubi_gluebi_init
ubi_register_volume_notifier
ubi_enumerate_volumes
ubi_notify_all
gluebi_notify nb->notifier_call()
gluebi_create
mtd_device_register
mtd_device_parse_register
add_mtd_device
blktrans_notify_add not->add()
ftl_add_mtd tr->add_mtd()
scan_header
mtd_read
mtd_read_oob
mtd_read_oob_std
gluebi_read mtd->read()
gluebi->desc - NULL

Detailed reproduction information available at the link[1],

The solution for the gluebi module is to run jffs2 on the UBI
volume without considering working with ftl or mtdblock.[2].
Therefore, this problem can be avoided by preventing gluebi
from creating mtdblock devices.

Fixes: 2ba3d76a1e29 ("UBI: make gluebi a separate module")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1]
Link: https://lore.kernel.org/lkml/[email protected]/ [2]
Signed-off-by: ZhaoLong Wang <[email protected]>
---
drivers/mtd/mtd_blkdevs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index ff18636e0889..5bc32108ca03 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -463,7 +463,7 @@ static void blktrans_notify_add(struct mtd_info *mtd)
{
struct mtd_blktrans_ops *tr;

- if (mtd->type == MTD_ABSENT)
+ if (mtd->type == MTD_ABSENT || mtd->type == MTD_UBIVOLUME)
return;

list_for_each_entry(tr, &blktrans_majors, list)
@@ -503,7 +503,7 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
mutex_lock(&mtd_table_mutex);
list_add(&tr->list, &blktrans_majors);
mtd_for_each_device(mtd)
- if (mtd->type != MTD_ABSENT)
+ if (mtd->type != MTD_ABSENT && mtd->type != MTD_UBIVOLUME)
tr->add_mtd(tr, mtd);
mutex_unlock(&mtd_table_mutex);
return 0;
--
2.31.1


2023-10-27 03:24:13

by Zhihao Cheng

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

在 2023/10/27 9:20, ZhaoLong Wang 写道:
> If both flt.ko and gluebi.ko are loaded, the notiier of ftl
> triggers NULL pointer dereference when trying to access
> ‘gluebi->desc’ in gluebi_read().
>
> ubi_gluebi_init
> ubi_register_volume_notifier
> ubi_enumerate_volumes
> ubi_notify_all
> gluebi_notify nb->notifier_call()
> gluebi_create
> mtd_device_register
> mtd_device_parse_register
> add_mtd_device
> blktrans_notify_add not->add()
> ftl_add_mtd tr->add_mtd()
> scan_header
> mtd_read
> mtd_read_oob
> mtd_read_oob_std
> gluebi_read mtd->read()
> gluebi->desc - NULL
>
> Detailed reproduction information available at the link[1],
>
> The solution for the gluebi module is to run jffs2 on the UBI
> volume without considering working with ftl or mtdblock.[2].
> Therefore, this problem can be avoided by preventing gluebi
> from creating mtdblock devices.
>
> Fixes: 2ba3d76a1e29 ("UBI: make gluebi a separate module")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1]
> Link: https://lore.kernel.org/lkml/[email protected]/ [2]
> Signed-off-by: ZhaoLong Wang <[email protected]>
> ---
> drivers/mtd/mtd_blkdevs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Zhihao Cheng <[email protected]>


2023-10-27 17:42:37

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

Hi ZhaoLong,

[email protected] wrote on Fri, 27 Oct 2023 09:20:33 +0800:

> If both flt.ko and gluebi.ko are loaded, the notiier of ftl

flt ? notifier

> triggers NULL pointer dereference when trying to access
> ‘gluebi->desc’ in gluebi_read().
>
> ubi_gluebi_init
> ubi_register_volume_notifier
> ubi_enumerate_volumes
> ubi_notify_all
> gluebi_notify nb->notifier_call()
> gluebi_create
> mtd_device_register
> mtd_device_parse_register
> add_mtd_device
> blktrans_notify_add not->add()
> ftl_add_mtd tr->add_mtd()

Glitches?

> scan_header
> mtd_read
> mtd_read_oob
> mtd_read_oob_std
> gluebi_read mtd->read()
> gluebi->desc - NULL
>
> Detailed reproduction information available at the link[1],
>
> The solution for the gluebi module is to run jffs2 on the UBI
> volume without considering working with ftl or mtdblock.[2].

I am sorry but ftl, gluebi, mtdblock, jffs2 and ubi in the same report
seem a little bit fuzzy. Are you sure about this sentence?

> Therefore, this problem can be avoided by preventing gluebi
> from creating mtdblock devices.

This sentence sounds wrong :)

> Fixes: 2ba3d76a1e29 ("UBI: make gluebi a separate module")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1]
> Link: https://lore.kernel.org/lkml/[email protected]/ [2]
> Signed-off-by: ZhaoLong Wang <[email protected]>
> ---
> drivers/mtd/mtd_blkdevs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index ff18636e0889..5bc32108ca03 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -463,7 +463,7 @@ static void blktrans_notify_add(struct mtd_info *mtd)
> {
> struct mtd_blktrans_ops *tr;
>
> - if (mtd->type == MTD_ABSENT)
> + if (mtd->type == MTD_ABSENT || mtd->type == MTD_UBIVOLUME)
> return;
>
> list_for_each_entry(tr, &blktrans_majors, list)
> @@ -503,7 +503,7 @@ int register_mtd_blktrans(struct mtd_blktrans_ops *tr)
> mutex_lock(&mtd_table_mutex);
> list_add(&tr->list, &blktrans_majors);
> mtd_for_each_device(mtd)
> - if (mtd->type != MTD_ABSENT)
> + if (mtd->type != MTD_ABSENT && mtd->type != MTD_UBIVOLUME)
> tr->add_mtd(tr, mtd);
> mutex_unlock(&mtd_table_mutex);
> return 0;


Thanks,
Miquèl

2023-10-27 20:32:20

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <[email protected]>
>> Detailed reproduction information available at the link[1],
>>
>> The solution for the gluebi module is to run jffs2 on the UBI
>> volume without considering working with ftl or mtdblock.[2].
>
> I am sorry but ftl, gluebi, mtdblock, jffs2 and ubi in the same report
> seem a little bit fuzzy. Are you sure about this sentence?

ZhaoLong Wang found an interesting bug while stacking various MTD components.
I suggested to just deny mtdblock on top of gluebi to avoid the whole
problem class instead of adding more duct tape.

Thanks,
//richard

The issue reminds me of one of my favorite Monty Python sketches, https://www.youtube.com/watch?v=LFrdqQZ8FFc
;-)

2023-10-29 13:50:58

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

Hi Richard,

[email protected] wrote on Fri, 27 Oct 2023 22:30:10 +0200 (CEST):

> ----- Ursprüngliche Mail -----
> > Von: "Miquel Raynal" <[email protected]>
> >> Detailed reproduction information available at the link[1],
> >>
> >> The solution for the gluebi module is to run jffs2 on the UBI
> >> volume without considering working with ftl or mtdblock.[2].
> >
> > I am sorry but ftl, gluebi, mtdblock, jffs2 and ubi in the same report
> > seem a little bit fuzzy. Are you sure about this sentence?
>
> ZhaoLong Wang found an interesting bug while stacking various MTD components.
> I suggested to just deny mtdblock on top of gluebi to avoid the whole
> problem class instead of adding more duct tape.

Ack. I'll queue this once the log fixed and your official Ack-by
received for mtd/next after -rc1 is out.

Thanks,
Miquèl

>
> Thanks,
> //richard
>
> The issue reminds me of one of my favorite Monty Python sketches, https://www.youtube.com/watch?v=LFrdqQZ8FFc
> ;-)

:-)

2023-10-29 15:40:06

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

----- Ursprüngliche Mail -----
> Von: "Miquel Raynal" <[email protected]>
> Ack. I'll queue this once the log fixed and your official Ack-by
> received for mtd/next after -rc1 is out.

Acked-by: Richard Weinberger <[email protected]>

Thanks,
//richard

2023-12-19 14:38:34

by ZhaoLong Wang

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

Friendly ping...

I'd like to know what the current status of this patch is. Is there a
need for further programme improvements?

Bast regards,
ZhaoLong Wang


> ----- Ursprüngliche Mail -----
>> Von: "Miquel Raynal" <[email protected]>
>> Ack. I'll queue this once the log fixed and your official Ack-by
>> received for mtd/next after -rc1 is out.
>
> Acked-by: Richard Weinberger <[email protected]>
>
> Thanks,
> //richard
>


2023-12-19 14:45:05

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

Hi ZhaoLong,

[email protected] wrote on Tue, 19 Dec 2023 22:36:50 +0800:

> Friendly ping...
>
> I'd like to know what the current status of this patch is. Is there a need for further programme improvements?

Easy, see below.

>
> Bast regards,
> ZhaoLong Wang
>
>
> > ----- Ursprüngliche Mail -----
> >> Von: "Miquel Raynal" <[email protected]>
> >> Ack. I'll queue this once the log fixed and your official Ack-by
> >> received for mtd/next after -rc1 is out.

Here I depict two conditions.

> > Acked-by: Richard Weinberger <[email protected]>

And here one of them is fulfilled, which means the other one is still
not AFAIK :)

Thanks,
Miquèl

2023-12-20 02:15:58

by ZhaoLong Wang

[permalink] [raw]
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by ftl notifier

Oh, sorry, I had missed a crucial part of your previous message,
specifically the segment about the `flt ? notifier,This oversight
led to my misunderstanding of the situation, and for that, I am
truly sorry.

Thank you for your understanding and patience. I am committed to
maintaining clear and accurate communication going forward.

I'll send a v4 version of the log fixed patch soon.

Warmest thanks,
ZhaoLong Wang

> Hi ZhaoLong,
>
> [email protected] wrote on Tue, 19 Dec 2023 22:36:50 +0800:
>
>> Friendly ping...
>>
>> I'd like to know what the current status of this patch is. Is there a need for further programme improvements?
>
> Easy, see below.
>
>>
>> Bast regards,
>> ZhaoLong Wang
>>
>>
>>> ----- Ursprüngliche Mail -----
>>>> Von: "Miquel Raynal" <[email protected]>
>>>> Ack. I'll queue this once the log fixed and your official Ack-by
>>>> received for mtd/next after -rc1 is out.
>
> Here I depict two conditions.
>
>>> Acked-by: Richard Weinberger <[email protected]>
>
> And here one of them is fulfilled, which means the other one is still
> not AFAIK :)
>
> Thanks,
> Miquèl