2023-10-18 12:11:49

by ZhaoLong Wang

[permalink] [raw]
Subject: [PATCH v2] ubi: gluebi: Fix 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
mtd_read_oob
gluebi_read mtd->read()
gluebi->desc - NULL

Detailed reproduction information available at the link[1],

In the normal case, obtain gluebi->desc in the gluebi_get_device(),
and accesses gluebi->desc in the gluebi_read(). However,
gluebi_get_device() is not executed in advance in the
ftl_add_mtd() process, which leads to NULL pointer dereference.

The value of gluebi->desc may also be a negative error code, which
triggers the page fault error.

This patch has the following modifications:

1. Do not assign gluebi->desc to the error code. Use the NULL instead.

2. Always check the validity of gluebi->desc in gluebi_read() If the
gluebi->desc is NULL, try to get MTD device.

Such a modification currently works because the mutex "mtd_table_mutex"
is held on all necessary paths, including the ftl_add_mtd() call path,
open and close paths. Therefore, many race condition can be avoided.

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

diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c
index 1b980d15d9fb..0ca7f104adbf 100644
--- a/drivers/mtd/ubi/gluebi.c
+++ b/drivers/mtd/ubi/gluebi.c
@@ -85,6 +85,7 @@ static int gluebi_get_device(struct mtd_info *mtd)
{
struct gluebi_device *gluebi;
int ubi_mode = UBI_READONLY;
+ struct ubi_volume_desc *vdesc;

if (mtd->flags & MTD_WRITEABLE)
ubi_mode = UBI_READWRITE;
@@ -109,12 +110,14 @@ static int gluebi_get_device(struct mtd_info *mtd)
* This is the first reference to this UBI volume via the MTD device
* interface. Open the corresponding volume in read-write mode.
*/
- gluebi->desc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id,
+ vdesc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id,
ubi_mode);
- if (IS_ERR(gluebi->desc)) {
+ if (IS_ERR(vdesc)) {
+ gluebi->desc = NULL;
mutex_unlock(&devices_mutex);
- return PTR_ERR(gluebi->desc);
+ return PTR_ERR(vdesc);
}
+ gluebi->desc = vdesc;
gluebi->refcnt += 1;
mutex_unlock(&devices_mutex);
return 0;
@@ -134,8 +137,10 @@ static void gluebi_put_device(struct mtd_info *mtd)
gluebi = container_of(mtd, struct gluebi_device, mtd);
mutex_lock(&devices_mutex);
gluebi->refcnt -= 1;
- if (gluebi->refcnt == 0)
+ if (gluebi->refcnt == 0) {
ubi_close_volume(gluebi->desc);
+ gluebi->desc = NULL;
+ }
mutex_unlock(&devices_mutex);
}

@@ -154,9 +159,26 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, unsigned char *buf)
{
int err = 0, lnum, offs, bytes_left;
- struct gluebi_device *gluebi;
+ struct gluebi_device *gluebi = container_of(mtd, struct gluebi_device,
+ mtd);
+ int no_desc = gluebi->desc == NULL ? 1 : 0;
+
+ /**
+ * In normal case, the UBI volume desc has been initialized by
+ * ->_get_device(). However, in the ftl notifier process, the
+ * ->_get_device() is not executed in advance and the MTD device
+ * is directly scanned which cause NULL pointer dereference.
+ * Therefore, try to get the MTD device here.
+ */
+ if (unlikely(no_desc)) {
+ err = __get_mtd_device(mtd);
+ if (err) {
+ err_msg("cannot get MTD device %d, UBI device %d, volume %d, error %d",
+ mtd->index, gluebi->ubi_num, gluebi->vol_id, err);
+ return err;
+ }
+ }

- gluebi = container_of(mtd, struct gluebi_device, mtd);
lnum = div_u64_rem(from, mtd->erasesize, &offs);
bytes_left = len;
while (bytes_left) {
@@ -176,6 +198,9 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len,
}

*retlen = len - bytes_left;
+
+ if (unlikely(no_desc))
+ __put_mtd_device(mtd);
return err;
}

