[PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier

[PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier

[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 <wangzhaolong1@huawei.com>
---
 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


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

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

[Zhihao Cheng]

在 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 <wangzhaolong1@huawei.com>
> ---
>   drivers/mtd/ubi/gluebi.c | 37 +++++++++++++++++++++++++++++++------
>   1 file changed, 31 insertions(+), 6 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>

> 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;
>   }
>   
> 


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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "ZhaoLong Wang" <wangzhaolong1@huawei.com>
> An: "richard" <richard@nod.at>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>,
> dpervushin@embeddedalley.com, "Artem Bityutskiy" <Artem.Bityutskiy@nokia.com>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "chengzhihao1"
> <chengzhihao1@huawei.com>, "ZhaoLong Wang" <wangzhaolong1@huawei.com>, "yi zhang" <yi.zhang@huawei.com>, "yangerkun"
> <yangerkun@huawei.com>
> 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

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

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

[Zhihao Cheng]

在 2023/10/20 4:27, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "ZhaoLong Wang" <wangzhaolong1@huawei.com>
>> An: "richard" <richard@nod.at>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>,
>> dpervushin@embeddedalley.com, "Artem Bityutskiy" <Artem.Bityutskiy@nokia.com>
>> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "chengzhihao1"
>> <chengzhihao1@huawei.com>, "ZhaoLong Wang" <wangzhaolong1@huawei.com>, "yi zhang" <yi.zhang@huawei.com>, "yangerkun"
>> <yangerkun@huawei.com>
>> 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
> 
> .
> 


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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>>> 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

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

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

[ZhaoLong Wang]

> 
> 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

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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "ZhaoLong Wang" <wangzhaolong1@huawei.com>
> An: "richard" <richard@nod.at>, "chengzhihao1" <chengzhihao1@huawei.com>
> CC: "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>, "dpervushin"
> <dpervushin@embeddedalley.com>, "Artem Bityutskiy" <Artem.Bityutskiy@nokia.com>, "linux-mtd"
> <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "yi zhang" <yi.zhang@huawei.com>,
> "yangerkun" <yangerkun@huawei.com>
> 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

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

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

[ZhaoLong Wang]

> ----- Ursprüngliche Mail -----
>> Von: "ZhaoLong Wang" <wangzhaolong1@huawei.com>
>> An: "richard" <richard@nod.at>, "chengzhihao1" <chengzhihao1@huawei.com>
>> CC: "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>, "dpervushin"
>> <dpervushin@embeddedalley.com>, "Artem Bityutskiy" <Artem.Bityutskiy@nokia.com>, "linux-mtd"
>> <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "yi zhang" <yi.zhang@huawei.com>,
>> "yangerkun" <yangerkun@huawei.com>
>> 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

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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "ZhaoLong Wang" <wangzhaolong1@huawei.com>
> 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

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

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

[Zhihao Cheng]

在 2023/10/22 0:09, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>>>> 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
> 
> .
> 


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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>>> 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

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

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

[Zhihao Cheng]

在 2023/10/23 15:15, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>>>> 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
>
> .



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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Gagan Sidhu" <broly@mac.com>
> An: "ZhaoLong Wang" <wangzhaolong1@huawei.com>
> i have attached a log of this behaviour. and by removing mr wang’s “fixes”, it
> mounts as we would expect.
> 
> this change must be reverted. extremely surprised the openwrt team has not
> raised issues over this by now.

No need to be offended, sometimes use-cases are forgotten.
Thanks for your report, we'll look into it.

Thanks,
//richard

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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Gagan Sidhu" <broly@mac.com>
> typically what happens is we will wrap a squashfs filesystem inside a UBI layer.
> openwrt people call this “ubinising” the root filesystem.
> 
> then, after we label the appropriate nand partitions as ‘uimage,fw’ to call the
> right mtdsplit, the mtd_ubi subsystem works its magic automatically, as long as
> the root partition is named “rootfs”.
> 
> at that point, the rootfs will be *AUTOMATICALLY* mounted AND booted from BY THE
> KERNEL. that is, no cmdline hacks are required.
> 
> this patch breaks that behaviour since mr wang’s additional conditions result in
> the failure of the partition to get added to the mtd list, and thus fails
> mount.
> 

Please help me understanding the layering.
So, you have sqaushfs inside an UBI volume and create a UBI block device from that?
And you have enabled gluebi to have the said UBI volume as MTD (and thus as mtdblock too)?

Thanks,
//richard

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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Gagan Sidhu" <broly@mac.com>
> yes, i have a squashfs inside a ubi volume, and i create ubi block device from
> it.
> 
> i do use config_mtd_ubi_block because the filesystem is squashfs.
> 
> so i think it’s affirmative to both questions. gluebi for the block device from
> the ubi.fs file, then config_mtd_ubi_block to mount this read-only filesystem
> as a block device.
> 
> the end result of both options is a read-only block device that can handle bad
> blocks on nand devices.
> 
> if i was using an M25P80, i wouldn’t even be using ubi.
> 
> so CONFIG_MTD_UBI_{GLUEBI,BLOCK} are handy for cases where you need an mtd block
> device with a read-only file system where the UBI takes care of the
> idiosyncrasies that make NAND (imo, ofc) inferior to SPI

Isn't MTD -> UBI -> GLUBI -> MTD -> MTDBLOCK performance wise a nightmare?
We have UBIBlock for this use case.

Thanks,
//richard

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

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

[Gagan Sidhu]

i don’t think my articulation is correct if you interpreted it as that.

as i understand it, gluebi simply makes it handy when you have a filesystem packed within a ubi file, and it will take that file and mount itas a block device.

so i would say it’s not MTD->UBI->GLUEBI->MTD->MTDBLOCK

it’d say it’s more MTD->GLUEBI->MTDBLOCK

that is, the squashfs (actual filesystem) is simply wrapped in a ubi file for gluebi, which will then make it a block device.

i’m not using an actual UBIFS anywhere, aside from sitting underneath the squashfs to handle the mapping from the squash to the nand (to handle bad blocks).

e.g. i have a rootfs in a squashfs file
	-i wrap that squashfs file into a squashfs.ubi
	-i use gluebi to detect this squashfs.ubi file as a block device and mount it

that’s about it.


Thanks,
Gagan

> On Jun 17, 2024, at 10:00 AM, Richard Weinberger <richard@nod.at> wrote:
> 
> ----- Ursprüngliche Mail -----
>> Von: "Gagan Sidhu" <broly@mac.com>
>> yes, i have a squashfs inside a ubi volume, and i create ubi block device from
>> it.
>> 
>> i do use config_mtd_ubi_block because the filesystem is squashfs.
>> 
>> so i think it’s affirmative to both questions. gluebi for the block device from
>> the ubi.fs file, then config_mtd_ubi_block to mount this read-only filesystem
>> as a block device.
>> 
>> the end result of both options is a read-only block device that can handle bad
>> blocks on nand devices.
>> 
>> if i was using an M25P80, i wouldn’t even be using ubi.
>> 
>> so CONFIG_MTD_UBI_{GLUEBI,BLOCK} are handy for cases where you need an mtd block
>> device with a read-only file system where the UBI takes care of the
>> idiosyncrasies that make NAND (imo, ofc) inferior to SPI
> 
> Isn't MTD -> UBI -> GLUBI -> MTD -> MTDBLOCK performance wise a nightmare?
> We have UBIBlock for this use case.
> 
> Thanks,
> //richard


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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Gagan Sidhu" <broly@mac.com>
> i don’t think my articulation is correct if you interpreted it as that.
> 
> as i understand it, gluebi simply makes it handy when you have a filesystem
> packed within a ubi file, and it will take that file and mount itas a block
> device.

There is no such thing as an UBI file. UBI hosts volumes.
You can install into these volumes whatever you want.
Also a file system such as UBIFS, but this seems not to be the case here.

> so i would say it’s not MTD->UBI->GLUEBI->MTD->MTDBLOCK
> 
> it’d say it’s more MTD->GLUEBI->MTDBLOCK

No. GLUBI emulates a MTD on top of an UBI volume.
So every read/write operation of the filesystem will first to through:

1. block layer
2. MTDBLOCK (and mtd)
3. GLUBI
4. UBI
5. MTD (this time the real one)

Is this really a setup OpenWRT is using?
I'm not saying it's impossible, but far from ideal.
We have UBIBlock for reasons.

Anyway, since the kernel has to be user space friendly and
users seems to use such "odd" stackings I consider reverting this patch.
ZhaoLong Wang, what do you think?

Thanks,
//richard

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

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

[Gagan Sidhu]

just to highlight this, let’s look at the failed boot with the changes discussed in this patch

[    5.462504] auto-attach mtd7
[    5.462525] ubi0: default fastmap pool size: 15
[    5.477309] ubi0: default fastmap WL pool size: 7
[    5.486683] ubi0: attaching mtd7
[    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
[    5.811299] ubi0: scanning is finished
[    5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
[    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
[    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
[    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
[    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
[    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
[    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
[    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
[    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
[    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
[    6.021426] rootfs: parsing partitions cmdlinepart
[    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
[    6.043694] rootfs: got parser (null)
[    6.051426] mtd: device 12 (rootfs) set to be root filesystem
[    6.062891] rootfs_data: parsing partitions cmdlinepart
[    6.073669] rootfs_data: got parser (null)
[    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
[    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
[    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
[    6.297406] Please append a correct "root=" boot option; here are the available partitions:
[    6.314054] 1f00             512 mtdblock0
[    6.314060]  (driver?)
[    6.327077] 1f01             256 mtdblock1
[    6.327081]  (driver?)
[    6.340101] 1f02             256 mtdblock2
[    6.340105]  (driver?)
[    6.353124] 1f03             256 mtdblock3
[    6.353129]  (driver?)
[    6.366153] 1f04           45056 mtdblock4
[    6.366158]  (driver?)
[    6.379175] 1f05           40572 mtdblock5
[    6.379179]  (driver?)
[    6.392217] 1f06            4096 mtdblock6
[    6.392222]  (driver?)
[    6.405240] 1f07           40960 mtdblock7
[    6.405244]  (driver?)
[    6.418272] 1f08           32768 mtdblock8
[    6.418277]  (driver?)
[    6.431296] 1f09           40960 mtdblock9
[    6.431300]  (driver?)
[    6.444324] 1f0a            6144 mtdblock10
[    6.444328]  (driver?)
[    6.457518] 1f0b            4608 mtdblock11
[    6.457523]  (driver?)
[    6.470720] fe00           33604 ubiblock0_0
[    6.470724]  (driver?)
[    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)
[    6.500892] Rebooting in 1 seconds..



here, i assume ubiblock0_0 is the device created from CONFIG_MTD_UBI_BLOCK, correct?

then, i don’t think it’s GLUEBI that is the reason my boot works. i think gluebi is useless now that you mention it, and isn’t the reason everything works.

as you can see, UBI_BLOCK is the reasno ubiblock0_0 is created.

this patch prevents this device from being registered/announced. so when ubi tries to set it (correctly) as the root partition (#12), it fails.

so doesn’t this change affect more than just GLUEBI? it seems to affect UBI_BLOCK as well.

Thanks,
Gagan

> On Jun 17, 2024, at 11:23 AM, Gagan Sidhu <broly@mac.com> wrote:
> 
> 
>> On Jun 17, 2024, at 10:52 AM, Richard Weinberger <richard@nod.at> wrote:
>> 
>> ----- Ursprüngliche Mail -----
>>> Von: "Gagan Sidhu" <broly@mac.com>
>>> i don’t think my articulation is correct if you interpreted it as that.
>>> 
>>> as i understand it, gluebi simply makes it handy when you have a filesystem
>>> packed within a ubi file, and it will take that file and mount itas a block
>>> device.
>> 
>> There is no such thing as an UBI file. UBI hosts volumes.
>> You can install into these volumes whatever you want.
>> Also a file system such as UBIFS, but this seems not to be the case here.
> that’s correct. the UBI sits underneath so it’s not ubifs. 
> 
>> 
>>> so i would say it’s not MTD->UBI->GLUEBI->MTD->MTDBLOCK
>>> 
>>> it’d say it’s more MTD->GLUEBI->MTDBLOCK
>> 
>> No. GLUBI emulates a MTD on top of an UBI volume.
>> So every read/write operation of the filesystem will first to through:
>> 
>> 1. block layer
>> 2. MTDBLOCK (and mtd)
>> 3. GLUBI
>> 4. UBI
>> 5. MTD (this time the real one)
>> 
>> Is this really a setup OpenWRT is using?
>> I'm not saying it's impossible, but far from ideal.
>> We have UBIBlock for reasons.
>> 
> i don’t understand what you mean. i didn’t think this was unusual haha.
> 
> all ubiblock does is give me the right to use a read-only filesystem. it doesn’t map the UBI to a block device.
> 
> are you saying there is an easy automated solution that allows me to remove gluebi, and maintain functionality? it doesn’t seem so easy.
> 
> for example, here is an openwrt setup: https://forum.openwrt.org/t/ubifs-mount-twice-at-booting/126198
> 
> so instead of using gluebi, they use an UBIFS. or they use an overlay. but up until that point, it’s similar.
> 
> i didn’t think gluebi was the reason this check was problematic.
> 	- are you saying MTD_UBIVOLUME is only a property of GLUEBI?
> 
> these lines seemed more general than that.
> 
> my position is this:
> 
> 1. ubi seems to take care of everything as long as i name the partition accordingly (here, i pack the ubi file with two volumes, one for the kernel, the other with the rootfs).
> 2. the change being discussed broke that. 
> 3. i don’t see how gluebi is the root of the problem though, because i have MTD_UBI_BLOCK enabled as well, so shouldn’t in spite of the change? it does not.
> 
> 
>> Anyway, since the kernel has to be user space friendly and
>> users seems to use such "odd" stackings I consider reverting this patch.
>> ZhaoLong Wang, what do you think?
>> 
>> Thanks,
>> //richard
> 


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

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

[Gagan Sidhu]

https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/gluebi.c#L297

it seems the GLUEBI is setting the mtd to MTD_UBIVOLUME

https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/block.c

where this doesn’t even have the text “mtd” anywhere.

but the boot partition is always the ubiblock device.

so is gluebi taking the same volume and adding the MTD_UBIVOLUME label or something?

that seems a little unusual.

Thanks,
Gagan

> On Jun 17, 2024, at 11:33 AM, Gagan Sidhu <broly@mac.com> wrote:
> 
> just to highlight this, let’s look at the failed boot with the changes discussed in this patch
> 
> [    5.462504] auto-attach mtd7
> [    5.462525] ubi0: default fastmap pool size: 15
> [    5.477309] ubi0: default fastmap WL pool size: 7
> [    5.486683] ubi0: attaching mtd7
> [    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
> [    5.811299] ubi0: scanning is finished
> [    5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
> [    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
> [    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
> [    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
> [    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
> [    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
> [    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
> [    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
> [    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
> [    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
> [    6.021426] rootfs: parsing partitions cmdlinepart
> [    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
> [    6.043694] rootfs: got parser (null)
> [    6.051426] mtd: device 12 (rootfs) set to be root filesystem
> [    6.062891] rootfs_data: parsing partitions cmdlinepart
> [    6.073669] rootfs_data: got parser (null)
> [    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
> [    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
> [    6.297406] Please append a correct "root=" boot option; here are the available partitions:
> [    6.314054] 1f00             512 mtdblock0
> [    6.314060]  (driver?)
> [    6.327077] 1f01             256 mtdblock1
> [    6.327081]  (driver?)
> [    6.340101] 1f02             256 mtdblock2
> [    6.340105]  (driver?)
> [    6.353124] 1f03             256 mtdblock3
> [    6.353129]  (driver?)
> [    6.366153] 1f04           45056 mtdblock4
> [    6.366158]  (driver?)
> [    6.379175] 1f05           40572 mtdblock5
> [    6.379179]  (driver?)
> [    6.392217] 1f06            4096 mtdblock6
> [    6.392222]  (driver?)
> [    6.405240] 1f07           40960 mtdblock7
> [    6.405244]  (driver?)
> [    6.418272] 1f08           32768 mtdblock8
> [    6.418277]  (driver?)
> [    6.431296] 1f09           40960 mtdblock9
> [    6.431300]  (driver?)
> [    6.444324] 1f0a            6144 mtdblock10
> [    6.444328]  (driver?)
> [    6.457518] 1f0b            4608 mtdblock11
> [    6.457523]  (driver?)
> [    6.470720] fe00           33604 ubiblock0_0
> [    6.470724]  (driver?)
> [    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)
> [    6.500892] Rebooting in 1 seconds..
> 
> 
> 
> here, i assume ubiblock0_0 is the device created from CONFIG_MTD_UBI_BLOCK, correct?
> 
> then, i don’t think it’s GLUEBI that is the reason my boot works. i think gluebi is useless now that you mention it, and isn’t the reason everything works.
> 
> as you can see, UBI_BLOCK is the reasno ubiblock0_0 is created.
> 
> this patch prevents this device from being registered/announced. so when ubi tries to set it (correctly) as the root partition (#12), it fails.
> 
> so doesn’t this change affect more than just GLUEBI? it seems to affect UBI_BLOCK as well.
> 
> Thanks,
> Gagan
> 
>> On Jun 17, 2024, at 11:23 AM, Gagan Sidhu <broly@mac.com> wrote:
>> 
>> 
>>> On Jun 17, 2024, at 10:52 AM, Richard Weinberger <richard@nod.at> wrote:
>>> 
>>> ----- Ursprüngliche Mail -----
>>>> Von: "Gagan Sidhu" <broly@mac.com>
>>>> i don’t think my articulation is correct if you interpreted it as that.
>>>> 
>>>> as i understand it, gluebi simply makes it handy when you have a filesystem
>>>> packed within a ubi file, and it will take that file and mount itas a block
>>>> device.
>>> 
>>> There is no such thing as an UBI file. UBI hosts volumes.
>>> You can install into these volumes whatever you want.
>>> Also a file system such as UBIFS, but this seems not to be the case here.
>> that’s correct. the UBI sits underneath so it’s not ubifs. 
>> 
>>> 
>>>> so i would say it’s not MTD->UBI->GLUEBI->MTD->MTDBLOCK
>>>> 
>>>> it’d say it’s more MTD->GLUEBI->MTDBLOCK
>>> 
>>> No. GLUBI emulates a MTD on top of an UBI volume.
>>> So every read/write operation of the filesystem will first to through:
>>> 
>>> 1. block layer
>>> 2. MTDBLOCK (and mtd)
>>> 3. GLUBI
>>> 4. UBI
>>> 5. MTD (this time the real one)
>>> 
>>> Is this really a setup OpenWRT is using?
>>> I'm not saying it's impossible, but far from ideal.
>>> We have UBIBlock for reasons.
>>> 
>> i don’t understand what you mean. i didn’t think this was unusual haha.
>> 
>> all ubiblock does is give me the right to use a read-only filesystem. it doesn’t map the UBI to a block device.
>> 
>> are you saying there is an easy automated solution that allows me to remove gluebi, and maintain functionality? it doesn’t seem so easy.
>> 
>> for example, here is an openwrt setup: https://forum.openwrt.org/t/ubifs-mount-twice-at-booting/126198
>> 
>> so instead of using gluebi, they use an UBIFS. or they use an overlay. but up until that point, it’s similar.
>> 
>> i didn’t think gluebi was the reason this check was problematic.
>> 	- are you saying MTD_UBIVOLUME is only a property of GLUEBI?
>> 
>> these lines seemed more general than that.
>> 
>> my position is this:
>> 
>> 1. ubi seems to take care of everything as long as i name the partition accordingly (here, i pack the ubi file with two volumes, one for the kernel, the other with the rootfs).
>> 2. the change being discussed broke that. 
>> 3. i don’t see how gluebi is the root of the problem though, because i have MTD_UBI_BLOCK enabled as well, so shouldn’t in spite of the change? it does not.
>> 
>> 
>>> Anyway, since the kernel has to be user space friendly and
>>> users seems to use such "odd" stackings I consider reverting this patch.
>>> ZhaoLong Wang, what do you think?
>>> 
>>> Thanks,
>>> //richard
>> 
> 


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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Gagan Sidhu" <broly@mac.com>
> https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/gluebi.c#L297
> 
> it seems the GLUEBI is setting the mtd to MTD_UBIVOLUME
> 
> https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/block.c
> 
> where this doesn’t even have the text “mtd” anywhere.
> 
> but the boot partition is always the ubiblock device.
> 
> so is gluebi taking the same volume and adding the MTD_UBIVOLUME label or
> something?

Yes, GLUEBI emulates a MTD on top of an UBI volume.
It sets the MTD device type to MTD_UBIVOLUME.

>> 
>> [    5.462504] auto-attach mtd7
>> [    5.462525] ubi0: default fastmap pool size: 15
>> [    5.477309] ubi0: default fastmap WL pool size: 7
>> [    5.486683] ubi0: attaching mtd7
>> [    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
>> [    5.811299] ubi0: scanning is finished
>> [    5.874546] gluebi (pid 1): gluebi_resized: got update notification for
>> unknown UBI device 0 volume 1
>> [    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
>> [    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
>> [    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
>> [    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
>> [    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
>> [    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
>> [    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count:
>> 128
>> [    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image
>> sequence number: 1613475955
>> [    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved
>> for bad PEB handling: 15
>> [    6.021426] rootfs: parsing partitions cmdlinepart
>> [    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
>> [    6.043694] rootfs: got parser (null)
>> [    6.051426] mtd: device 12 (rootfs) set to be root filesystem

AFAICT, this log line is not part of the mainline kernel.

>> [    6.062891] rootfs_data: parsing partitions cmdlinepart
>> [    6.073669] rootfs_data: got parser (null)
>> [    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
>> [    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
>> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12):
>> error -6
>> [    6.297406] Please append a correct "root=" boot option; here are the
>> available partitions:
>> [    6.314054] 1f00             512 mtdblock0
>> [    6.314060]  (driver?)
>> [    6.327077] 1f01             256 mtdblock1
>> [    6.327081]  (driver?)
>> [    6.340101] 1f02             256 mtdblock2
>> [    6.340105]  (driver?)
>> [    6.353124] 1f03             256 mtdblock3
>> [    6.353129]  (driver?)
>> [    6.366153] 1f04           45056 mtdblock4
>> [    6.366158]  (driver?)
>> [    6.379175] 1f05           40572 mtdblock5
>> [    6.379179]  (driver?)
>> [    6.392217] 1f06            4096 mtdblock6
>> [    6.392222]  (driver?)
>> [    6.405240] 1f07           40960 mtdblock7
>> [    6.405244]  (driver?)
>> [    6.418272] 1f08           32768 mtdblock8
>> [    6.418277]  (driver?)
>> [    6.431296] 1f09           40960 mtdblock9
>> [    6.431300]  (driver?)
>> [    6.444324] 1f0a            6144 mtdblock10
>> [    6.444328]  (driver?)
>> [    6.457518] 1f0b            4608 mtdblock11
>> [    6.457523]  (driver?)
>> [    6.470720] fe00           33604 ubiblock0_0
>> [    6.470724]  (driver?)
>> [    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on
>> unknown-block(31,12)

(31, 12) would be mtdblock12.
How does your kernel know that mtdblock12 shall be the rootfs?

I have a hard time understanding your current setup.

Thanks,
//richard

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

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

[Gagan Sidhu]

> On Jun 17, 2024, at 12:09 PM, Richard Weinberger <richard@nod.at> wrote:
> 
> ----- Ursprüngliche Mail -----
>> Von: "Gagan Sidhu" <broly@mac.com>
>> https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/gluebi.c#L297
>> 
>> it seems the GLUEBI is setting the mtd to MTD_UBIVOLUME
>> 
>> https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/block.c
>> 
>> where this doesn’t even have the text “mtd” anywhere.
>> 
>> but the boot partition is always the ubiblock device.
>> 
>> so is gluebi taking the same volume and adding the MTD_UBIVOLUME label or
>> something?
> 
> Yes, GLUEBI emulates a MTD on top of an UBI volume.
> It sets the MTD device type to MTD_UBIVOLUME.
> 
>>> 
>>> [    5.462504] auto-attach mtd7
>>> [    5.462525] ubi0: default fastmap pool size: 15
>>> [    5.477309] ubi0: default fastmap WL pool size: 7
>>> [    5.486683] ubi0: attaching mtd7
>>> [    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
>>> [    5.811299] ubi0: scanning is finished
>>> [    5.874546] gluebi (pid 1): gluebi_resized: got update notification for
>>> unknown UBI device 0 volume 1
>>> [    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
>>> [    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
>>> [    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
>>> [    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
>>> [    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
>>> [    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
>>> [    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count:
>>> 128
>>> [    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image
>>> sequence number: 1613475955
>>> [    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved
>>> for bad PEB handling: 15
>>> [    6.021426] rootfs: parsing partitions cmdlinepart
>>> [    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
>>> [    6.043694] rootfs: got parser (null)
>>> [    6.051426] mtd: device 12 (rootfs) set to be root filesystem
> 
> AFAICT, this log line is not part of the mainline kernel.

this is mainline. it’s just not 6.x. it’s 4.14.
> 
>>> [    6.062891] rootfs_data: parsing partitions cmdlinepart
>>> [    6.073669] rootfs_data: got parser (null)
>>> [    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
>>> [    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
>>> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12):
>>> error -6
>>> [    6.297406] Please append a correct "root=" boot option; here are the
>>> available partitions:
>>> [    6.314054] 1f00             512 mtdblock0
>>> [    6.314060]  (driver?)
>>> [    6.327077] 1f01             256 mtdblock1
>>> [    6.327081]  (driver?)
>>> [    6.340101] 1f02             256 mtdblock2
>>> [    6.340105]  (driver?)
>>> [    6.353124] 1f03             256 mtdblock3
>>> [    6.353129]  (driver?)
>>> [    6.366153] 1f04           45056 mtdblock4
>>> [    6.366158]  (driver?)
>>> [    6.379175] 1f05           40572 mtdblock5
>>> [    6.379179]  (driver?)
>>> [    6.392217] 1f06            4096 mtdblock6
>>> [    6.392222]  (driver?)
>>> [    6.405240] 1f07           40960 mtdblock7
>>> [    6.405244]  (driver?)
>>> [    6.418272] 1f08           32768 mtdblock8
>>> [    6.418277]  (driver?)
>>> [    6.431296] 1f09           40960 mtdblock9
>>> [    6.431300]  (driver?)
>>> [    6.444324] 1f0a            6144 mtdblock10
>>> [    6.444328]  (driver?)
>>> [    6.457518] 1f0b            4608 mtdblock11
>>> [    6.457523]  (driver?)
>>> [    6.470720] fe00           33604 ubiblock0_0
>>> [    6.470724]  (driver?)
>>> [    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on
>>> unknown-block(31,12)
> 
> (31, 12) would be mtdblock12.
> How does your kernel know that mtdblock12 shall be the rootfs?

this is an openwrt approach: https://openwrt.org/docs/techref/filesystems (under “technical details”, third paragraph)

essentially there’s a feature they add to the kernel (via patch) where you can enable a feature that sets the root device based on the name of the partition.

in this case as long as the volume within your ubi file contains the name “rootfs”, openwrt will follow it as it gets unpacked and set that as the rootdevice for you.

—

> 
> I have a hard time understanding your current setup.

i think we are saying the same thing when you say (31,12) is mtdblock12, which is shown as ubiblock0_0, because once i remove the change under discussion here, the boot is fine. 

is it possible that gluebi is adding properties to the device created by CONFIG_MTD_UBI_BLOCK or something like that?

i wouldn’t worry about how the kernel knows which partition to set as root. openwrt has been doing this for over ten years. 

the real question is, if we know what partition to set as root (ubiblock0_0 or mtdblock12), why does this change prevent it from finishing the boot?

the only explanation seems to be that gluebi is adding features to the ubiblock0_0 device (MTD_UBIVOLUME)?



> 
> Thanks,
> //richard


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

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

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Gagan Sidhu" <broly@mac.com>
>> AFAICT, this log line is not part of the mainline kernel.
> 
> this is mainline. it’s just not 6.x. it’s 4.14.

I've double checked and disagree.
This line comes from:
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-4.14/480-mtd-set-rootfs-to-be-root-dev.patch;h=6cddaf01b75cb58cfb377f568f2c375af87e2f1b;hb=c3bd1321de1e0d814f5cfc4f494f6b2fb1f5133b

In recent OpenWRT kernels I see:
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD

Looks like in recent versions the patch in question does *not* cause a regression.

>> (31, 12) would be mtdblock12.
>> How does your kernel know that mtdblock12 shall be the rootfs?
> 
> this is an openwrt approach: https://openwrt.org/docs/techref/filesystems (under
> “technical details”, third paragraph)
> 
> essentially there’s a feature they add to the kernel (via patch) where you can
> enable a feature that sets the root device based on the name of the partition.

So, this is all not mainline. :-/
 
> in this case as long as the volume within your ubi file contains the name
> “rootfs”, openwrt will follow it as it gets unpacked and set that as the
> rootdevice for you.

Thanks,
//richard

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

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

[Gagan Sidhu]

> On Jun 17, 2024, at 12:32 PM, Richard Weinberger <richard@nod.at> wrote:
> 
> ----- Ursprüngliche Mail -----
>> Von: "Gagan Sidhu" <broly@mac.com>
>>> AFAICT, this log line is not part of the mainline kernel.
>> 
>> this is mainline. it’s just not 6.x. it’s 4.14.
> 
> I've double checked and disagree.
> This line comes from:
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-4.14/480-mtd-set-rootfs-to-be-root-dev.patch;h=6cddaf01b75cb58cfb377f568f2c375af87e2f1b;hb=c3bd1321de1e0d814f5cfc4f494f6b2fb1f5133b

no i know that, that’s the patch i showed you. i meant the rest of it is mainline. the patch obviously is not.
> 
> In recent OpenWRT kernels I see:
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD
> 
> Looks like in recent versions the patch in question does *not* cause a regression.

that patch is also applied in my version as well, so i don’t see how this avoids the regression.

https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774

mine says "[6.051426] mtd: device 12 (rootfs) set to be root filesystem"

which is simply the call from drivers/mtd/mtdcore.c

so the rootfs device is set correctly. it’s just not booting from it.

the regression comes from having GLUEBI+BLOCK enabled, it seems, are they fighting for/operating on the same partition?

> 
>>> (31, 12) would be mtdblock12.
>>> How does your kernel know that mtdblock12 shall be the rootfs?
>> 
>> this is an openwrt approach: https://openwrt.org/docs/techref/filesystems (under
>> “technical details”, third paragraph)
>> 
>> essentially there’s a feature they add to the kernel (via patch) where you can
>> enable a feature that sets the root device based on the name of the partition.
> 
> So, this is all not mainline. :-/

i did say openwrt at the start, and i think that’s pretty close to mainline as it gets.

sometimes these patches aren’t appropriate to push upstream. this one is not the one causing the issue.

it seems to me that there is a problem with GLUEBI+BLOCK playing together. 

as far as i can see, the setting of the device is being doing by mtdcore.c

it’s just not working with gluebi and block are enabled, and i need to know whether disabling gluebi will allow it to work.

in other words, is it possible for gluebi to use the partition created by ubi_block, and add the MTD_UBIVOLUME flag?

> 
>> in this case as long as the volume within your ubi file contains the name
>> “rootfs”, openwrt will follow it as it gets unpacked and set that as the
>> rootdevice for you.
> 
> Thanks,
> //richard


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

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

[Richard Weinberger]

[CC'ing Daniel]

----- Ursprüngliche Mail -----
> Von: "Gagan Sidhu" <broly@mac.com>
> An: "richard" <richard@nod.at>
> CC: "ZhaoLong Wang" <wangzhaolong1@huawei.com>, "chengzhihao1" <chengzhihao1@huawei.com>, "dpervushin"
> <dpervushin@embeddedalley.com>, "linux-kernel" <linux-kernel@vger.kernel.org>, "linux-mtd"
> <linux-mtd@lists.infradead.org>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>,
> "yangerkun" <yangerkun@huawei.com>, "yi zhang" <yi.zhang@huawei.com>
> Gesendet: Montag, 17. Juni 2024 20:46:10
> Betreff: Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier

>> On Jun 17, 2024, at 12:32 PM, Richard Weinberger <richard@nod.at> wrote:
>> 
>> ----- Ursprüngliche Mail -----
>>> Von: "Gagan Sidhu" <broly@mac.com>
>>>> AFAICT, this log line is not part of the mainline kernel.
>>> 
>>> this is mainline. it’s just not 6.x. it’s 4.14.
>> 
>> I've double checked and disagree.
>> This line comes from:
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-4.14/480-mtd-set-rootfs-to-be-root-dev.patch;h=6cddaf01b75cb58cfb377f568f2c375af87e2f1b;hb=c3bd1321de1e0d814f5cfc4f494f6b2fb1f5133b
> 
> no i know that, that’s the patch i showed you. i meant the rest of it is
> mainline. the patch obviously is not.
>> 
>> In recent OpenWRT kernels I see:
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD
>> 
>> Looks like in recent versions the patch in question does *not* cause a
>> regression.
> 
> that patch is also applied in my version as well, so i don’t see how this avoids
> the regression.
> 
> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774
> 
> mine says "[6.051426] mtd: device 12 (rootfs) set to be root filesystem"
> 
> which is simply the call from drivers/mtd/mtdcore.c
> 
> so the rootfs device is set correctly. it’s just not booting from it.
> 
> the regression comes from having GLUEBI+BLOCK enabled, it seems, are they
> fighting for/operating on the same partition?

I don't know. Let's ask Daniel.

> 
>> 
>>>> (31, 12) would be mtdblock12.
>>>> How does your kernel know that mtdblock12 shall be the rootfs?
>>> 
>>> this is an openwrt approach: https://openwrt.org/docs/techref/filesystems (under
>>> “technical details”, third paragraph)
>>> 
>>> essentially there’s a feature they add to the kernel (via patch) where you can
>>> enable a feature that sets the root device based on the name of the partition.
>> 
>> So, this is all not mainline. :-/
> 
> i did say openwrt at the start, and i think that’s pretty close to mainline as
> it gets.
> 
> sometimes these patches aren’t appropriate to push upstream. this one is not the
> one causing the issue.
> 
> it seems to me that there is a problem with GLUEBI+BLOCK playing together.
> 
> as far as i can see, the setting of the device is being doing by mtdcore.c
> 
> it’s just not working with gluebi and block are enabled, and i need to know
> whether disabling gluebi will allow it to work.
> 
> in other words, is it possible for gluebi to use the partition created by
> ubi_block, and add the MTD_UBIVOLUME flag?

No. UBIBlock works on top of UBI volumes and creates a block device.

We'll sort this out. :)

Thanks,
//richard

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

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

[Daniel Golle]

Hi Richard,

On Mon, Jun 17, 2024 at 08:52:55PM +0200, Richard Weinberger wrote:
> [CC'ing Daniel]
> 
> ----- Ursprüngliche Mail -----
> > Von: "Gagan Sidhu" <broly@mac.com>
> > An: "richard" <richard@nod.at>
> > CC: "ZhaoLong Wang" <wangzhaolong1@huawei.com>, "chengzhihao1" <chengzhihao1@huawei.com>, "dpervushin"
> > <dpervushin@embeddedalley.com>, "linux-kernel" <linux-kernel@vger.kernel.org>, "linux-mtd"
> > <linux-mtd@lists.infradead.org>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>,
> > "yangerkun" <yangerkun@huawei.com>, "yi zhang" <yi.zhang@huawei.com>
> > Gesendet: Montag, 17. Juni 2024 20:46:10
> > Betreff: Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
> 
> >> On Jun 17, 2024, at 12:32 PM, Richard Weinberger <richard@nod.at> wrote:
> >> 
> >> ----- Ursprüngliche Mail -----
> >>> Von: "Gagan Sidhu" <broly@mac.com>
> >>>> AFAICT, this log line is not part of the mainline kernel.
> >>> 
> >>> this is mainline. it’s just not 6.x. it’s 4.14.
> >> 
> >> I've double checked and disagree.
> >> This line comes from:
> >> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-4.14/480-mtd-set-rootfs-to-be-root-dev.patch;h=6cddaf01b75cb58cfb377f568f2c375af87e2f1b;hb=c3bd1321de1e0d814f5cfc4f494f6b2fb1f5133b
> > 
> > no i know that, that’s the patch i showed you. i meant the rest of it is
> > mainline. the patch obviously is not.
> >> 
> >> In recent OpenWRT kernels I see:
> >> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD
> >> 
> >> Looks like in recent versions the patch in question does *not* cause a
> >> regression.
> > 
> > that patch is also applied in my version as well, so i don’t see how this avoids
> > the regression.
> > 
> > https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774
> > 
> > mine says "[6.051426] mtd: device 12 (rootfs) set to be root filesystem"
> > 
> > which is simply the call from drivers/mtd/mtdcore.c
> > 
> > so the rootfs device is set correctly. it’s just not booting from it.
> > 
> > the regression comes from having GLUEBI+BLOCK enabled, it seems, are they
> > fighting for/operating on the same partition?
> 
> I don't know. Let's ask Daniel.

I've tried to follow up with this thread and to understand what this is all
about.

Let me add a few things:

 * In OpenWrt we are trying to get rid of the downstream patches
   mentioned here. They were introduced at a time when we did not have
   Device Tree and hence no option to use kernel cmdline to attach UBI,
   or use root= parameter. Nowadays they are legacy and we should not
   use the various auto-rootfs hacks in favor of doing the same thing
   using /chosen/bootargs in DT and the like.

 * Shortly after the introduction of ubiblock we happily got rid of gluebi.
   In OpenWrt gluebi has not been enabled for a very long time, and
   hence I can tell little about potential problems it may cause, or at
   least I can't do more than anyone else can which is reading the code
   and the logs supplied by the user.

All that being said there are of course cases where one may simply
require to use a very old kernel (4.14 here) in order to be able to run
proprietary out-of-tree drivers (rt2860 wifi here). And, of course, if
you can compile a kernel with gluebi and ubiblock both enabled, then
that should work as well and not result in either of them going south.

So reading the logs I do understand what has happened:
Note that the error for unknown-block(31,12) is ENXIO ("No such device
or address"), and that makes sense as that mtdblock12 device indeed
doesn't exist:

> [    6.457518] 1f0b            4608 mtdblock11
> [    6.457523]  (driver?)
> [    6.470720] fe00           33604 ubiblock0_0
> [    6.470724]  (driver?)

And yes, that's due to the added tests for mtd->type != MTD_UBIVOLUME
which prevent mtdblock from being created for gluebi devices.

I understand that the board depends on OpenWrt's patches, as back in the
days of Linux 4.14 MT7621 was still using platform C code for each board
and there was no way to set, append or replace bootargs from DT, simply
because there isn't DT.

As at the time also ubiblock wasn't used and also most likely won't work
with that semi-proprietary firmware based Linux 4.14 which expects JFFS2
to be used, my suggestion is to disabled CONFIG_FTL (if it isn't already
disabled anyway) and apply this patch (which should be further
discussed) in order to fix commit ("mtd: Fix gluebi NULL pointer
dereference caused by ftl notifier"):

diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
index 3caa0717d46c..3ef57dd56288 100644
--- a/drivers/mtd/mtd_blkdevs.c
+++ b/drivers/mtd/mtd_blkdevs.c
@@ -461,7 +461,7 @@ static void blktrans_notify_add(struct mtd_info *mtd)
 {
 	struct mtd_blktrans_ops *tr;
 
-	if (mtd->type == MTD_ABSENT || mtd->type == MTD_UBIVOLUME)
+	if (mtd->type == MTD_ABSENT || (IS_ENABLED(CONFIG_FTL) && mtd->type == MTD_UBIVOLUME))
 		return;
 
 	list_for_each_entry(tr, &blktrans_majors, list)
@@ -501,7 +501,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 && mtd->type != MTD_UBIVOLUME)
+		if (mtd->type != MTD_ABSENT && (!IS_ENABLED(CONFIG_FTL || mtd->type != MTD_UBIVOLUME)))
 			tr->add_mtd(tr, mtd);
 	mutex_unlock(&mtd_table_mutex);
 	return 0;


Let me know if that helps and lets discuss if it could be considered for
being applied in upstream Linux.

> 
> > 
> >> 
> >>>> (31, 12) would be mtdblock12.
> >>>> How does your kernel know that mtdblock12 shall be the rootfs?
> >>> 
> >>> this is an openwrt approach: https://openwrt.org/docs/techref/filesystems (under
> >>> “technical details”, third paragraph)
> >>> 
> >>> essentially there’s a feature they add to the kernel (via patch) where you can
> >>> enable a feature that sets the root device based on the name of the partition.
> >> 
> >> So, this is all not mainline. :-/
> > 
> > i did say openwrt at the start, and i think that’s pretty close to mainline as
> > it gets.
> > 
> > sometimes these patches aren’t appropriate to push upstream. this one is not the
> > one causing the issue.
> > 
> > it seems to me that there is a problem with GLUEBI+BLOCK playing together.
> > 
> > as far as i can see, the setting of the device is being doing by mtdcore.c
> > 
> > it’s just not working with gluebi and block are enabled, and i need to know
> > whether disabling gluebi will allow it to work.
> > 
> > in other words, is it possible for gluebi to use the partition created by
> > ubi_block, and add the MTD_UBIVOLUME flag?
> 
> No. UBIBlock works on top of UBI volumes and creates a block device.
> 
> We'll sort this out. :)
> 
> Thanks,
> //richard
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

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

[Gagan Sidhu]

hi daniel,

most of your understanding is correct, but i am using a device tree. i will answer in-line

Thanks,
Gagan

> On Jun 17, 2024, at 2:29 PM, Daniel Golle <daniel@makrotopia.org> wrote:
> 
> Hi Richard,
> 
> On Mon, Jun 17, 2024 at 08:52:55PM +0200, Richard Weinberger wrote:
>> [CC'ing Daniel]
>> 
>> ----- Ursprüngliche Mail -----
>>> Von: "Gagan Sidhu" <broly@mac.com>
>>> An: "richard" <richard@nod.at>
>>> CC: "ZhaoLong Wang" <wangzhaolong1@huawei.com>, "chengzhihao1" <chengzhihao1@huawei.com>, "dpervushin"
>>> <dpervushin@embeddedalley.com>, "linux-kernel" <linux-kernel@vger.kernel.org>, "linux-mtd"
>>> <linux-mtd@lists.infradead.org>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>,
>>> "yangerkun" <yangerkun@huawei.com>, "yi zhang" <yi.zhang@huawei.com>
>>> Gesendet: Montag, 17. Juni 2024 20:46:10
>>> Betreff: Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
>> 
>>>> On Jun 17, 2024, at 12:32 PM, Richard Weinberger <richard@nod.at> wrote:
>>>> 
>>>> ----- Ursprüngliche Mail -----
>>>>> Von: "Gagan Sidhu" <broly@mac.com>
>>>>>> AFAICT, this log line is not part of the mainline kernel.
>>>>> 
>>>>> this is mainline. it’s just not 6.x. it’s 4.14.
>>>> 
>>>> I've double checked and disagree.
>>>> This line comes from:
>>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-4.14/480-mtd-set-rootfs-to-be-root-dev.patch;h=6cddaf01b75cb58cfb377f568f2c375af87e2f1b;hb=c3bd1321de1e0d814f5cfc4f494f6b2fb1f5133b
>>> 
>>> no i know that, that’s the patch i showed you. i meant the rest of it is
>>> mainline. the patch obviously is not.
>>>> 
>>>> In recent OpenWRT kernels I see:
>>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD
>>>> 
>>>> Looks like in recent versions the patch in question does *not* cause a
>>>> regression.
>>> 
>>> that patch is also applied in my version as well, so i don’t see how this avoids
>>> the regression.
>>> 
>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774
>>> 
>>> mine says "[6.051426] mtd: device 12 (rootfs) set to be root filesystem"
>>> 
>>> which is simply the call from drivers/mtd/mtdcore.c
>>> 
>>> so the rootfs device is set correctly. it’s just not booting from it.
>>> 
>>> the regression comes from having GLUEBI+BLOCK enabled, it seems, are they
>>> fighting for/operating on the same partition?
>> 
>> I don't know. Let's ask Daniel.
> 
> I've tried to follow up with this thread and to understand what this is all
> about.
> 
> Let me add a few things:
> 
> * In OpenWrt we are trying to get rid of the downstream patches
>   mentioned here. They were introduced at a time when we did not have
>   Device Tree and hence no option to use kernel cmdline to attach UBI,
>   or use root= parameter. Nowadays they are legacy and we should not
>   use the various auto-rootfs hacks in favor of doing the same thing
>   using /chosen/bootargs in DT and the like.
> 
> * Shortly after the introduction of ubiblock we happily got rid of gluebi.
>   In OpenWrt gluebi has not been enabled for a very long time, and
>   hence I can tell little about potential problems it may cause, or at
>   least I can't do more than anyone else can which is reading the code
>   and the logs supplied by the user.
> 
> All that being said there are of course cases where one may simply
> require to use a very old kernel (4.14 here) in order to be able to run
> proprietary out-of-tree drivers (rt2860 wifi here). And, of course, if
> you can compile a kernel with gluebi and ubiblock both enabled, then
> that should work as well and not result in either of them going south.
> 
> So reading the logs I do understand what has happened:
> Note that the error for unknown-block(31,12) is ENXIO ("No such device
> or address"), and that makes sense as that mtdblock12 device indeed
> doesn't exist:
> 
>> [    6.457518] 1f0b            4608 mtdblock11
>> [    6.457523]  (driver?)
>> [    6.470720] fe00           33604 ubiblock0_0
>> [    6.470724]  (driver?)
> 
> And yes, that's due to the added tests for mtd->type != MTD_UBIVOLUME
> which prevent mtdblock from being created for gluebi devices.
> 
> I understand that the board depends on OpenWrt's patches, as back in the
> days of Linux 4.14 MT7621 was still using platform C code for each board
> and there was no way to set, append or replace bootargs from DT, simply
> because there isn't DT.

i am using the DT that is very similar to this one:
https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621_dlink_dir_nand_128m.dtsi

as in, the mtd layout you’re seeing in the boot logs is fixed partitions:
```
[    3.188484] 9 fixed-partitions partitions found on MTD device MT7621-NAND
[    3.202005] Creating 9 MTD partitions on "MT7621-NAND":
[    3.212430] 0x000000000000-0x000000080000 : "Bootloader"
[    3.224024] 0x0000000c0000-0x000000100000 : "Config"
[    3.234684] 0x000000100000-0x000000140000 : "Factory"
[    3.245518] 0x000000140000-0x000000180000 : "Config2"
[    3.256379] 0x000000180000-0x000002d80000 : "sysv"
[    3.895176] 1 squashfs-split partitions found on MTD device sysv
[    3.907164] 0x0000005c1000-0x000002d60000 : "ddwrt"
[    3.920925] 2 uimage-fw partitions found on MTD device sysv
[    3.932031] Creating 2 MTD partitions on "sysv":
[    3.941232] 0x000000000000-0x000000400000 : "kernel"
[    3.951995] 0x000000400000-0x000002c00000 : "ubi"
[    3.962325] 0x000002d80000-0x000004d80000 : "private"
[    3.973322] 0x000004d80000-0x000007580000 : "firmware2"
[    3.984759] 0x000007580000-0x000007b80000 : "mydlink"
[    3.995699] 0x000007b80000-0x000008000000 : “reserved”
[    4.006687] [mtk_nand] probe successfully!
[    4.015584] Signature matched and data read!
[    4.024090] load_fact_bbt success 1023

```
here is the rest of the log that richard & i are honing in upon:

```
[    5.462504] auto-attach mtd7
[    5.462525] ubi0: default fastmap pool size: 15
[    5.477309] ubi0: default fastmap WL pool size: 7
[    5.486683] ubi0: attaching mtd7
[    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
[    5.811299] ubi0: scanning is finished
[    5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
[    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
[    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
[    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
[    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
[    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
[    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
[    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
[    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
[    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
[    6.021426] rootfs: parsing partitions cmdlinepart
[    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
[    6.043694] rootfs: got parser (null)
[    6.051426] mtd: device 12 (rootfs) set to be root filesystem
[    6.062891] rootfs_data: parsing partitions cmdlinepart
[    6.073669] rootfs_data: got parser (null)
[    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
[    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
[    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
[    6.297406] Please append a correct "root=" boot option; here are the available partitions:
[    6.314054] 1f00             512 mtdblock0
[    6.314060]  (driver?)
[    6.327077] 1f01             256 mtdblock1
[    6.327081]  (driver?)
[    6.340101] 1f02             256 mtdblock2
[    6.340105]  (driver?)
[    6.353124] 1f03             256 mtdblock3
[    6.353129]  (driver?)
[    6.366153] 1f04           45056 mtdblock4
[    6.366158]  (driver?)
[    6.379175] 1f05           40572 mtdblock5
[    6.379179]  (driver?)
[    6.392217] 1f06            4096 mtdblock6
[    6.392222]  (driver?)
[    6.405240] 1f07           40960 mtdblock7
[    6.405244]  (driver?)
[    6.418272] 1f08           32768 mtdblock8
[    6.418277]  (driver?)
[    6.431296] 1f09           40960 mtdblock9
[    6.431300]  (driver?)
[    6.444324] 1f0a            6144 mtdblock10
[    6.444328]  (driver?)
[    6.457518] 1f0b            4608 mtdblock11
[    6.457523]  (driver?)
[    6.470720] fe00           33604 ubiblock0_0
[    6.470724]  (driver?)
[    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)

```

> 
> As at the time also ubiblock wasn't used and also most likely won't work
> with that semi-proprietary firmware based Linux 4.14 which expects JFFS2
> to be used, my suggestion is to disabled CONFIG_FTL (if it isn't already
> disabled anyway) and apply this patch (which should be further
> discussed) in order to fix commit ("mtd: Fix gluebi NULL pointer
> dereference caused by ftl notifier”):

i’m not using JFFS2, either.

my question is about the interaction between GLUEBI and UBI_BLOCK.

it seems to me that the device is created by UBI_BLOCK, so that’s fine.

the question is: is there interaction between GLUEBI and UBI_BLOCK?

i don’t understand how, if the device is created by UBI_BLOCK, that it could get the MTD_UBIVOLUME tag unless GLUEBI scanned the MTD volumes and found the block device created by UBI Block, and decided that it should have this tag.

there is something wrong here. as richard said: if the ubiblock0_0 device is created by ubi_block as is shown above:

[    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)

then how the heck is this device not getting mounted after boot? the only explanation is that GLUEBI is interfering and doesn’t realise the UBI_BLOCK device should not be tagged.

i would test this myself but i don’t have this router on me and i don’t want to make a user flash their router a few times for me to figure this out. they’ve been through enough with this whole ordeal (lol).

> 
> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
> index 3caa0717d46c..3ef57dd56288 100644
> --- a/drivers/mtd/mtd_blkdevs.c
> +++ b/drivers/mtd/mtd_blkdevs.c
> @@ -461,7 +461,7 @@ static void blktrans_notify_add(struct mtd_info *mtd)
> {
> 	struct mtd_blktrans_ops *tr;
> 
> -	if (mtd->type == MTD_ABSENT || mtd->type == MTD_UBIVOLUME)
> +	if (mtd->type == MTD_ABSENT || (IS_ENABLED(CONFIG_FTL) && mtd->type == MTD_UBIVOLUME))
> 		return;
> 
> 	list_for_each_entry(tr, &blktrans_majors, list)
> @@ -501,7 +501,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 && mtd->type != MTD_UBIVOLUME)
> +		if (mtd->type != MTD_ABSENT && (!IS_ENABLED(CONFIG_FTL || mtd->type != MTD_UBIVOLUME)))
> 			tr->add_mtd(tr, mtd);
> 	mutex_unlock(&mtd_table_mutex);
> 	return 0;
> 
> 
> Let me know if that helps and lets discuss if it could be considered for
> being applied in upstream Linux.

we could do that, but i think we need to find out whether GLUEBI is taking our UBI_BLOCK device and doing something it shouldn’t be doing.

> 
>> 
>>> 
>>>> 
>>>>>> (31, 12) would be mtdblock12.
>>>>>> How does your kernel know that mtdblock12 shall be the rootfs?
>>>>> 
>>>>> this is an openwrt approach: https://openwrt.org/docs/techref/filesystems (under
>>>>> “technical details”, third paragraph)
>>>>> 
>>>>> essentially there’s a feature they add to the kernel (via patch) where you can
>>>>> enable a feature that sets the root device based on the name of the partition.
>>>> 
>>>> So, this is all not mainline. :-/
>>> 
>>> i did say openwrt at the start, and i think that’s pretty close to mainline as
>>> it gets.
>>> 
>>> sometimes these patches aren’t appropriate to push upstream. this one is not the
>>> one causing the issue.
>>> 
>>> it seems to me that there is a problem with GLUEBI+BLOCK playing together.
>>> 
>>> as far as i can see, the setting of the device is being doing by mtdcore.c
>>> 
>>> it’s just not working with gluebi and block are enabled, and i need to know
>>> whether disabling gluebi will allow it to work.
>>> 
>>> in other words, is it possible for gluebi to use the partition created by
>>> ubi_block, and add the MTD_UBIVOLUME flag?
>> 
>> No. UBIBlock works on top of UBI volumes and creates a block device.
>> 
>> We'll sort this out. :)
>> 
>> Thanks,
>> //richard
>> 
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/


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

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

[Gagan Sidhu]

spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine.

so we need to think about either deprecating GLUEBI or setting an option in the Kconfig that ensures they are mutually exclusive.

gluebi is definitely highjacking the block device created by UBI_BLOCK and adding the MTD_UBIVOLUME flag to it.

there is no other explanation.

looks like this was an absolutely amazing exchange that even furthered our understanding of wtf is going on.

thanks for being a great moderator for MTD rich

Thanks,
Gagan

> On Jun 17, 2024, at 3:22 PM, Gagan Sidhu <broly@mac.com> wrote:
> 
> hi daniel,
> 
> most of your understanding is correct, but i am using a device tree. i will answer in-line
> 
> Thanks,
> Gagan
> 
>> On Jun 17, 2024, at 2:29 PM, Daniel Golle <daniel@makrotopia.org> wrote:
>> 
>> Hi Richard,
>> 
>> On Mon, Jun 17, 2024 at 08:52:55PM +0200, Richard Weinberger wrote:
>>> [CC'ing Daniel]
>>> 
>>> ----- Ursprüngliche Mail -----
>>>> Von: "Gagan Sidhu" <broly@mac.com>
>>>> An: "richard" <richard@nod.at>
>>>> CC: "ZhaoLong Wang" <wangzhaolong1@huawei.com>, "chengzhihao1" <chengzhihao1@huawei.com>, "dpervushin"
>>>> <dpervushin@embeddedalley.com>, "linux-kernel" <linux-kernel@vger.kernel.org>, "linux-mtd"
>>>> <linux-mtd@lists.infradead.org>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "Vignesh Raghavendra" <vigneshr@ti.com>,
>>>> "yangerkun" <yangerkun@huawei.com>, "yi zhang" <yi.zhang@huawei.com>
>>>> Gesendet: Montag, 17. Juni 2024 20:46:10
>>>> Betreff: Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
>>> 
>>>>> On Jun 17, 2024, at 12:32 PM, Richard Weinberger <richard@nod.at> wrote:
>>>>> 
>>>>> ----- Ursprüngliche Mail -----
>>>>>> Von: "Gagan Sidhu" <broly@mac.com>
>>>>>>> AFAICT, this log line is not part of the mainline kernel.
>>>>>> 
>>>>>> this is mainline. it’s just not 6.x. it’s 4.14.
>>>>> 
>>>>> I've double checked and disagree.
>>>>> This line comes from:
>>>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-4.14/480-mtd-set-rootfs-to-be-root-dev.patch;h=6cddaf01b75cb58cfb377f568f2c375af87e2f1b;hb=c3bd1321de1e0d814f5cfc4f494f6b2fb1f5133b
>>>> 
>>>> no i know that, that’s the patch i showed you. i meant the rest of it is
>>>> mainline. the patch obviously is not.
>>>>> 
>>>>> In recent OpenWRT kernels I see:
>>>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD
>>>>> 
>>>>> Looks like in recent versions the patch in question does *not* cause a
>>>>> regression.
>>>> 
>>>> that patch is also applied in my version as well, so i don’t see how this avoids
>>>> the regression.
>>>> 
>>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774
>>>> 
>>>> mine says "[6.051426] mtd: device 12 (rootfs) set to be root filesystem"
>>>> 
>>>> which is simply the call from drivers/mtd/mtdcore.c
>>>> 
>>>> so the rootfs device is set correctly. it’s just not booting from it.
>>>> 
>>>> the regression comes from having GLUEBI+BLOCK enabled, it seems, are they
>>>> fighting for/operating on the same partition?
>>> 
>>> I don't know. Let's ask Daniel.
>> 
>> I've tried to follow up with this thread and to understand what this is all
>> about.
>> 
>> Let me add a few things:
>> 
>> * In OpenWrt we are trying to get rid of the downstream patches
>>  mentioned here. They were introduced at a time when we did not have
>>  Device Tree and hence no option to use kernel cmdline to attach UBI,
>>  or use root= parameter. Nowadays they are legacy and we should not
>>  use the various auto-rootfs hacks in favor of doing the same thing
>>  using /chosen/bootargs in DT and the like.
>> 
>> * Shortly after the introduction of ubiblock we happily got rid of gluebi.
>>  In OpenWrt gluebi has not been enabled for a very long time, and
>>  hence I can tell little about potential problems it may cause, or at
>>  least I can't do more than anyone else can which is reading the code
>>  and the logs supplied by the user.
>> 
>> All that being said there are of course cases where one may simply
>> require to use a very old kernel (4.14 here) in order to be able to run
>> proprietary out-of-tree drivers (rt2860 wifi here). And, of course, if
>> you can compile a kernel with gluebi and ubiblock both enabled, then
>> that should work as well and not result in either of them going south.
>> 
>> So reading the logs I do understand what has happened:
>> Note that the error for unknown-block(31,12) is ENXIO ("No such device
>> or address"), and that makes sense as that mtdblock12 device indeed
>> doesn't exist:
>> 
>>> [    6.457518] 1f0b            4608 mtdblock11
>>> [    6.457523]  (driver?)
>>> [    6.470720] fe00           33604 ubiblock0_0
>>> [    6.470724]  (driver?)
>> 
>> And yes, that's due to the added tests for mtd->type != MTD_UBIVOLUME
>> which prevent mtdblock from being created for gluebi devices.
>> 
>> I understand that the board depends on OpenWrt's patches, as back in the
>> days of Linux 4.14 MT7621 was still using platform C code for each board
>> and there was no way to set, append or replace bootargs from DT, simply
>> because there isn't DT.
> 
> i am using the DT that is very similar to this one:
> https://github.com/openwrt/openwrt/blob/master/target/linux/ramips/dts/mt7621_dlink_dir_nand_128m.dtsi
> 
> as in, the mtd layout you’re seeing in the boot logs is fixed partitions:
> ```
> [    3.188484] 9 fixed-partitions partitions found on MTD device MT7621-NAND
> [    3.202005] Creating 9 MTD partitions on "MT7621-NAND":
> [    3.212430] 0x000000000000-0x000000080000 : "Bootloader"
> [    3.224024] 0x0000000c0000-0x000000100000 : "Config"
> [    3.234684] 0x000000100000-0x000000140000 : "Factory"
> [    3.245518] 0x000000140000-0x000000180000 : "Config2"
> [    3.256379] 0x000000180000-0x000002d80000 : "sysv"
> [    3.895176] 1 squashfs-split partitions found on MTD device sysv
> [    3.907164] 0x0000005c1000-0x000002d60000 : "ddwrt"
> [    3.920925] 2 uimage-fw partitions found on MTD device sysv
> [    3.932031] Creating 2 MTD partitions on "sysv":
> [    3.941232] 0x000000000000-0x000000400000 : "kernel"
> [    3.951995] 0x000000400000-0x000002c00000 : "ubi"
> [    3.962325] 0x000002d80000-0x000004d80000 : "private"
> [    3.973322] 0x000004d80000-0x000007580000 : "firmware2"
> [    3.984759] 0x000007580000-0x000007b80000 : "mydlink"
> [    3.995699] 0x000007b80000-0x000008000000 : “reserved”
> [    4.006687] [mtk_nand] probe successfully!
> [    4.015584] Signature matched and data read!
> [    4.024090] load_fact_bbt success 1023
> 
> ```
> here is the rest of the log that richard & i are honing in upon:
> 
> ```
> [    5.462504] auto-attach mtd7
> [    5.462525] ubi0: default fastmap pool size: 15
> [    5.477309] ubi0: default fastmap WL pool size: 7
> [    5.486683] ubi0: attaching mtd7
> [    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
> [    5.811299] ubi0: scanning is finished
> [    5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
> [    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
> [    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
> [    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
> [    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
> [    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
> [    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
> [    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
> [    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
> [    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
> [    6.021426] rootfs: parsing partitions cmdlinepart
> [    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
> [    6.043694] rootfs: got parser (null)
> [    6.051426] mtd: device 12 (rootfs) set to be root filesystem
> [    6.062891] rootfs_data: parsing partitions cmdlinepart
> [    6.073669] rootfs_data: got parser (null)
> [    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
> [    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
> [    6.297406] Please append a correct "root=" boot option; here are the available partitions:
> [    6.314054] 1f00             512 mtdblock0
> [    6.314060]  (driver?)
> [    6.327077] 1f01             256 mtdblock1
> [    6.327081]  (driver?)
> [    6.340101] 1f02             256 mtdblock2
> [    6.340105]  (driver?)
> [    6.353124] 1f03             256 mtdblock3
> [    6.353129]  (driver?)
> [    6.366153] 1f04           45056 mtdblock4
> [    6.366158]  (driver?)
> [    6.379175] 1f05           40572 mtdblock5
> [    6.379179]  (driver?)
> [    6.392217] 1f06            4096 mtdblock6
> [    6.392222]  (driver?)
> [    6.405240] 1f07           40960 mtdblock7
> [    6.405244]  (driver?)
> [    6.418272] 1f08           32768 mtdblock8
> [    6.418277]  (driver?)
> [    6.431296] 1f09           40960 mtdblock9
> [    6.431300]  (driver?)
> [    6.444324] 1f0a            6144 mtdblock10
> [    6.444328]  (driver?)
> [    6.457518] 1f0b            4608 mtdblock11
> [    6.457523]  (driver?)
> [    6.470720] fe00           33604 ubiblock0_0
> [    6.470724]  (driver?)
> [    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)
> 
> ```
> 
>> 
>> As at the time also ubiblock wasn't used and also most likely won't work
>> with that semi-proprietary firmware based Linux 4.14 which expects JFFS2
>> to be used, my suggestion is to disabled CONFIG_FTL (if it isn't already
>> disabled anyway) and apply this patch (which should be further
>> discussed) in order to fix commit ("mtd: Fix gluebi NULL pointer
>> dereference caused by ftl notifier”):
> 
> i’m not using JFFS2, either.
> 
> my question is about the interaction between GLUEBI and UBI_BLOCK.
> 
> it seems to me that the device is created by UBI_BLOCK, so that’s fine.
> 
> the question is: is there interaction between GLUEBI and UBI_BLOCK?
> 
> i don’t understand how, if the device is created by UBI_BLOCK, that it could get the MTD_UBIVOLUME tag unless GLUEBI scanned the MTD volumes and found the block device created by UBI Block, and decided that it should have this tag.
> 
> there is something wrong here. as richard said: if the ubiblock0_0 device is created by ubi_block as is shown above:
> 
> [    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
> 
> then how the heck is this device not getting mounted after boot? the only explanation is that GLUEBI is interfering and doesn’t realise the UBI_BLOCK device should not be tagged.
> 
> i would test this myself but i don’t have this router on me and i don’t want to make a user flash their router a few times for me to figure this out. they’ve been through enough with this whole ordeal (lol).
> 
>> 
>> diff --git a/drivers/mtd/mtd_blkdevs.c b/drivers/mtd/mtd_blkdevs.c
>> index 3caa0717d46c..3ef57dd56288 100644
>> --- a/drivers/mtd/mtd_blkdevs.c
>> +++ b/drivers/mtd/mtd_blkdevs.c
>> @@ -461,7 +461,7 @@ static void blktrans_notify_add(struct mtd_info *mtd)
>> {
>> 	struct mtd_blktrans_ops *tr;
>> 
>> -	if (mtd->type == MTD_ABSENT || mtd->type == MTD_UBIVOLUME)
>> +	if (mtd->type == MTD_ABSENT || (IS_ENABLED(CONFIG_FTL) && mtd->type == MTD_UBIVOLUME))
>> 		return;
>> 
>> 	list_for_each_entry(tr, &blktrans_majors, list)
>> @@ -501,7 +501,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 && mtd->type != MTD_UBIVOLUME)
>> +		if (mtd->type != MTD_ABSENT && (!IS_ENABLED(CONFIG_FTL || mtd->type != MTD_UBIVOLUME)))
>> 			tr->add_mtd(tr, mtd);
>> 	mutex_unlock(&mtd_table_mutex);
>> 	return 0;
>> 
>> 
>> Let me know if that helps and lets discuss if it could be considered for
>> being applied in upstream Linux.
> 
> we could do that, but i think we need to find out whether GLUEBI is taking our UBI_BLOCK device and doing something it shouldn’t be doing.
> 
>> 
>>> 
>>>> 
>>>>> 
>>>>>>> (31, 12) would be mtdblock12.
>>>>>>> How does your kernel know that mtdblock12 shall be the rootfs?
>>>>>> 
>>>>>> this is an openwrt approach: https://openwrt.org/docs/techref/filesystems (under
>>>>>> “technical details”, third paragraph)
>>>>>> 
>>>>>> essentially there’s a feature they add to the kernel (via patch) where you can
>>>>>> enable a feature that sets the root device based on the name of the partition.
>>>>> 
>>>>> So, this is all not mainline. :-/
>>>> 
>>>> i did say openwrt at the start, and i think that’s pretty close to mainline as
>>>> it gets.
>>>> 
>>>> sometimes these patches aren’t appropriate to push upstream. this one is not the
>>>> one causing the issue.
>>>> 
>>>> it seems to me that there is a problem with GLUEBI+BLOCK playing together.
>>>> 
>>>> as far as i can see, the setting of the device is being doing by mtdcore.c
>>>> 
>>>> it’s just not working with gluebi and block are enabled, and i need to know
>>>> whether disabling gluebi will allow it to work.
>>>> 
>>>> in other words, is it possible for gluebi to use the partition created by
>>>> ubi_block, and add the MTD_UBIVOLUME flag?
>>> 
>>> No. UBIBlock works on top of UBI volumes and creates a block device.
>>> 
>>> We'll sort this out. :)
>>> 
>>> Thanks,
>>> //richard
>>> 
>>> ______________________________________________________
>>> Linux MTD discussion mailing list
>>> http://lists.infradead.org/mailman/listinfo/linux-mtd/


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

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

[Zhihao Cheng]

在 2024/6/18 6:13, Gagan Sidhu 写道:
Hi,
> spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine.
> 

May I have the layers' information about mtd/ubi, you can get it by 
'mtdinfo -a' and 'ubinfo -a' after booting the device.
I guess your device boots from ubiblock0_0. There are two ways loading 
booting device:
1. mtd(nand)
    ubi(holds volume ubi0_0)
    mtd12 (gluebi)
    mtdblock12  (This way is cut by this patch, so mtdblock12 is not 
generated, just like Daniel&Richard analyzed)
2. mtd(nand)
    ubi(holds volume ubi0_0)
    ubiblock0_0

> so we need to think about either deprecating GLUEBI or setting an option in the Kconfig that ensures they are mutually exclusive.
> 
> gluebi is definitely highjacking the block device created by UBI_BLOCK and adding the MTD_UBIVOLUME flag to it.

The gluebi(mtd) and ubiblock could exist on the same UBI volume at the 
same time, but they cannot be opened at the same time. Here is an 
example I tested on the local machine:

                                              ↗ ubiblock0_0
mtd0(nandsim) -> ubi0 (holds volume ubi0_0)
                                              ↘ gluebi(mtd1)

[root@localhost ~]# ubinfo -a
UBI version:                    1
Count of UBI devices:           1
UBI control device major/minor: 10:61
Present UBI devices:            ubi0

ubi0
Volumes count:                           1
Logical eraseblock size:                 126976 bytes, 124.0 KiB
Total amount of logical eraseblocks:     8192 (1040187392 bytes, 992.0 MiB)
Amount of available logical eraseblocks: 0 (0 bytes)
Maximum count of volumes                 128
Count of bad physical eraseblocks:       0
Count of reserved physical eraseblocks:  160
Current maximum erase counter value:     2
Minimum input/output unit size:          2048 bytes
Character device major/minor:            246:0
Present volumes:                         0

Volume ID:   0 (on ubi0)
Type:        dynamic
Alignment:   1
Size:        8026 LEBs (1019109376 bytes, 971.8 MiB)
State:       OK
Name:        vol_a
Character device major/minor: 246:1
[root@localhost ~]# mtdinfo -a
Count of MTD devices:           2
Present MTD devices:            mtd0, mtd1
Sysfs interface supported:      yes

mtd0
Name:                           NAND simulator partition 0
Type:                           nand
Eraseblock size:                131072 bytes, 128.0 KiB
Amount of eraseblocks:          8192 (1073741824 bytes, 1024.0 MiB)
Minimum input/output unit size: 2048 bytes
Sub-page size:                  512 bytes
OOB size:                       64 bytes
Character device major/minor:   90:0
Bad blocks are allowed:         true
Device is writable:             true

mtd1
Name:                           vol_a
Type:                           ubi
Eraseblock size:                126976 bytes, 124.0 KiB
Amount of eraseblocks:          8026 (1019109376 bytes, 971.8 MiB)
Minimum input/output unit size: 2048 bytes
Sub-page size:                  2048 bytes
Character device major/minor:   90:2
Bad blocks are allowed:         false
Device is writable:             true

[root@localhost ~]# lsblk | grep ubi
ubiblock0_0 251:0    0 971.9M  0 disk

> 
> there is no other explanation.
> 
> looks like this was an absolutely amazing exchange that even furthered our understanding of wtf is going on.
> 
> thanks for being a great moderator for MTD rich
> 
> Thanks,
> Gagan


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

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

[Gagan Sidhu]

hi zhihao,

so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?

i was just joking about you being a spy by the way. it was post-fix euphoria ;)

master rich, shepherd weinberger, what say ye?

your wisdom and insight would be greatly appreciated as closure and maybe even a patch to reflect the beauty of collaborating over current ;)

Thanks,
Gagan

> On Jun 17, 2024, at 10:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
> 
> 在 2024/6/18 6:13, Gagan Sidhu 写道:
> Hi,
>> spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine.
> 
> May I have the layers' information about mtd/ubi, you can get it by 'mtdinfo -a' and 'ubinfo -a' after booting the device.
> I guess your device boots from ubiblock0_0. There are two ways loading booting device:
> 1. mtd(nand)
>   ubi(holds volume ubi0_0)
>   mtd12 (gluebi)
>   mtdblock12  (This way is cut by this patch, so mtdblock12 is not generated, just like Daniel&Richard analyzed)
> 2. mtd(nand)
>   ubi(holds volume ubi0_0)
>   ubiblock0_0
> 
>> so we need to think about either deprecating GLUEBI or setting an option in the Kconfig that ensures they are mutually exclusive.
>> gluebi is definitely highjacking the block device created by UBI_BLOCK and adding the MTD_UBIVOLUME flag to it.
> 
> The gluebi(mtd) and ubiblock could exist on the same UBI volume at the same time, but they cannot be opened at the same time. Here is an example I tested on the local machine:
> 
>                                             ↗ ubiblock0_0
> mtd0(nandsim) -> ubi0 (holds volume ubi0_0)
>                                             ↘ gluebi(mtd1)
> 
> [root@localhost ~]# ubinfo -a
> UBI version:                    1
> Count of UBI devices:           1
> UBI control device major/minor: 10:61
> Present UBI devices:            ubi0
> 
> ubi0
> Volumes count:                           1
> Logical eraseblock size:                 126976 bytes, 124.0 KiB
> Total amount of logical eraseblocks:     8192 (1040187392 bytes, 992.0 MiB)
> Amount of available logical eraseblocks: 0 (0 bytes)
> Maximum count of volumes                 128
> Count of bad physical eraseblocks:       0
> Count of reserved physical eraseblocks:  160
> Current maximum erase counter value:     2
> Minimum input/output unit size:          2048 bytes
> Character device major/minor:            246:0
> Present volumes:                         0
> 
> Volume ID:   0 (on ubi0)
> Type:        dynamic
> Alignment:   1
> Size:        8026 LEBs (1019109376 bytes, 971.8 MiB)
> State:       OK
> Name:        vol_a
> Character device major/minor: 246:1
> [root@localhost ~]# mtdinfo -a
> Count of MTD devices:           2
> Present MTD devices:            mtd0, mtd1
> Sysfs interface supported:      yes
> 
> mtd0
> Name:                           NAND simulator partition 0
> Type:                           nand
> Eraseblock size:                131072 bytes, 128.0 KiB
> Amount of eraseblocks:          8192 (1073741824 bytes, 1024.0 MiB)
> Minimum input/output unit size: 2048 bytes
> Sub-page size:                  512 bytes
> OOB size:                       64 bytes
> Character device major/minor:   90:0
> Bad blocks are allowed:         true
> Device is writable:             true
> 
> mtd1
> Name:                           vol_a
> Type:                           ubi
> Eraseblock size:                126976 bytes, 124.0 KiB
> Amount of eraseblocks:          8026 (1019109376 bytes, 971.8 MiB)
> Minimum input/output unit size: 2048 bytes
> Sub-page size:                  2048 bytes
> Character device major/minor:   90:2
> Bad blocks are allowed:         false
> Device is writable:             true
> 
> [root@localhost ~]# lsblk | grep ubi
> ubiblock0_0 251:0    0 971.9M  0 disk
> 
>> there is no other explanation.
>> looks like this was an absolutely amazing exchange that even furthered our understanding of wtf is going on.
>> thanks for being a great moderator for MTD rich
>> Thanks,
>> Gagan
> 


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

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

[Zhihao Cheng]

在 2024/6/21 6:06, Gagan Sidhu 写道:
> hi zhihao,
> 
> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
> 

Uhhh, I don't think I mean this. We will make it clear after getting the 
layers' information about your device.
Everything goes well from you guys talking, this patch did reject the 
mtdblock loading from a gluebi device, which may lead booting failed if 
your rootfs depends on a mtdblock(which is generated from the gluebi 
device).
 From your description 'spoke to a user, gave him a build without 
MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it 
booted fine'. One thing I'm curious about, if the device boots from the 
mtdblock(which is generated from gluebi device), the gluebi device won't 
be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then 
the device boots successfully, which means that your rootfs is most 
likely loaded from ubiblock0_0. My questions are:
Q1. According to previous talking, the booting configuration is
https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, 
why the device could boot from ubiblock0_0?(it looks like that the 
device uses config from 
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD 
[pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel 
know much), is that possible the device could automatically choose boot 
device in the openwrt framework?
Q2. Take a step back, if the gluebi device and ubiblock are generated 
from the same UBI volume, there will be error message like 'cannot open 
device %d, volume %d, error -16' if we open gluebi device and ubiblock 
at the same time, but I cannot find the error message from the log.
Q3. If the Q1's answer is 'yes', which means that your device could 
automatically choose to boot rootfs from ubiblock0_0 when it cannot 
dectect a mtdblock(which is generated from the gluebi device), why it 
can fail booting before reverting this patch? I mean, the mtdblock won't 
be generated before reverting this patch, the device should 
automatically choose to boot rootfs from ubiblock0_0, but it does not, 
and the error message in Q2 is not shown in your log.

So, we will answer above questions if we have the layers' information 
about your device('mtdinfo -a' and 'ubinfo -a' after booting the 
device). There are two situations:
1. the layers' information after reverting this patch
2. the layers' information after disable CONFIG_MTD_UBI_GLUEBI

> i was just joking about you being a spy by the way. it was post-fix euphoria ;)

It's nothing, I knew your are not serious.
> 
> master rich, shepherd weinberger, what say ye?
> 
> your wisdom and insight would be greatly appreciated as closure and maybe even a patch to reflect the beauty of collaborating over current ;)
> 
> Thanks,
> Gagan
> 
>> On Jun 17, 2024, at 10:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>
>> 在 2024/6/18 6:13, Gagan Sidhu 写道:
>> Hi,
>>> spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine.
>>
>> May I have the layers' information about mtd/ubi, you can get it by 'mtdinfo -a' and 'ubinfo -a' after booting the device.
>> I guess your device boots from ubiblock0_0. There are two ways loading booting device:
>> 1. mtd(nand)
>>    ubi(holds volume ubi0_0)
>>    mtd12 (gluebi)
>>    mtdblock12  (This way is cut by this patch, so mtdblock12 is not generated, just like Daniel&Richard analyzed)
>> 2. mtd(nand)
>>    ubi(holds volume ubi0_0)
>>    ubiblock0_0
>>
>>> so we need to think about either deprecating GLUEBI or setting an option in the Kconfig that ensures they are mutually exclusive.
>>> gluebi is definitely highjacking the block device created by UBI_BLOCK and adding the MTD_UBIVOLUME flag to it.
>>
>> The gluebi(mtd) and ubiblock could exist on the same UBI volume at the same time, but they cannot be opened at the same time. Here is an example I tested on the local machine:
>>
>>                                              ↗ ubiblock0_0
>> mtd0(nandsim) -> ubi0 (holds volume ubi0_0)
>>                                              ↘ gluebi(mtd1)
>>
>> [root@localhost ~]# ubinfo -a
>> UBI version:                    1
>> Count of UBI devices:           1
>> UBI control device major/minor: 10:61
>> Present UBI devices:            ubi0
>>
>> ubi0
>> Volumes count:                           1
>> Logical eraseblock size:                 126976 bytes, 124.0 KiB
>> Total amount of logical eraseblocks:     8192 (1040187392 bytes, 992.0 MiB)
>> Amount of available logical eraseblocks: 0 (0 bytes)
>> Maximum count of volumes                 128
>> Count of bad physical eraseblocks:       0
>> Count of reserved physical eraseblocks:  160
>> Current maximum erase counter value:     2
>> Minimum input/output unit size:          2048 bytes
>> Character device major/minor:            246:0
>> Present volumes:                         0
>>
>> Volume ID:   0 (on ubi0)
>> Type:        dynamic
>> Alignment:   1
>> Size:        8026 LEBs (1019109376 bytes, 971.8 MiB)
>> State:       OK
>> Name:        vol_a
>> Character device major/minor: 246:1
>> [root@localhost ~]# mtdinfo -a
>> Count of MTD devices:           2
>> Present MTD devices:            mtd0, mtd1
>> Sysfs interface supported:      yes
>>
>> mtd0
>> Name:                           NAND simulator partition 0
>> Type:                           nand
>> Eraseblock size:                131072 bytes, 128.0 KiB
>> Amount of eraseblocks:          8192 (1073741824 bytes, 1024.0 MiB)
>> Minimum input/output unit size: 2048 bytes
>> Sub-page size:                  512 bytes
>> OOB size:                       64 bytes
>> Character device major/minor:   90:0
>> Bad blocks are allowed:         true
>> Device is writable:             true
>>
>> mtd1
>> Name:                           vol_a
>> Type:                           ubi
>> Eraseblock size:                126976 bytes, 124.0 KiB
>> Amount of eraseblocks:          8026 (1019109376 bytes, 971.8 MiB)
>> Minimum input/output unit size: 2048 bytes
>> Sub-page size:                  2048 bytes
>> Character device major/minor:   90:2
>> Bad blocks are allowed:         false
>> Device is writable:             true
>>
>> [root@localhost ~]# lsblk | grep ubi
>> ubiblock0_0 251:0    0 971.9M  0 disk
>>
>>> there is no other explanation.
>>> looks like this was an absolutely amazing exchange that even furthered our understanding of wtf is going on.
>>> thanks for being a great moderator for MTD rich
>>> Thanks,
>>> Gagan
>>
> 
> .
> 


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

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

[Gagan Sidhu]

Thanks,
Gagan

> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
> 
> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>> hi zhihao,
>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
> 
> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
> From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
> Q1. According to previous talking, the booting configuration is
> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition

> Q2. Take a step back, if the gluebi device and ubiblock are generated from the same UBI volume, there will be error message like 'cannot open device %d, volume %d, error -16' if we open gluebi device and ubiblock at the same time, but I cannot find the error message from the log.
it’s in  the very first email about this topic. i posted a full bootlog. look there, it will show you that’s what happened,  but i think it said error -6 not -16
[    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6


> Q3. If the Q1's answer is 'yes', which means that your device could automatically choose to boot rootfs from ubiblock0_0 when it cannot dectect a mtdblock(which is generated from the gluebi device), why it can fail booting before reverting this patch? I mean, the mtdblock won't be generated before reverting this patch, the device should automatically choose to boot rootfs from ubiblock0_0, but it does not, and the error message in 
because it seems to have to do with gluebi and ubi_block doing something to the same partition.

config_mtd_ubi_block is not dependent on config_mtd_ubi_gluebi. i was confused about this too. i thought since both are about block devices, there is a dependency, but there isn’t.

effectively config_mtd_ubi_block can work without config_mtd_ubi_gluebi, but it’s only for read-only filesystems (like squashfs, which is what i’m using)

> Q2 is not shown in your log.

first email. it’s there.
> 
> So, we will answer above questions if we have the layers' information about your device('mtdinfo -a' and 'ubinfo -a' after booting the device). There are two situations:
> 1. the layers' information after reverting this patch
> 2. the layers' information after disable CONFIG_MTD_UBI_GLUEBI
> 
i personally think there’s some kind of interplay between CONFIG_MTD_UBI_GLUEBI and CONFIG_MTD_UBI_BLOCK. it seemed to me that gluebi re-labeled the UBI_BLOCK partition, or added the MTD_UBIVOLUME flag when it did not need to.

i think your question is more applicable outside of the read-only file systems situation. in my specific case, CONFIG_MTD_UBI_BLOCK suffices because of the niche filesystem i am using (squashfs, read only). i think if it was something else (like jffs2, or i am writing to this partition), then your question becomes more relevant.

i suspect you’ll do some thorough evaluation and provide us some much-appreciated insight, because i cannot how CONFIG_MTD_UBI_BLOCK enabled and CONFIG_MTD_UBI_GLUEBI disabled worked.

>> i was just joking about you being a spy by the way. it was post-fix euphoria ;)
> 
> It's nothing, I knew your are not serious.
>> master rich, shepherd weinberger, what say ye?
>> your wisdom and insight would be greatly appreciated as closure and maybe even a patch to reflect the beauty of collaborating over current ;)
>> Thanks,
>> Gagan
>>> On Jun 17, 2024, at 10:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>> 
>>> 在 2024/6/18 6:13, Gagan Sidhu 写道:
>>> Hi,
>>>> spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine.
>>> 
>>> May I have the layers' information about mtd/ubi, you can get it by 'mtdinfo -a' and 'ubinfo -a' after booting the device.
>>> I guess your device boots from ubiblock0_0. There are two ways loading booting device:
>>> 1. mtd(nand)
>>>   ubi(holds volume ubi0_0)
>>>   mtd12 (gluebi)
>>>   mtdblock12  (This way is cut by this patch, so mtdblock12 is not generated, just like Daniel&Richard analyzed)
>>> 2. mtd(nand)
>>>   ubi(holds volume ubi0_0)
>>>   ubiblock0_0
>>> 
>>>> so we need to think about either deprecating GLUEBI or setting an option in the Kconfig that ensures they are mutually exclusive.
>>>> gluebi is definitely highjacking the block device created by UBI_BLOCK and adding the MTD_UBIVOLUME flag to it.
>>> 
>>> The gluebi(mtd) and ubiblock could exist on the same UBI volume at the same time, but they cannot be opened at the same time. Here is an example I tested on the local machine:
>>> 
>>>                                             ↗ ubiblock0_0
>>> mtd0(nandsim) -> ubi0 (holds volume ubi0_0)
>>>                                             ↘ gluebi(mtd1)
>>> 
>>> [root@localhost ~]# ubinfo -a
>>> UBI version:                    1
>>> Count of UBI devices:           1
>>> UBI control device major/minor: 10:61
>>> Present UBI devices:            ubi0
>>> 
>>> ubi0
>>> Volumes count:                           1
>>> Logical eraseblock size:                 126976 bytes, 124.0 KiB
>>> Total amount of logical eraseblocks:     8192 (1040187392 bytes, 992.0 MiB)
>>> Amount of available logical eraseblocks: 0 (0 bytes)
>>> Maximum count of volumes                 128
>>> Count of bad physical eraseblocks:       0
>>> Count of reserved physical eraseblocks:  160
>>> Current maximum erase counter value:     2
>>> Minimum input/output unit size:          2048 bytes
>>> Character device major/minor:            246:0
>>> Present volumes:                         0
>>> 
>>> Volume ID:   0 (on ubi0)
>>> Type:        dynamic
>>> Alignment:   1
>>> Size:        8026 LEBs (1019109376 bytes, 971.8 MiB)
>>> State:       OK
>>> Name:        vol_a
>>> Character device major/minor: 246:1
>>> [root@localhost ~]# mtdinfo -a
>>> Count of MTD devices:           2
>>> Present MTD devices:            mtd0, mtd1
>>> Sysfs interface supported:      yes
>>> 
>>> mtd0
>>> Name:                           NAND simulator partition 0
>>> Type:                           nand
>>> Eraseblock size:                131072 bytes, 128.0 KiB
>>> Amount of eraseblocks:          8192 (1073741824 bytes, 1024.0 MiB)
>>> Minimum input/output unit size: 2048 bytes
>>> Sub-page size:                  512 bytes
>>> OOB size:                       64 bytes
>>> Character device major/minor:   90:0
>>> Bad blocks are allowed:         true
>>> Device is writable:             true
>>> 
>>> mtd1
>>> Name:                           vol_a
>>> Type:                           ubi
>>> Eraseblock size:                126976 bytes, 124.0 KiB
>>> Amount of eraseblocks:          8026 (1019109376 bytes, 971.8 MiB)
>>> Minimum input/output unit size: 2048 bytes
>>> Sub-page size:                  2048 bytes
>>> Character device major/minor:   90:2
>>> Bad blocks are allowed:         false
>>> Device is writable:             true
>>> 
>>> [root@localhost ~]# lsblk | grep ubi
>>> ubiblock0_0 251:0    0 971.9M  0 disk
>>> 
>>>> there is no other explanation.
>>>> looks like this was an absolutely amazing exchange that even furthered our understanding of wtf is going on.
>>>> thanks for being a great moderator for MTD rich
>>>> Thanks,
>>>> Gagan
>>> 
>> .
> 


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

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

[Zhihao Cheng]

在 2024/6/21 10:09, Gagan Sidhu 写道:
> 
> Thanks,
> Gagan
> 
>> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>
>> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>>> hi zhihao,
>>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
>>
>> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
>> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
>>  From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
>> Q1. According to previous talking, the booting configuration is
>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
> yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
> once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition

Oh, maybe I know what has happened. According to the configuration of 
https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, 
openwrt decides to boot rootfs from mtdblock(which is generated from the 
gluebi device), because the mtd char device (which is genertated from 
gluebi device) is found by openwrt. However, this patch stops generating 
mtdblock from gluebi device, so the mounting failed by missed mtdblock. 
After disabling the CONFIG_MTD_UBI_GLUEBI, the gluebi device is not 
generated, so openwrt decides to boot rootfs from ubiblock0_0, then your 
device booted successfully.
The key is the rootfs device judgement in openwrt, openwrt chooses the 
rootfs device according to the existence of mtd char device, openwrt not 
check whether the corresponding mtdblock exists before mouting it. 
Should openwrt check the existence of mtdblock beforing using it? Or 
maybe openwrt could turn to use ubiblock if the mtdblock device is not 
found?
> 
>> Q2. Take a step back, if the gluebi device and ubiblock are generated from the same UBI volume, there will be error message like 'cannot open device %d, volume %d, error -16' if we open gluebi device and ubiblock at the same time, but I cannot find the error message from the log.
> it’s in  the very first email about this topic. i posted a full bootlog. look there, it will show you that’s what happened,  but i think it said error -6 not -16
> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
> 
> 
>> Q3. If the Q1's answer is 'yes', which means that your device could automatically choose to boot rootfs from ubiblock0_0 when it cannot dectect a mtdblock(which is generated from the gluebi device), why it can fail booting before reverting this patch? I mean, the mtdblock won't be generated before reverting this patch, the device should automatically choose to boot rootfs from ubiblock0_0, but it does not, and the error message in
> because it seems to have to do with gluebi and ubi_block doing something to the same partition.
> 
> config_mtd_ubi_block is not dependent on config_mtd_ubi_gluebi. i was confused about this too. i thought since both are about block devices, there is a dependency, but there isn’t.
> 
> effectively config_mtd_ubi_block can work without config_mtd_ubi_gluebi, but it’s only for read-only filesystems (like squashfs, which is what i’m using)
> 
>> Q2 is not shown in your log.
> 
> first email. it’s there.
>>
>> So, we will answer above questions if we have the layers' information about your device('mtdinfo -a' and 'ubinfo -a' after booting the device). There are two situations:
>> 1. the layers' information after reverting this patch
>> 2. the layers' information after disable CONFIG_MTD_UBI_GLUEBI
>>
> i personally think there’s some kind of interplay between CONFIG_MTD_UBI_GLUEBI and CONFIG_MTD_UBI_BLOCK. it seemed to me that gluebi re-labeled the UBI_BLOCK partition, or added the MTD_UBIVOLUME flag when it did not need to.
> 
> i think your question is more applicable outside of the read-only file systems situation. in my specific case, CONFIG_MTD_UBI_BLOCK suffices because of the niche filesystem i am using (squashfs, read only). i think if it was something else (like jffs2, or i am writing to this partition), then your question becomes more relevant.
> 
> i suspect you’ll do some thorough evaluation and provide us some much-appreciated insight, because i cannot how CONFIG_MTD_UBI_BLOCK enabled and CONFIG_MTD_UBI_GLUEBI disabled worked.
> 
>>> i was just joking about you being a spy by the way. it was post-fix euphoria ;)
>>
>> It's nothing, I knew your are not serious.
>>> master rich, shepherd weinberger, what say ye?
>>> your wisdom and insight would be greatly appreciated as closure and maybe even a patch to reflect the beauty of collaborating over current ;)
>>> Thanks,
>>> Gagan
>>>> On Jun 17, 2024, at 10:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>
>>>> 在 2024/6/18 6:13, Gagan Sidhu 写道:
>>>> Hi,
>>>>> spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine.
>>>>
>>>> May I have the layers' information about mtd/ubi, you can get it by 'mtdinfo -a' and 'ubinfo -a' after booting the device.
>>>> I guess your device boots from ubiblock0_0. There are two ways loading booting device:
>>>> 1. mtd(nand)
>>>>    ubi(holds volume ubi0_0)
>>>>    mtd12 (gluebi)
>>>>    mtdblock12  (This way is cut by this patch, so mtdblock12 is not generated, just like Daniel&Richard analyzed)
>>>> 2. mtd(nand)
>>>>    ubi(holds volume ubi0_0)
>>>>    ubiblock0_0
>>>>
>>>>> so we need to think about either deprecating GLUEBI or setting an option in the Kconfig that ensures they are mutually exclusive.
>>>>> gluebi is definitely highjacking the block device created by UBI_BLOCK and adding the MTD_UBIVOLUME flag to it.
>>>>
>>>> The gluebi(mtd) and ubiblock could exist on the same UBI volume at the same time, but they cannot be opened at the same time. Here is an example I tested on the local machine:
>>>>
>>>>                                              ↗ ubiblock0_0
>>>> mtd0(nandsim) -> ubi0 (holds volume ubi0_0)
>>>>                                              ↘ gluebi(mtd1)
>>>>
>>>> [root@localhost ~]# ubinfo -a
>>>> UBI version:                    1
>>>> Count of UBI devices:           1
>>>> UBI control device major/minor: 10:61
>>>> Present UBI devices:            ubi0
>>>>
>>>> ubi0
>>>> Volumes count:                           1
>>>> Logical eraseblock size:                 126976 bytes, 124.0 KiB
>>>> Total amount of logical eraseblocks:     8192 (1040187392 bytes, 992.0 MiB)
>>>> Amount of available logical eraseblocks: 0 (0 bytes)
>>>> Maximum count of volumes                 128
>>>> Count of bad physical eraseblocks:       0
>>>> Count of reserved physical eraseblocks:  160
>>>> Current maximum erase counter value:     2
>>>> Minimum input/output unit size:          2048 bytes
>>>> Character device major/minor:            246:0
>>>> Present volumes:                         0
>>>>
>>>> Volume ID:   0 (on ubi0)
>>>> Type:        dynamic
>>>> Alignment:   1
>>>> Size:        8026 LEBs (1019109376 bytes, 971.8 MiB)
>>>> State:       OK
>>>> Name:        vol_a
>>>> Character device major/minor: 246:1
>>>> [root@localhost ~]# mtdinfo -a
>>>> Count of MTD devices:           2
>>>> Present MTD devices:            mtd0, mtd1
>>>> Sysfs interface supported:      yes
>>>>
>>>> mtd0
>>>> Name:                           NAND simulator partition 0
>>>> Type:                           nand
>>>> Eraseblock size:                131072 bytes, 128.0 KiB
>>>> Amount of eraseblocks:          8192 (1073741824 bytes, 1024.0 MiB)
>>>> Minimum input/output unit size: 2048 bytes
>>>> Sub-page size:                  512 bytes
>>>> OOB size:                       64 bytes
>>>> Character device major/minor:   90:0
>>>> Bad blocks are allowed:         true
>>>> Device is writable:             true
>>>>
>>>> mtd1
>>>> Name:                           vol_a
>>>> Type:                           ubi
>>>> Eraseblock size:                126976 bytes, 124.0 KiB
>>>> Amount of eraseblocks:          8026 (1019109376 bytes, 971.8 MiB)
>>>> Minimum input/output unit size: 2048 bytes
>>>> Sub-page size:                  2048 bytes
>>>> Character device major/minor:   90:2
>>>> Bad blocks are allowed:         false
>>>> Device is writable:             true
>>>>
>>>> [root@localhost ~]# lsblk | grep ubi
>>>> ubiblock0_0 251:0    0 971.9M  0 disk
>>>>
>>>>> there is no other explanation.
>>>>> looks like this was an absolutely amazing exchange that even furthered our understanding of wtf is going on.
>>>>> thanks for being a great moderator for MTD rich
>>>>> Thanks,
>>>>> Gagan
>>>>
>>> .
>>
> 
> .
> 


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

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

[Gagan Sidhu]

> On Jun 20, 2024, at 9:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
> 
> 在 2024/6/21 10:09, Gagan Sidhu 写道:
>> Thanks,
>> Gagan
>>> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>> 
>>> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>>>> hi zhihao,
>>>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
>>> 
>>> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
>>> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
>>> From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
>>> Q1. According to previous talking, the booting configuration is
>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
>> yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
>> once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition
> 
> Oh, maybe I know what has happened. According to the configuration of https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, openwrt decides to boot rootfs from mtdblock(which is generated from the gluebi device), because the mtd char device (which is genertated from gluebi device) is found by openwrt. However, this patch stops generating mtdblock from gluebi device, so the mounting failed by missed mtdblock. After disabling the CONFIG_MTD_UBI_GLUEBI, the gluebi device is not generated, so openwrt decides to boot rootfs from ubiblock0_0, then your device booted successfully.
> The key is the rootfs device judgement in openwrt, openwrt chooses the rootfs device according to the existence of mtd char device, openwrt not check whether the corresponding mtdblock exists before mouting it. Should openwrt check the existence of mtdblock beforing using it? Or maybe openwrt could turn to use ubiblock if the mtdblock device is not found?

as i understand it, the openwrt patch requires the mtdblock device to exist before finding it. it does not rely on gluebi to generate anything.

the mtd char device is not generated from gluebi. openwrt does not use gluebi at all. i was just being paranoid and had too many options enabled.
	-however, my paranoia has illuminated an issue with regards to GLUEBI and UBI_BLOCK’s coexistence.

https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/400-mtd-mtdsplit-support.patch;h=46ef15d127dfb686e4458fd5838c3eaec8aa2cd7;hb=HEAD

openwrt relies on device tree, in this case with attribute “fixed-partition” and additional parameter
openwrt’s split_rootfs_dev merely requires a partition with the label rootfs to be used, and it will automatically create the rootfs (if the splitting criteria) are satisfied.

split_rootfs_dev requires, as a prerequisite, that there is a partition on the flash that can be “split”. after that, it will rely on ubi to create rootfs for boot.
	-i’m a little lazy right now on explaining the details, but trust me it doesn’t need gluebi to create the block device.


>>> Q2. Take a step back, if the gluebi device and ubiblock are generated from the same UBI volume, there will be error message like 'cannot open device %d, volume %d, error -16' if we open gluebi device and ubiblock at the same time, but I cannot find the error message from the log.
>> it’s in  the very first email about this topic. i posted a full bootlog. look there, it will show you that’s what happened,  but i think it said error -6 not -16
>> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
>>> Q3. If the Q1's answer is 'yes', which means that your device could automatically choose to boot rootfs from ubiblock0_0 when it cannot dectect a mtdblock(which is generated from the gluebi device), why it can fail booting before reverting this patch? I mean, the mtdblock won't be generated before reverting this patch, the device should automatically choose to boot rootfs from ubiblock0_0, but it does not, and the error message in
>> because it seems to have to do with gluebi and ubi_block doing something to the same partition.
>> config_mtd_ubi_block is not dependent on config_mtd_ubi_gluebi. i was confused about this too. i thought since both are about block devices, there is a dependency, but there isn’t.
>> effectively config_mtd_ubi_block can work without config_mtd_ubi_gluebi, but it’s only for read-only filesystems (like squashfs, which is what i’m using)
>>> Q2 is not shown in your log.
>> first email. it’s there.
>>> 
>>> So, we will answer above questions if we have the layers' information about your device('mtdinfo -a' and 'ubinfo -a' after booting the device). There are two situations:
>>> 1. the layers' information after reverting this patch
>>> 2. the layers' information after disable CONFIG_MTD_UBI_GLUEBI
>>> 
>> i personally think there’s some kind of interplay between CONFIG_MTD_UBI_GLUEBI and CONFIG_MTD_UBI_BLOCK. it seemed to me that gluebi re-labeled the UBI_BLOCK partition, or added the MTD_UBIVOLUME flag when it did not need to.
>> i think your question is more applicable outside of the read-only file systems situation. in my specific case, CONFIG_MTD_UBI_BLOCK suffices because of the niche filesystem i am using (squashfs, read only). i think if it was something else (like jffs2, or i am writing to this partition), then your question becomes more relevant.
>> i suspect you’ll do some thorough evaluation and provide us some much-appreciated insight, because i cannot how CONFIG_MTD_UBI_BLOCK enabled and CONFIG_MTD_UBI_GLUEBI disabled worked.
>>>> i was just joking about you being a spy by the way. it was post-fix euphoria ;)
>>> 
>>> It's nothing, I knew your are not serious.
>>>> master rich, shepherd weinberger, what say ye?
>>>> your wisdom and insight would be greatly appreciated as closure and maybe even a patch to reflect the beauty of collaborating over current ;)
>>>> Thanks,
>>>> Gagan
>>>>> On Jun 17, 2024, at 10:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>> 
>>>>> 在 2024/6/18 6:13, Gagan Sidhu 写道:
>>>>> Hi,
>>>>>> spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine.
>>>>> 
>>>>> May I have the layers' information about mtd/ubi, you can get it by 'mtdinfo -a' and 'ubinfo -a' after booting the device.
>>>>> I guess your device boots from ubiblock0_0. There are two ways loading booting device:
>>>>> 1. mtd(nand)
>>>>>   ubi(holds volume ubi0_0)
>>>>>   mtd12 (gluebi)
>>>>>   mtdblock12  (This way is cut by this patch, so mtdblock12 is not generated, just like Daniel&Richard analyzed)
>>>>> 2. mtd(nand)
>>>>>   ubi(holds volume ubi0_0)
>>>>>   ubiblock0_0
>>>>> 
>>>>>> so we need to think about either deprecating GLUEBI or setting an option in the Kconfig that ensures they are mutually exclusive.
>>>>>> gluebi is definitely highjacking the block device created by UBI_BLOCK and adding the MTD_UBIVOLUME flag to it.
>>>>> 
>>>>> The gluebi(mtd) and ubiblock could exist on the same UBI volume at the same time, but they cannot be opened at the same time. Here is an example I tested on the local machine:
>>>>> 
>>>>>                                             ↗ ubiblock0_0
>>>>> mtd0(nandsim) -> ubi0 (holds volume ubi0_0)
>>>>>                                             ↘ gluebi(mtd1)
>>>>> 
>>>>> [root@localhost ~]# ubinfo -a
>>>>> UBI version:                    1
>>>>> Count of UBI devices:           1
>>>>> UBI control device major/minor: 10:61
>>>>> Present UBI devices:            ubi0
>>>>> 
>>>>> ubi0
>>>>> Volumes count:                           1
>>>>> Logical eraseblock size:                 126976 bytes, 124.0 KiB
>>>>> Total amount of logical eraseblocks:     8192 (1040187392 bytes, 992.0 MiB)
>>>>> Amount of available logical eraseblocks: 0 (0 bytes)
>>>>> Maximum count of volumes                 128
>>>>> Count of bad physical eraseblocks:       0
>>>>> Count of reserved physical eraseblocks:  160
>>>>> Current maximum erase counter value:     2
>>>>> Minimum input/output unit size:          2048 bytes
>>>>> Character device major/minor:            246:0
>>>>> Present volumes:                         0
>>>>> 
>>>>> Volume ID:   0 (on ubi0)
>>>>> Type:        dynamic
>>>>> Alignment:   1
>>>>> Size:        8026 LEBs (1019109376 bytes, 971.8 MiB)
>>>>> State:       OK
>>>>> Name:        vol_a
>>>>> Character device major/minor: 246:1
>>>>> [root@localhost ~]# mtdinfo -a
>>>>> Count of MTD devices:           2
>>>>> Present MTD devices:            mtd0, mtd1
>>>>> Sysfs interface supported:      yes
>>>>> 
>>>>> mtd0
>>>>> Name:                           NAND simulator partition 0
>>>>> Type:                           nand
>>>>> Eraseblock size:                131072 bytes, 128.0 KiB
>>>>> Amount of eraseblocks:          8192 (1073741824 bytes, 1024.0 MiB)
>>>>> Minimum input/output unit size: 2048 bytes
>>>>> Sub-page size:                  512 bytes
>>>>> OOB size:                       64 bytes
>>>>> Character device major/minor:   90:0
>>>>> Bad blocks are allowed:         true
>>>>> Device is writable:             true
>>>>> 
>>>>> mtd1
>>>>> Name:                           vol_a
>>>>> Type:                           ubi
>>>>> Eraseblock size:                126976 bytes, 124.0 KiB
>>>>> Amount of eraseblocks:          8026 (1019109376 bytes, 971.8 MiB)
>>>>> Minimum input/output unit size: 2048 bytes
>>>>> Sub-page size:                  2048 bytes
>>>>> Character device major/minor:   90:2
>>>>> Bad blocks are allowed:         false
>>>>> Device is writable:             true
>>>>> 
>>>>> [root@localhost ~]# lsblk | grep ubi
>>>>> ubiblock0_0 251:0    0 971.9M  0 disk
>>>>> 
>>>>>> there is no other explanation.
>>>>>> looks like this was an absolutely amazing exchange that even furthered our understanding of wtf is going on.
>>>>>> thanks for being a great moderator for MTD rich
>>>>>> Thanks,
>>>>>> Gagan
>>>>> 
>>>> .
>>> 
>> .
> 


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

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

[Zhihao Cheng]

在 2024/6/21 12:27, Gagan Sidhu 写道:
> 
> 
>> On Jun 20, 2024, at 9:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>
>> 在 2024/6/21 10:09, Gagan Sidhu 写道:
>>> Thanks,
>>> Gagan
>>>> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>
>>>> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>>>>> hi zhihao,
>>>>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
>>>>
>>>> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
>>>> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
>>>>  From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
>>>> Q1. According to previous talking, the booting configuration is
>>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
>>> yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
>>> once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition
>>
>> Oh, maybe I know what has happened. According to the configuration of https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, openwrt decides to boot rootfs from mtdblock(which is generated from the gluebi device), because the mtd char device (which is genertated from gluebi device) is found by openwrt. However, this patch stops generating mtdblock from gluebi device, so the mounting failed by missed mtdblock. After disabling the CONFIG_MTD_UBI_GLUEBI, the gluebi device is not generated, so openwrt decides to boot rootfs from ubiblock0_0, then your device booted successfully.
>> The key is the rootfs device judgement in openwrt, openwrt chooses the rootfs device according to the existence of mtd char device, openwrt not check whether the corresponding mtdblock exists before mouting it. Should openwrt check the existence of mtdblock beforing using it? Or maybe openwrt could turn to use ubiblock if the mtdblock device is not found?
> 
> as i understand it, the openwrt patch requires the mtdblock device to exist before finding it. it does not rely on gluebi to generate anything.

Yes, openwrt knows nothing under the mtd layer(Whatever the mtd is 
generated by a gluebi device or the mtd is a real physical nand flash). 
I mean, the layers' inforamtion on your device could be(The squashfs 
image is stored in UBI volume ubi0_0.):

                                       ↗ ubiblock0_0
mtdX(nand) -> UBI(holds volume ubi0_0)
                                       ↘ mtd12(gluebi) -> mtdblock12

The openwrt only sees ubiblock0_0, mtd12 and mtdblock12(The ubi0_0 and 
gluebi are not awared by openwrt). The openwrt detects that mtd12 has 
label 'linux,rootfs' in device tree, so it tries mouting mtdblock12 
without checking the existence of mtdblock12.
Could that be possible?

> 
> the mtd char device is not generated from gluebi. openwrt does not use gluebi at all. i was just being paranoid and had too many options enabled.
> 	-however, my paranoia has illuminated an issue with regards to GLUEBI and UBI_BLOCK’s coexistence.
> 
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/400-mtd-mtdsplit-support.patch;h=46ef15d127dfb686e4458fd5838c3eaec8aa2cd7;hb=HEAD
> 
> openwrt relies on device tree, in this case with attribute “fixed-partition” and additional parameter
> openwrt’s split_rootfs_dev merely requires a partition with the label rootfs to be used, and it will automatically create the rootfs (if the splitting criteria) are satisfied.
> 
> split_rootfs_dev requires, as a prerequisite, that there is a partition on the flash that can be “split”. after that, it will rely on ubi to create rootfs for boot.
> 	-i’m a little lazy right now on explaining the details, but trust me it doesn’t need gluebi to create the block device.
> 
> 



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

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

[Gagan Sidhu]

> On Jun 20, 2024, at 10:55 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
> 
> 在 2024/6/21 12:27, Gagan Sidhu 写道:
>>> On Jun 20, 2024, at 9:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>> 
>>> 在 2024/6/21 10:09, Gagan Sidhu 写道:
>>>> Thanks,
>>>> Gagan
>>>>> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>> 
>>>>> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>>>>>> hi zhihao,
>>>>>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
>>>>> 
>>>>> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
>>>>> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
>>>>> From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
>>>>> Q1. According to previous talking, the booting configuration is
>>>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
>>>> yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
>>>> once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition
>>> 
>>> Oh, maybe I know what has happened. According to the configuration of https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, openwrt decides to boot rootfs from mtdblock(which is generated from the gluebi device), because the mtd char device (which is genertated from gluebi device) is found by openwrt. However, this patch stops generating mtdblock from gluebi device, so the mounting failed by missed mtdblock. After disabling the CONFIG_MTD_UBI_GLUEBI, the gluebi device is not generated, so openwrt decides to boot rootfs from ubiblock0_0, then your device booted successfully.
>>> The key is the rootfs device judgement in openwrt, openwrt chooses the rootfs device according to the existence of mtd char device, openwrt not check whether the corresponding mtdblock exists before mouting it. Should openwrt check the existence of mtdblock beforing using it? Or maybe openwrt could turn to use ubiblock if the mtdblock device is not found?
>> as i understand it, the openwrt patch requires the mtdblock device to exist before finding it. it does not rely on gluebi to generate anything.
> 
> Yes, openwrt knows nothing under the mtd layer(Whatever the mtd is generated by a gluebi device or the mtd is a real physical nand flash). I mean, the layers' inforamtion on your device could be(The squashfs image is stored in UBI volume ubi0_0.):
> 
>                                      ↗ ubiblock0_0
> mtdX(nand) -> UBI(holds volume ubi0_0)
>                                      ↘ mtd12(gluebi) -> mtdblock12
> 
> The openwrt only sees ubiblock0_0, mtd12 and mtdblock12(The ubi0_0 and gluebi are not awared by openwrt). The openwrt detects that mtd12 has label 'linux,rootfs' in device tree, so it tries mouting mtdblock12 without checking the existence of mtdblock12.
> Could that be possible?

just to share the relevant part of the log again:
```
[    3.188484] 9 fixed-partitions partitions found on MTD device MT7621-NAND
[    3.202005] Creating 9 MTD partitions on "MT7621-NAND":
[    3.212430] 0x000000000000-0x000000080000 : "Bootloader"
[    3.224024] 0x0000000c0000-0x000000100000 : "Config"
[    3.234684] 0x000000100000-0x000000140000 : "Factory"
[    3.245518] 0x000000140000-0x000000180000 : "Config2"
[    3.256379] 0x000000180000-0x000002d80000 : "sysv"
[    3.895176] 1 squashfs-split partitions found on MTD device sysv
[    3.907164] 0x0000005c1000-0x000002d60000 : "ddwrt"
[    3.920925] 2 uimage-fw partitions found on MTD device sysv
[    3.932031] Creating 2 MTD partitions on "sysv":
[    3.941232] 0x000000000000-0x000000400000 : "kernel"
[    3.951995] 0x000000400000-0x000002c00000 : "ubi"
[    3.962325] 0x000002d80000-0x000004d80000 : "private"
[    3.973322] 0x000004d80000-0x000007580000 : "firmware2"
[    3.984759] 0x000007580000-0x000007b80000 : "mydlink"
[    3.995699] 0x000007b80000-0x000008000000 : "reserved"
[    4.006687] [mtk_nand] probe successfully!
```
and

```
[    5.462504] auto-attach mtd7
[    5.462525] ubi0: default fastmap pool size: 15
[    5.477309] ubi0: default fastmap WL pool size: 7
[    5.486683] ubi0: attaching mtd7
[    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
[    5.811299] ubi0: scanning is finished
[    5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
[    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
[    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
[    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
[    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
[    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
[    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
[    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
[    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
[    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
[    6.021426] rootfs: parsing partitions cmdlinepart
[    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
[    6.043694] rootfs: got parser (null)
[    6.051426] mtd: device 12 (rootfs) set to be root filesystem
[    6.062891] rootfs_data: parsing partitions cmdlinepart
[    6.073669] rootfs_data: got parser (null)
[    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
[    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
[    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
[    6.297406] Please append a correct "root=" boot option; here are the available partitions:
[    6.314054] 1f00             512 mtdblock0
[    6.314060]  (driver?)
[    6.327077] 1f01             256 mtdblock1
[    6.327081]  (driver?)
[    6.340101] 1f02             256 mtdblock2
[    6.340105]  (driver?)
[    6.353124] 1f03             256 mtdblock3
[    6.353129]  (driver?)
[    6.366153] 1f04           45056 mtdblock4
[    6.366158]  (driver?)
[    6.379175] 1f05           40572 mtdblock5
[    6.379179]  (driver?)
[    6.392217] 1f06            4096 mtdblock6
[    6.392222]  (driver?)
[    6.405240] 1f07           40960 mtdblock7
[    6.405244]  (driver?)
[    6.418272] 1f08           32768 mtdblock8
[    6.418277]  (driver?)
[    6.431296] 1f09           40960 mtdblock9
[    6.431300]  (driver?)
[    6.444324] 1f0a            6144 mtdblock10
[    6.444328]  (driver?)
[    6.457518] 1f0b            4608 mtdblock11
[    6.457523]  (driver?)
[    6.470720] fe00           33604 ubiblock0_0
[    6.470724]  (driver?)
[    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)
```
openwrt does see ubi0_0 because it has to, in order to create ubiblock0_0.

regarding openwrt not checking the existence of mtdblock12: i think what richard said earlier was that ubiblock0_0 _is_ mtdblock12. as i understand it, the ubiblock0_0 is merely a convenient label for /dev/mtdblock12

that’s why i was complaining and posted the error log, because VFS seemed to be mounting the right partition but was failing.

i could be wrong on this, though.

> 
>> the mtd char device is not generated from gluebi. openwrt does not use gluebi at all. i was just being paranoid and had too many options enabled.
>> 	-however, my paranoia has illuminated an issue with regards to GLUEBI and UBI_BLOCK’s coexistence.
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/400-mtd-mtdsplit-support.patch;h=46ef15d127dfb686e4458fd5838c3eaec8aa2cd7;hb=HEAD
>> openwrt relies on device tree, in this case with attribute “fixed-partition” and additional parameter
>> openwrt’s split_rootfs_dev merely requires a partition with the label rootfs to be used, and it will automatically create the rootfs (if the splitting criteria) are satisfied.
>> split_rootfs_dev requires, as a prerequisite, that there is a partition on the flash that can be “split”. after that, it will rely on ubi to create rootfs for boot.
>> 	-i’m a little lazy right now on explaining the details, but trust me it doesn’t need gluebi to create the block device.


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

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

[Zhihao Cheng]

在 2024/6/21 19:36, Gagan Sidhu 写道:
> 
>> On Jun 20, 2024, at 10:55 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>
>> 在 2024/6/21 12:27, Gagan Sidhu 写道:
>>>> On Jun 20, 2024, at 9:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>
>>>> 在 2024/6/21 10:09, Gagan Sidhu 写道:
>>>>> Thanks,
>>>>> Gagan
>>>>>> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>>>
>>>>>> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>>>>>>> hi zhihao,
>>>>>>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
>>>>>>
>>>>>> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
>>>>>> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
>>>>>>  From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
>>>>>> Q1. According to previous talking, the booting configuration is
>>>>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
>>>>> yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
>>>>> once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition
>>>>
>>>> Oh, maybe I know what has happened. According to the configuration of https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, openwrt decides to boot rootfs from mtdblock(which is generated from the gluebi device), because the mtd char device (which is genertated from gluebi device) is found by openwrt. However, this patch stops generating mtdblock from gluebi device, so the mounting failed by missed mtdblock. After disabling the CONFIG_MTD_UBI_GLUEBI, the gluebi device is not generated, so openwrt decides to boot rootfs from ubiblock0_0, then your device booted successfully.
>>>> The key is the rootfs device judgement in openwrt, openwrt chooses the rootfs device according to the existence of mtd char device, openwrt not check whether the corresponding mtdblock exists before mouting it. Should openwrt check the existence of mtdblock beforing using it? Or maybe openwrt could turn to use ubiblock if the mtdblock device is not found?
>>> as i understand it, the openwrt patch requires the mtdblock device to exist before finding it. it does not rely on gluebi to generate anything.
>>
>> Yes, openwrt knows nothing under the mtd layer(Whatever the mtd is generated by a gluebi device or the mtd is a real physical nand flash). I mean, the layers' inforamtion on your device could be(The squashfs image is stored in UBI volume ubi0_0.):
>>
>>                                       ↗ ubiblock0_0
>> mtdX(nand) -> UBI(holds volume ubi0_0)
>>                                       ↘ mtd12(gluebi) -> mtdblock12
>>
>> The openwrt only sees ubiblock0_0, mtd12 and mtdblock12(The ubi0_0 and gluebi are not awared by openwrt). The openwrt detects that mtd12 has label 'linux,rootfs' in device tree, so it tries mouting mtdblock12 without checking the existence of mtdblock12.
>> Could that be possible?
> 
> just to share the relevant part of the log again:
> ```
> [    3.188484] 9 fixed-partitions partitions found on MTD device MT7621-NAND
> [    3.202005] Creating 9 MTD partitions on "MT7621-NAND":
> [    3.212430] 0x000000000000-0x000000080000 : "Bootloader"
> [    3.224024] 0x0000000c0000-0x000000100000 : "Config"
> [    3.234684] 0x000000100000-0x000000140000 : "Factory"
> [    3.245518] 0x000000140000-0x000000180000 : "Config2"
> [    3.256379] 0x000000180000-0x000002d80000 : "sysv"
> [    3.895176] 1 squashfs-split partitions found on MTD device sysv
> [    3.907164] 0x0000005c1000-0x000002d60000 : "ddwrt"
> [    3.920925] 2 uimage-fw partitions found on MTD device sysv
> [    3.932031] Creating 2 MTD partitions on "sysv":
> [    3.941232] 0x000000000000-0x000000400000 : "kernel"
> [    3.951995] 0x000000400000-0x000002c00000 : "ubi"
> [    3.962325] 0x000002d80000-0x000004d80000 : "private"
> [    3.973322] 0x000004d80000-0x000007580000 : "firmware2"
> [    3.984759] 0x000007580000-0x000007b80000 : "mydlink"
> [    3.995699] 0x000007b80000-0x000008000000 : "reserved"
> [    4.006687] [mtk_nand] probe successfully!
> ```
> and
> 
> ```
> [    5.462504] auto-attach mtd7
> [    5.462525] ubi0: default fastmap pool size: 15
> [    5.477309] ubi0: default fastmap WL pool size: 7
> [    5.486683] ubi0: attaching mtd7
> [    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
> [    5.811299] ubi0: scanning is finished
> [    5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
> [    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
> [    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
> [    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
> [    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
> [    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
> [    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
> [    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
> [    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
> [    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
> [    6.021426] rootfs: parsing partitions cmdlinepart
> [    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
> [    6.043694] rootfs: got parser (null)
> [    6.051426] mtd: device 12 (rootfs) set to be root filesystem
> [    6.062891] rootfs_data: parsing partitions cmdlinepart
> [    6.073669] rootfs_data: got parser (null)
> [    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
> [    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
> [    6.297406] Please append a correct "root=" boot option; here are the available partitions:
> [    6.314054] 1f00             512 mtdblock0
> [    6.314060]  (driver?)
> [    6.327077] 1f01             256 mtdblock1
> [    6.327081]  (driver?)
> [    6.340101] 1f02             256 mtdblock2
> [    6.340105]  (driver?)
> [    6.353124] 1f03             256 mtdblock3
> [    6.353129]  (driver?)
> [    6.366153] 1f04           45056 mtdblock4
> [    6.366158]  (driver?)
> [    6.379175] 1f05           40572 mtdblock5
> [    6.379179]  (driver?)
> [    6.392217] 1f06            4096 mtdblock6
> [    6.392222]  (driver?)
> [    6.405240] 1f07           40960 mtdblock7
> [    6.405244]  (driver?)
> [    6.418272] 1f08           32768 mtdblock8
> [    6.418277]  (driver?)
> [    6.431296] 1f09           40960 mtdblock9
> [    6.431300]  (driver?)
> [    6.444324] 1f0a            6144 mtdblock10
> [    6.444328]  (driver?)
> [    6.457518] 1f0b            4608 mtdblock11
> [    6.457523]  (driver?)
> [    6.470720] fe00           33604 ubiblock0_0
> [    6.470724]  (driver?)
> [    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)
> ```
> openwrt does see ubi0_0 because it has to, in order to create ubiblock0_0.
> 
> regarding openwrt not checking the existence of mtdblock12: i think what richard said earlier was that ubiblock0_0 _is_ mtdblock12. as i understand it, the ubiblock0_0 is merely a convenient label for /dev/mtdblock12

I think they are two different devices, according above log:
1. 'block ubiblock0_0: created from ubi0:0(rootfs)', which comes from 
https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/block.c#L430, 
and it means that the ubiblock0_0 is generated from volume ubi0_0(which 
has label rootfs)
2. 'VFS: Unable to mount root fs on unknown-block(31,12)', it means that 
the major device number is 31, it should be a mtdblock according to 
https://github.com/torvalds/linux/blob/master/include/uapi/linux/major.h#L56. 
The mtdblock device is generated from a mtd char device, according to 
https://github.com/torvalds/linux/blob/master/drivers/mtd/mtd_blkdevs.c#L350.
> 
> that’s why i was complaining and posted the error log, because VFS seemed to be mounting the right partition but was failing.
> 
> i could be wrong on this, though.
> 
>>
>>> the mtd char device is not generated from gluebi. openwrt does not use gluebi at all. i was just being paranoid and had too many options enabled.
>>> 	-however, my paranoia has illuminated an issue with regards to GLUEBI and UBI_BLOCK’s coexistence.
>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/400-mtd-mtdsplit-support.patch;h=46ef15d127dfb686e4458fd5838c3eaec8aa2cd7;hb=HEAD
>>> openwrt relies on device tree, in this case with attribute “fixed-partition” and additional parameter
>>> openwrt’s split_rootfs_dev merely requires a partition with the label rootfs to be used, and it will automatically create the rootfs (if the splitting criteria) are satisfied.
>>> split_rootfs_dev requires, as a prerequisite, that there is a partition on the flash that can be “split”. after that, it will rely on ubi to create rootfs for boot.
>>> 	-i’m a little lazy right now on explaining the details, but trust me it doesn’t need gluebi to create the block device.
> 
> .
> 


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

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

[Gagan Sidhu]

> On Jun 21, 2024, at 8:37 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
> 
> 在 2024/6/21 19:36, Gagan Sidhu 写道:
>>> On Jun 20, 2024, at 10:55 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>> 
>>> 在 2024/6/21 12:27, Gagan Sidhu 写道:
>>>>> On Jun 20, 2024, at 9:03 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>> 
>>>>> 在 2024/6/21 10:09, Gagan Sidhu 写道:
>>>>>> Thanks,
>>>>>> Gagan
>>>>>>> On Jun 20, 2024, at 7:59 PM, Zhihao Cheng <chengzhihao1@huawei.com> wrote:
>>>>>>> 
>>>>>>> 在 2024/6/21 6:06, Gagan Sidhu 写道:
>>>>>>>> hi zhihao,
>>>>>>>> so i assume my crude paraphrase is correct? that i may have unintentionally pointed the finger at you, but the real issue is GLUEBI existing with BLOCK on the same volume?
>>>>>>> 
>>>>>>> Uhhh, I don't think I mean this. We will make it clear after getting the layers' information about your device.
>>>>>>> Everything goes well from you guys talking, this patch did reject the mtdblock loading from a gluebi device, which may lead booting failed if your rootfs depends on a mtdblock(which is generated from the gluebi device).
>>>>>>> From your description 'spoke to a user, gave him a build without MTD_GLUEBI, restoring changes made by (HAHAHA you are! huawei), it booted fine'. One thing I'm curious about, if the device boots from the mtdblock(which is generated from gluebi device), the gluebi device won't be generated because you have turned off the CONFIG_MTD_UBI_GLUEBI, then the device boots successfully, which means that your rootfs is most likely loaded from ubiblock0_0. My questions are:
>>>>>>> Q1. According to previous talking, the booting configuration is
>>>>>>> https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, why the device could boot from ubiblock0_0?(it looks like that the device uses config from https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/493-ubi-set-ROOT_DEV-to-ubiblock-rootfs-if-unset.patch;h=266a6331c2acc0f7c17d9ac72f54659d31b56249;hb=HEAD [pointed by Richard]). I'm unfamilar with openwrt(Maybe you and Daniel know much), is that possible the device could automatically choose boot device in the openwrt framework?
>>>>>> yes, that patch will, effectively, allow an “autoselect” of the ubi partition because it looks for the string “rootfs” in the ubi-formatted file.
>>>>>> once it finds the “rootfs” partition, it will rename it to “ubi” and that will be set as the boot/root partition
>>>>> 
>>>>> Oh, maybe I know what has happened. According to the configuration of https://github.com/torvalds/linux/blob/master/drivers/mtd/mtdcore.c#L774, openwrt decides to boot rootfs from mtdblock(which is generated from the gluebi device), because the mtd char device (which is genertated from gluebi device) is found by openwrt. However, this patch stops generating mtdblock from gluebi device, so the mounting failed by missed mtdblock. After disabling the CONFIG_MTD_UBI_GLUEBI, the gluebi device is not generated, so openwrt decides to boot rootfs from ubiblock0_0, then your device booted successfully.
>>>>> The key is the rootfs device judgement in openwrt, openwrt chooses the rootfs device according to the existence of mtd char device, openwrt not check whether the corresponding mtdblock exists before mouting it. Should openwrt check the existence of mtdblock beforing using it? Or maybe openwrt could turn to use ubiblock if the mtdblock device is not found?
>>>> as i understand it, the openwrt patch requires the mtdblock device to exist before finding it. it does not rely on gluebi to generate anything.
>>> 
>>> Yes, openwrt knows nothing under the mtd layer(Whatever the mtd is generated by a gluebi device or the mtd is a real physical nand flash). I mean, the layers' inforamtion on your device could be(The squashfs image is stored in UBI volume ubi0_0.):
>>> 
>>>                                      ↗ ubiblock0_0
>>> mtdX(nand) -> UBI(holds volume ubi0_0)
>>>                                      ↘ mtd12(gluebi) -> mtdblock12
>>> 
>>> The openwrt only sees ubiblock0_0, mtd12 and mtdblock12(The ubi0_0 and gluebi are not awared by openwrt). The openwrt detects that mtd12 has label 'linux,rootfs' in device tree, so it tries mouting mtdblock12 without checking the existence of mtdblock12.
>>> Could that be possible?
>> just to share the relevant part of the log again:
>> ```
>> [    3.188484] 9 fixed-partitions partitions found on MTD device MT7621-NAND
>> [    3.202005] Creating 9 MTD partitions on "MT7621-NAND":
>> [    3.212430] 0x000000000000-0x000000080000 : "Bootloader"
>> [    3.224024] 0x0000000c0000-0x000000100000 : "Config"
>> [    3.234684] 0x000000100000-0x000000140000 : "Factory"
>> [    3.245518] 0x000000140000-0x000000180000 : "Config2"
>> [    3.256379] 0x000000180000-0x000002d80000 : "sysv"
>> [    3.895176] 1 squashfs-split partitions found on MTD device sysv
>> [    3.907164] 0x0000005c1000-0x000002d60000 : "ddwrt"
>> [    3.920925] 2 uimage-fw partitions found on MTD device sysv
>> [    3.932031] Creating 2 MTD partitions on "sysv":
>> [    3.941232] 0x000000000000-0x000000400000 : "kernel"
>> [    3.951995] 0x000000400000-0x000002c00000 : "ubi"
>> [    3.962325] 0x000002d80000-0x000004d80000 : "private"
>> [    3.973322] 0x000004d80000-0x000007580000 : "firmware2"
>> [    3.984759] 0x000007580000-0x000007b80000 : "mydlink"
>> [    3.995699] 0x000007b80000-0x000008000000 : "reserved"
>> [    4.006687] [mtk_nand] probe successfully!
>> ```
>> and
>> ```
>> [    5.462504] auto-attach mtd7
>> [    5.462525] ubi0: default fastmap pool size: 15
>> [    5.477309] ubi0: default fastmap WL pool size: 7
>> [    5.486683] ubi0: attaching mtd7
>> [    5.811240] UBI: EOF marker found, PEBs from 273 will be erased
>> [    5.811299] ubi0: scanning is finished
>> [    5.874546] gluebi (pid 1): gluebi_resized: got update notification for unknown UBI device 0 volume 1
>> [    5.892927] ubi0: volume 1 ("rootfs_data") re-sized from 9 to 28 LEBs
>> [    5.906683] ubi0: attached mtd7 (name "ubi", size 40 MiB)
>> [    5.917446] ubi0: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
>> [    5.931132] ubi0: min./max. I/O unit sizes: 2048/2048, sub-page size 2048
>> [    5.944654] ubi0: VID header offset: 2048 (aligned 2048), data offset: 4096
>> [    5.958513] ubi0: good PEBs: 320, bad PEBs: 0, corrupted PEBs: 0
>> [    5.970472] ubi0: user volume: 2, internal volumes: 1, max. volumes count: 128
>> [    5.984859] ubi0: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 1613475955
>> [    6.003045] ubi0: available PEBs: 0, total reserved PEBs: 320, PEBs reserved for bad PEB handling: 15
>> [    6.021426] rootfs: parsing partitions cmdlinepart
>> [    6.021444] ubi0: background thread "ubi_bgt0d" started, PID 97
>> [    6.043694] rootfs: got parser (null)
>> [    6.051426] mtd: device 12 (rootfs) set to be root filesystem
>> [    6.062891] rootfs_data: parsing partitions cmdlinepart
>> [    6.073669] rootfs_data: got parser (null)
>> [    6.211240] block ubiblock0_0: created from ubi0:0(rootfs)
>> [    6.259545] rtc-pcf8563 0-0051: hctosys: unable to read the hardware clock
>> [    6.282125] VFS: Cannot open root device "(null)" or unknown-block(31,12): error -6
>> [    6.297406] Please append a correct "root=" boot option; here are the available partitions:
>> [    6.314054] 1f00             512 mtdblock0
>> [    6.314060]  (driver?)
>> [    6.327077] 1f01             256 mtdblock1
>> [    6.327081]  (driver?)
>> [    6.340101] 1f02             256 mtdblock2
>> [    6.340105]  (driver?)
>> [    6.353124] 1f03             256 mtdblock3
>> [    6.353129]  (driver?)
>> [    6.366153] 1f04           45056 mtdblock4
>> [    6.366158]  (driver?)
>> [    6.379175] 1f05           40572 mtdblock5
>> [    6.379179]  (driver?)
>> [    6.392217] 1f06            4096 mtdblock6
>> [    6.392222]  (driver?)
>> [    6.405240] 1f07           40960 mtdblock7
>> [    6.405244]  (driver?)
>> [    6.418272] 1f08           32768 mtdblock8
>> [    6.418277]  (driver?)
>> [    6.431296] 1f09           40960 mtdblock9
>> [    6.431300]  (driver?)
>> [    6.444324] 1f0a            6144 mtdblock10
>> [    6.444328]  (driver?)
>> [    6.457518] 1f0b            4608 mtdblock11
>> [    6.457523]  (driver?)
>> [    6.470720] fe00           33604 ubiblock0_0
>> [    6.470724]  (driver?)
>> [    6.484090] Kernel panic - not syncing: VFS: Unable to mount root fs on unknown-block(31,12)
>> ```
>> openwrt does see ubi0_0 because it has to, in order to create ubiblock0_0.
>> regarding openwrt not checking the existence of mtdblock12: i think what richard said earlier was that ubiblock0_0 _is_ mtdblock12. as i understand it, the ubiblock0_0 is merely a convenient label for /dev/mtdblock12
> 
> I think they are two different devices, according above log:
> 1. 'block ubiblock0_0: created from ubi0:0(rootfs)', which comes from https://github.com/torvalds/linux/blob/master/drivers/mtd/ubi/block.c#L430, and it means that the ubiblock0_0 is generated from volume ubi0_0(which has label rootfs)
> 2. 'VFS: Unable to mount root fs on unknown-block(31,12)', it means that the major device number is 31, it should be a mtdblock according to https://github.com/torvalds/linux/blob/master/include/uapi/linux/major.h#L56. The mtdblock device is generated from a mtd char device, according to https://github.com/torvalds/linux/blob/master/drivers/mtd/mtd_blkdevs.c#L350.

if the user’s expectation is two separately created devices (something i don’t dispute), then why does the MTD_UBIVOLUME flag interfere? 
	- it shouldn’t affect ubiblock0_0 and the openwrt patch should work without issue.
		-MTD_UBIVOLUME should only be set on the other device we expected to be created by GLUEBI, right?

as i said, the issue arose/arises when GLUEBI and UBI_BLOCK are both enabled.

if ubiblock0_0 is its own device, created from UBI_BLOCK, how come having GLUEBI enabled results in the failure of the mount? 

GLUEBI should be operating on the /dev/mtdblockX device and not ubiblock0_0 and thus the boot procedure created by openwrt should be unaffected.

at least that’s how i would understand the situation if each are creating their own devices.

we need to figure out a solution or maybe master rich should end our ongoing discussion because technically “it’s not mainline” and we’re probably boring the recipients.


>> that’s why i was complaining and posted the error log, because VFS seemed to be mounting the right partition but was failing.
>> i could be wrong on this, though.
>>> 
>>>> the mtd char device is not generated from gluebi. openwrt does not use gluebi at all. i was just being paranoid and had too many options enabled.
>>>> 	-however, my paranoia has illuminated an issue with regards to GLUEBI and UBI_BLOCK’s coexistence.
>>>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/pending-5.15/400-mtd-mtdsplit-support.patch;h=46ef15d127dfb686e4458fd5838c3eaec8aa2cd7;hb=HEAD
>>>> openwrt relies on device tree, in this case with attribute “fixed-partition” and additional parameter
>>>> openwrt’s split_rootfs_dev merely requires a partition with the label rootfs to be used, and it will automatically create the rootfs (if the splitting criteria) are satisfied.
>>>> split_rootfs_dev requires, as a prerequisite, that there is a partition on the flash that can be “split”. after that, it will rely on ubi to create rootfs for boot.
>>>> 	-i’m a little lazy right now on explaining the details, but trust me it doesn’t need gluebi to create the block device.
>> .


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

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

[Daniel Golle]

On Fri, Jun 21, 2024 at 08:43:47PM -0600, Gagan Sidhu wrote:
> [...]
> GLUEBI should be operating on the /dev/mtdblockX device and not ubiblock0_0 and thus the boot procedure created by openwrt should be unaffected.
> 
> at least that’s how i would understand the situation if each are creating their own devices.
> 
> we need to figure out a solution or maybe master rich should end our ongoing discussion because technically “it’s not mainline” and we’re probably boring the recipients.

I've suggested a simple solution here:

https://lkml.org/lkml/2024/6/17/1602

You could try that and let us know if it resolves the issue for you.

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

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

[Gagan Sidhu]

actually the issue was resolved. we are discussing unexpected behaviour when both MTD_UBI_GLUEBI and MTD_UBI_BLOCK are enabled.

disabling MTD_UBI_GLUEBI fixed my issue, and everything mounted as expected afterwards.

the question was why ubiblock0_0 was not being mounted when MTD_UBI_GLUEBI was enabled, and the evidence suggested it was somehow being assigned the label MTD_UBIVOLUME

but if no one cares that’s fine lol

Thanks,
Gagan

> On Jun 22, 2024, at 3:07 PM, Daniel Golle <daniel@makrotopia.org> wrote:
> 
> On Fri, Jun 21, 2024 at 08:43:47PM -0600, Gagan Sidhu wrote:
>> [...]
>> GLUEBI should be operating on the /dev/mtdblockX device and not ubiblock0_0 and thus the boot procedure created by openwrt should be unaffected.
>> 
>> at least that’s how i would understand the situation if each are creating their own devices.
>> 
>> we need to figure out a solution or maybe master rich should end our ongoing discussion because technically “it’s not mainline” and we’re probably boring the recipients.
> 
> I've suggested a simple solution here:
> 
> https://lkml.org/lkml/2024/6/17/1602
> 
> You could try that and let us know if it resolves the issue for you.


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