--
2.31.1


2023-10-19 01:57:47

by Zhihao Cheng

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

在 2023/10/18 20:16, 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
> mtd_read_oob
> gluebi_read mtd->read()
> gluebi->desc - NULL
>
> Detailed reproduction information available at the link[1],
>
> In the normal case, obtain gluebi->desc in the gluebi_get_device(),
> and accesses gluebi->desc in the gluebi_read(). However,
> gluebi_get_device() is not executed in advance in the
> ftl_add_mtd() process, which leads to NULL pointer dereference.
>
> The value of gluebi->desc may also be a negative error code, which
> triggers the page fault error.
>
> This patch has the following modifications:
>
> 1. Do not assign gluebi->desc to the error code. Use the NULL instead.
>
> 2. Always check the validity of gluebi->desc in gluebi_read() If the
> gluebi->desc is NULL, try to get MTD device.
>
> Such a modification currently works because the mutex "mtd_table_mutex"
> is held on all necessary paths, including the ftl_add_mtd() call path,
> open and close paths. Therefore, many race condition can be avoided.
>
> Fixes: 2ba3d76a1e29 ("UBI: make gluebi a separate module")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1]
> Signed-off-by: ZhaoLong Wang <[email protected]>
> ---
> drivers/mtd/ubi/gluebi.c | 37 +++++++++++++++++++++++++++++++------
> 1 file changed, 31 insertions(+), 6 deletions(-)
>

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

> diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c
> index 1b980d15d9fb..0ca7f104adbf 100644
> --- a/drivers/mtd/ubi/gluebi.c
> +++ b/drivers/mtd/ubi/gluebi.c
> @@ -85,6 +85,7 @@ static int gluebi_get_device(struct mtd_info *mtd)
> {
> struct gluebi_device *gluebi;
> int ubi_mode = UBI_READONLY;
> + struct ubi_volume_desc *vdesc;
>
> if (mtd->flags & MTD_WRITEABLE)
> ubi_mode = UBI_READWRITE;
> @@ -109,12 +110,14 @@ static int gluebi_get_device(struct mtd_info *mtd)
> * This is the first reference to this UBI volume via the MTD device
> * interface. Open the corresponding volume in read-write mode.
> */
> - gluebi->desc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id,
> + vdesc = ubi_open_volume(gluebi->ubi_num, gluebi->vol_id,
> ubi_mode);
> - if (IS_ERR(gluebi->desc)) {
> + if (IS_ERR(vdesc)) {
> + gluebi->desc = NULL;
> mutex_unlock(&devices_mutex);
> - return PTR_ERR(gluebi->desc);
> + return PTR_ERR(vdesc);
> }
> + gluebi->desc = vdesc;
> gluebi->refcnt += 1;
> mutex_unlock(&devices_mutex);
> return 0;
> @@ -134,8 +137,10 @@ static void gluebi_put_device(struct mtd_info *mtd)
> gluebi = container_of(mtd, struct gluebi_device, mtd);
> mutex_lock(&devices_mutex);
> gluebi->refcnt -= 1;
> - if (gluebi->refcnt == 0)
> + if (gluebi->refcnt == 0) {
> ubi_close_volume(gluebi->desc);
> + gluebi->desc = NULL;
> + }
> mutex_unlock(&devices_mutex);
> }
>
> @@ -154,9 +159,26 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len,
> size_t *retlen, unsigned char *buf)
> {
> int err = 0, lnum, offs, bytes_left;
> - struct gluebi_device *gluebi;
> + struct gluebi_device *gluebi = container_of(mtd, struct gluebi_device,
> + mtd);
> + int no_desc = gluebi->desc == NULL ? 1 : 0;
> +
> + /**
> + * In normal case, the UBI volume desc has been initialized by
> + * ->_get_device(). However, in the ftl notifier process, the
> + * ->_get_device() is not executed in advance and the MTD device
> + * is directly scanned which cause NULL pointer dereference.
> + * Therefore, try to get the MTD device here.
> + */
> + if (unlikely(no_desc)) {
> + err = __get_mtd_device(mtd);
> + if (err) {
> + err_msg("cannot get MTD device %d, UBI device %d, volume %d, error %d",
> + mtd->index, gluebi->ubi_num, gluebi->vol_id, err);
> + return err;
> + }
> + }
>
> - gluebi = container_of(mtd, struct gluebi_device, mtd);
> lnum = div_u64_rem(from, mtd->erasesize, &offs);
> bytes_left = len;
> while (bytes_left) {
> @@ -176,6 +198,9 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len,
> }
>
> *retlen = len - bytes_left;
> +
> + if (unlikely(no_desc))
> + __put_mtd_device(mtd);
> return err;
> }
>
>

2023-10-19 20:27:43

by Richard Weinberger

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

----- Ursprüngliche Mail -----
> Von: "ZhaoLong Wang" <[email protected]>
> An: "richard" <[email protected]>, "Miquel Raynal" <[email protected]>, "Vignesh Raghavendra" <[email protected]>,
> [email protected], "Artem Bityutskiy" <[email protected]>
> CC: "linux-mtd" <[email protected]>, "linux-kernel" <[email protected]>, "chengzhihao1"
> <[email protected]>, "ZhaoLong Wang" <[email protected]>, "yi zhang" <[email protected]>, "yangerkun"
> <[email protected]>
> Gesendet: Mittwoch, 18. Oktober 2023 14:16:18
> Betreff: [PATCH v2] ubi: gluebi: Fix 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
> mtd_read_oob
> gluebi_read mtd->read()
> gluebi->desc - NULL
>
> Detailed reproduction information available at the link[1],
>
> In the normal case, obtain gluebi->desc in the gluebi_get_device(),
> and accesses gluebi->desc in the gluebi_read(). However,
> gluebi_get_device() is not executed in advance in the
> ftl_add_mtd() process, which leads to NULL pointer dereference.
>
> The value of gluebi->desc may also be a negative error code, which
> triggers the page fault error.
>
> This patch has the following modifications:
>
> 1. Do not assign gluebi->desc to the error code. Use the NULL instead.
>
> 2. Always check the validity of gluebi->desc in gluebi_read() If the
> gluebi->desc is NULL, try to get MTD device.
>
> Such a modification currently works because the mutex "mtd_table_mutex"
> is held on all necessary paths, including the ftl_add_mtd() call path,
> open and close paths. Therefore, many race condition can be avoided.

I see the problem, but I'm not really satisfied by the solution.
Adding this hack to gluebi_read() is not nice at all.

Is there a strong reason why have to defer ubi_open_volume() to
gluebi_get_device()?

Miquel, what do you think?

Thanks,
//richard

2023-10-20 02:28:54

by Zhihao Cheng

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

在 2023/10/20 4:27, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "ZhaoLong Wang" <[email protected]>
>> An: "richard" <[email protected]>, "Miquel Raynal" <[email protected]>, "Vignesh Raghavendra" <[email protected]>,
>> [email protected], "Artem Bityutskiy" <[email protected]>
>> CC: "linux-mtd" <[email protected]>, "linux-kernel" <[email protected]>, "chengzhihao1"
>> <[email protected]>, "ZhaoLong Wang" <[email protected]>, "yi zhang" <[email protected]>, "yangerkun"
>> <[email protected]>
>> Gesendet: Mittwoch, 18. Oktober 2023 14:16:18
>> Betreff: [PATCH v2] ubi: gluebi: Fix 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
>> mtd_read_oob
>> gluebi_read mtd->read()
>> gluebi->desc - NULL
>>
>> Detailed reproduction information available at the link[1],
>>
>> In the normal case, obtain gluebi->desc in the gluebi_get_device(),
>> and accesses gluebi->desc in the gluebi_read(). However,
>> gluebi_get_device() is not executed in advance in the
>> ftl_add_mtd() process, which leads to NULL pointer dereference.
>>
>> The value of gluebi->desc may also be a negative error code, which
>> triggers the page fault error.
>>
>> This patch has the following modifications:
>>
>> 1. Do not assign gluebi->desc to the error code. Use the NULL instead.
>>
>> 2. Always check the validity of gluebi->desc in gluebi_read() If the
>> gluebi->desc is NULL, try to get MTD device.
>>
>> Such a modification currently works because the mutex "mtd_table_mutex"
>> is held on all necessary paths, including the ftl_add_mtd() call path,
>> open and close paths. Therefore, many race condition can be avoided.
>
> I see the problem, but I'm not really satisfied by the solution.
> Adding this hack to gluebi_read() is not nice at all.

Yes, it's jsut a workaround. At the begining, I prefer that increasing
volume refcnt (by ubi_open_volume) in gluebi_create and releasing volume
refcnt in gluebi_remove. It looks more reasonable that holding a refcnt
of UBI volume when gluebi is alive. After looking through the code, the
creation/destroying of gluebi is triggered by volume
actions(UBI_VOLUME_ADDED/UBI_VOLUME_REMOVED), which means that:
1. gluebi_remove is depended on UBI_VOLUME_REMOVED(triggered by
ubi_remove_volume)
2. ubi_remove_volume won't be executed until the refcnt of volume
becomes 0(released by gluebi_remove)

If we add new ioctls to control creation/destroying of gluebi, then
gluebi mtd won't be automatically created when UBI volume is added. I'm
not certain whether this change will effect existing startup process
that depends on gluebi.

>
> Is there a strong reason why have to defer ubi_open_volume() to
> gluebi_get_device()?
>
> Miquel, what do you think?
>
> Thanks,
> //richard
>
> .
>

2023-10-21 16:10:01

by Richard Weinberger

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

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <[email protected]>
>>> Such a modification currently works because the mutex "mtd_table_mutex"
>>> is held on all necessary paths, including the ftl_add_mtd() call path,
>>> open and close paths. Therefore, many race condition can be avoided.
>>
>> I see the problem, but I'm not really satisfied by the solution.
>> Adding this hack to gluebi_read() is not nice at all.
>
> Yes, it's jsut a workaround. At the begining, I prefer that increasing
> volume refcnt (by ubi_open_volume) in gluebi_create and releasing volume
> refcnt in gluebi_remove. It looks more reasonable that holding a refcnt
> of UBI volume when gluebi is alive. After looking through the code, the
> creation/destroying of gluebi is triggered by volume
> actions(UBI_VOLUME_ADDED/UBI_VOLUME_REMOVED), which means that:
> 1. gluebi_remove is depended on UBI_VOLUME_REMOVED(triggered by
> ubi_remove_volume)
> 2. ubi_remove_volume won't be executed until the refcnt of volume
> becomes 0(released by gluebi_remove)
>
> If we add new ioctls to control creation/destroying of gluebi, then
> gluebi mtd won't be automatically created when UBI volume is added. I'm
> not certain whether this change will effect existing startup process
> that depends on gluebi.

Let's take a stack back. The sole purpose of gluebi is providing
a way to run JFFS2 on top of UBI.
IMHO there is no need to run an FTL on top of UBI or even mtdblock.
This kind of stacking does not make sense.

So, I'd go so far and propose the following:
diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index ff18636e08897..b362a64411ebd 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)

IOW, no mtdblock (hence, also no FTLs) on top of gluebi.

What do you guys think?

Thanks,
//richard

2023-10-23 06:42:00

by ZhaoLong Wang

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

>
> IOW, no mtdblock (hence, also no FTLs) on top of gluebi.
>
> What do you guys think?
>
> Thanks,
> //richard
>


JFFS2 needs to work on the block device, so the mtdblock needs to work
on the top of gluebi.

This is directly reflected in the jffs2 mount command, such as

# mount -t jffs2 /dev/mtdblockX /mnt/jffs2

2023-10-23 06:46:37

by Richard Weinberger

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

----- Ursprüngliche Mail -----
> Von: "ZhaoLong Wang" <[email protected]>
> An: "richard" <[email protected]>, "chengzhihao1" <[email protected]>
> CC: "Miquel Raynal" <[email protected]>, "Vignesh Raghavendra" <[email protected]>, "dpervushin"
> <[email protected]>, "Artem Bityutskiy" <[email protected]>, "linux-mtd"
> <[email protected]>, "linux-kernel" <[email protected]>, "yi zhang" <[email protected]>,
> "yangerkun" <[email protected]>
> Gesendet: Montag, 23. Oktober 2023 08:41:12
> Betreff: Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier

>>
>> IOW, no mtdblock (hence, also no FTLs) on top of gluebi.
>>
>> What do you guys think?
>>
>> Thanks,
>> //richard
>>
>
>
> JFFS2 needs to work on the block device, so the mtdblock needs to work
> on the top of gluebi.
>
> This is directly reflected in the jffs2 mount command, such as
>
> # mount -t jffs2 /dev/mtdblockX /mnt/jffs2

The limitation is long gone. It comes from the dark and old times
where Linux was only able to mount block devices.

You can do: mount -t jffs2 mtdX /mnt/jffs2

Thanks,
//richard

2023-10-23 07:10:05

by Zhihao Cheng

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

在 2023/10/22 0:09, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "chengzhihao1" <[email protected]>
>>>> Such a modification currently works because the mutex "mtd_table_mutex"
>>>> is held on all necessary paths, including the ftl_add_mtd() call path,
>>>> open and close paths. Therefore, many race condition can be avoided.
>>>
>>> I see the problem, but I'm not really satisfied by the solution.
>>> Adding this hack to gluebi_read() is not nice at all.
>>
>> Yes, it's jsut a workaround. At the begining, I prefer that increasing
>> volume refcnt (by ubi_open_volume) in gluebi_create and releasing volume
>> refcnt in gluebi_remove. It looks more reasonable that holding a refcnt
>> of UBI volume when gluebi is alive. After looking through the code, the
>> creation/destroying of gluebi is triggered by volume
>> actions(UBI_VOLUME_ADDED/UBI_VOLUME_REMOVED), which means that:
>> 1. gluebi_remove is depended on UBI_VOLUME_REMOVED(triggered by
>> ubi_remove_volume)
>> 2. ubi_remove_volume won't be executed until the refcnt of volume
>> becomes 0(released by gluebi_remove)
>>
>> If we add new ioctls to control creation/destroying of gluebi, then
>> gluebi mtd won't be automatically created when UBI volume is added. I'm
>> not certain whether this change will effect existing startup process
>> that depends on gluebi.
>
> Let's take a stack back. The sole purpose of gluebi is providing
> a way to run JFFS2 on top of UBI.

Is it possible that someone runs ext4 on mtdblock based on gluebi, for
the advantage of wear-leveling?

> IMHO there is no need to run an FTL on top of UBI or even mtdblock.
> This kind of stacking does not make sense.
>
> So, I'd go so far and propose the following:
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index ff18636e08897..b362a64411ebd 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)
>
> IOW, no mtdblock (hence, also no FTLs) on top of gluebi.
>
> What do you guys think?
>
> Thanks,
> //richard
>
> .
>

2023-10-23 07:12:51

by ZhaoLong Wang

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


> ----- Ursprüngliche Mail -----
>> Von: "ZhaoLong Wang" <[email protected]>
>> An: "richard" <[email protected]>, "chengzhihao1" <[email protected]>
>> CC: "Miquel Raynal" <[email protected]>, "Vignesh Raghavendra" <[email protected]>, "dpervushin"
>> <[email protected]>, "Artem Bityutskiy" <[email protected]>, "linux-mtd"
>> <[email protected]>, "linux-kernel" <[email protected]>, "yi zhang" <[email protected]>,
>> "yangerkun" <[email protected]>
>> Gesendet: Montag, 23. Oktober 2023 08:41:12
>> Betreff: Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
>
>>>
>>> IOW, no mtdblock (hence, also no FTLs) on top of gluebi.
>>>
>>> What do you guys think?
>>>
>>> Thanks,
>>> //richard
>>>
>>
>>
>> JFFS2 needs to work on the block device, so the mtdblock needs to work
>> on the top of gluebi.
>>
>> This is directly reflected in the jffs2 mount command, such as
>>
>> # mount -t jffs2 /dev/mtdblockX /mnt/jffs2
>
> The limitation is long gone. It comes from the dark and old times
> where Linux was only able to mount block devices.
>
> You can do: mount -t jffs2 mtdX /mnt/jffs2
>
> Thanks,
> //richard
>

Yes, I tried it and it worked. Thanks for the reminder.

But this modification rejects this usage. and rejects other block device
file systems (such as ext4) from working on gluebi.

Thank you.
Zhaolong

2023-10-23 07:15:54

by Richard Weinberger

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

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <[email protected]>
>>> If we add new ioctls to control creation/destroying of gluebi, then
>>> gluebi mtd won't be automatically created when UBI volume is added. I'm
>>> not certain whether this change will effect existing startup process
>>> that depends on gluebi.
>>
>> Let's take a stack back. The sole purpose of gluebi is providing
>> a way to run JFFS2 on top of UBI.
>
> Is it possible that someone runs ext4 on mtdblock based on gluebi, for
> the advantage of wear-leveling?

Unless they want to kill their NAND immediately, no.
UBIblock hat initially an RW-Mode but it was rejected because
UBI is not an FTL and will all dangerous setups.

What I'm trying to say is, I'd like to avoid adding complicated solutions
or workarounds just to make artificial stacking of outdated MTD components
possible.
Let's keep the big picture in mind.

Thanks,
//richard

2023-10-23 07:16:49

by Richard Weinberger

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

----- Ursprüngliche Mail -----
> Von: "ZhaoLong Wang" <[email protected]>
> But this modification rejects this usage. and rejects other block device
> file systems (such as ext4) from working on gluebi.

Are you using ext4 on top of gluebi?
Please explain in detail why.

Thanks,
//richard

2023-10-23 07:36:59

by Zhihao Cheng

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

在 2023/10/23 15:15, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "chengzhihao1" <[email protected]>
>>>> If we add new ioctls to control creation/destroying of gluebi, then
>>>> gluebi mtd won't be automatically created when UBI volume is added. I'm
>>>> not certain whether this change will effect existing startup process
>>>> that depends on gluebi.
>>> Let's take a stack back. The sole purpose of gluebi is providing
>>> a way to run JFFS2 on top of UBI.
>> Is it possible that someone runs ext4 on mtdblock based on gluebi, for
>> the advantage of wear-leveling?
> Unless they want to kill their NAND immediately, no.
> UBIblock hat initially an RW-Mode but it was rejected because
> UBI is not an FTL and will all dangerous setups.
>
> What I'm trying to say is, I'd like to avoid adding complicated solutions
> or workarounds just to make artificial stacking of outdated MTD components
> possible.
> Let's keep the big picture in mind.

Makes sense. Zhaolong and I didn't make clear the boundary between FTL
and ubiblock/gluebi. Now, I believe that UBI needn't be responsible much
for the extension mechanism(eg. mtdblock based on gluebi). So, let's
pick your solution.

> Thanks,
> //richard
>
> .