public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
@ 2023-10-18 12:16 ZhaoLong Wang
  2023-10-19  1:57 ` Zhihao Cheng
  2023-10-19 20:27 ` Richard Weinberger
  0 siblings, 2 replies; 41+ messages in thread
From: ZhaoLong Wang @ 2023-10-18 12:16 UTC (permalink / raw)
  To: richard, miquel.raynal, vigneshr, dpervushin, Artem.Bityutskiy
  Cc: linux-mtd, linux-kernel, chengzhihao1, wangzhaolong1, yi.zhang,
	yangerkun

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-18 12:16 ZhaoLong Wang
@ 2023-10-19  1:57 ` Zhihao Cheng
  2023-10-19 20:27 ` Richard Weinberger
  1 sibling, 0 replies; 41+ messages in thread
From: Zhihao Cheng @ 2023-10-19  1:57 UTC (permalink / raw)
  To: ZhaoLong Wang, richard, miquel.raynal, vigneshr, dpervushin,
	Artem.Bityutskiy
  Cc: linux-mtd, linux-kernel, yi.zhang, yangerkun

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-18 12:16 ZhaoLong Wang
  2023-10-19  1:57 ` Zhihao Cheng
@ 2023-10-19 20:27 ` Richard Weinberger
  2023-10-20  2:27   ` Zhihao Cheng
  1 sibling, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2023-10-19 20:27 UTC (permalink / raw)
  To: ZhaoLong Wang
  Cc: Miquel Raynal, Vignesh Raghavendra, dpervushin, Artem Bityutskiy,
	linux-mtd, linux-kernel, chengzhihao1, yi zhang, yangerkun

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-19 20:27 ` Richard Weinberger
@ 2023-10-20  2:27   ` Zhihao Cheng
  2023-10-21 16:09     ` Richard Weinberger
  0 siblings, 1 reply; 41+ messages in thread
From: Zhihao Cheng @ 2023-10-20  2:27 UTC (permalink / raw)
  To: Richard Weinberger, ZhaoLong Wang
  Cc: Miquel Raynal, Vignesh Raghavendra, dpervushin, Artem Bityutskiy,
	linux-mtd, linux-kernel, yi zhang, yangerkun

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-20  2:27   ` Zhihao Cheng
@ 2023-10-21 16:09     ` Richard Weinberger
  2023-10-23  6:41       ` ZhaoLong Wang
  2023-10-23  7:09       ` Zhihao Cheng
  0 siblings, 2 replies; 41+ messages in thread
From: Richard Weinberger @ 2023-10-21 16:09 UTC (permalink / raw)
  To: chengzhihao1
  Cc: ZhaoLong Wang, Miquel Raynal, Vignesh Raghavendra, dpervushin,
	Artem Bityutskiy, linux-mtd, linux-kernel, yi zhang, yangerkun

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-21 16:09     ` Richard Weinberger
@ 2023-10-23  6:41       ` ZhaoLong Wang
  2023-10-23  6:46         ` Richard Weinberger
  2023-10-23  7:09       ` Zhihao Cheng
  1 sibling, 1 reply; 41+ messages in thread
From: ZhaoLong Wang @ 2023-10-23  6:41 UTC (permalink / raw)
  To: Richard Weinberger, chengzhihao1
  Cc: Miquel Raynal, Vignesh Raghavendra, dpervushin, Artem Bityutskiy,
	linux-mtd, linux-kernel, yi zhang, yangerkun

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-23  6:41       ` ZhaoLong Wang
@ 2023-10-23  6:46         ` Richard Weinberger
  2023-10-23  7:12           ` ZhaoLong Wang
  0 siblings, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2023-10-23  6:46 UTC (permalink / raw)
  To: ZhaoLong Wang
  Cc: chengzhihao1, Miquel Raynal, Vignesh Raghavendra, dpervushin,
	Artem Bityutskiy, linux-mtd, linux-kernel, yi zhang, yangerkun

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-21 16:09     ` Richard Weinberger
  2023-10-23  6:41       ` ZhaoLong Wang
@ 2023-10-23  7:09       ` Zhihao Cheng
  2023-10-23  7:15         ` Richard Weinberger
  1 sibling, 1 reply; 41+ messages in thread
From: Zhihao Cheng @ 2023-10-23  7:09 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: ZhaoLong Wang, Miquel Raynal, Vignesh Raghavendra, dpervushin,
	Artem Bityutskiy, linux-mtd, linux-kernel, yi zhang, yangerkun

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-23  6:46         ` Richard Weinberger
@ 2023-10-23  7:12           ` ZhaoLong Wang
  2023-10-23  7:16             ` Richard Weinberger
  0 siblings, 1 reply; 41+ messages in thread
From: ZhaoLong Wang @ 2023-10-23  7:12 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: chengzhihao1, Miquel Raynal, Vignesh Raghavendra, dpervushin,
	Artem Bityutskiy, linux-mtd, linux-kernel, yi zhang, yangerkun


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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-23  7:09       ` Zhihao Cheng
@ 2023-10-23  7:15         ` Richard Weinberger
  2023-10-23  7:36           ` Zhihao Cheng
  0 siblings, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2023-10-23  7:15 UTC (permalink / raw)
  To: chengzhihao1
  Cc: ZhaoLong Wang, Miquel Raynal, Vignesh Raghavendra, dpervushin,
	Artem Bityutskiy, linux-mtd, linux-kernel, yi zhang, yangerkun

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-23  7:12           ` ZhaoLong Wang
@ 2023-10-23  7:16             ` Richard Weinberger
  0 siblings, 0 replies; 41+ messages in thread
From: Richard Weinberger @ 2023-10-23  7:16 UTC (permalink / raw)
  To: ZhaoLong Wang
  Cc: chengzhihao1, Miquel Raynal, Vignesh Raghavendra, dpervushin,
	Artem Bityutskiy, linux-mtd, linux-kernel, yi zhang, yangerkun

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2023-10-23  7:15         ` Richard Weinberger
@ 2023-10-23  7:36           ` Zhihao Cheng
  0 siblings, 0 replies; 41+ messages in thread
From: Zhihao Cheng @ 2023-10-23  7:36 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: ZhaoLong Wang, Miquel Raynal, Vignesh Raghavendra, dpervushin,
	Artem Bityutskiy, linux-mtd, linux-kernel, yi zhang, yangerkun

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



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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
@ 2024-06-17 14:21 Gagan Sidhu
  0 siblings, 0 replies; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 14:21 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-mtd

hi,

this patch isn’t a good one.

it must be reverted.

the problem with mr wang's changes is that it breaks expected mounting behaviour of a filesystem within an UBI, as is the case for openwrt.

i am surprised no one has raised the issue about this. i see mr raynal did raise an issue with one of mr zhihao’s fundamental errors:

https://lore.kernel.org/lkml/20231027194026.1bc32dfe@xps-13/

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

this is absolutely wrong.

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.

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.

```


3: System Boot system code via Flash.
## Booting image at bc180000 ...
   Image Name:   DD-WRT v24 Linux Kernel Imag
   Image Type:   MIPS Linux Kernel Image (lzma compressed)
   Data Size:    3875031 Bytes =  3.7 MB
   Load Address: 80001000
   Entry Point:  807d9e20
............................................................   Verifying Checksum ... OK
   Uncompressing Kernel Image ... OK
No initrd
## Transferring control to Linux (at address 807d9e20) ...
## Giving linux memsize in MB, 256

Starting kernel ...

[    0.000000] Linux version 4.14.348-openela-rt159 (
Gagan@GagansMacPro.local
) (gcc version 14.1.0 (GCC)) #5426 SMP Sat Jun 15 07:23:17 MDT 2024
[    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
[    0.000000] bootconsole [early0] enabled
[    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
[    0.000000] MIPS: machine is D-Link DIR-2640 rev. A1
[    0.000000] Determined physical RAM map:
[    0.000000]  memory: 10000000 @ 00000000 (usable)
[    0.000000] VPE topology {2,2} total 4
[    0.000000] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.000000] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.000000] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.000000] Zone ranges:
[    0.000000]   DMA      [mem 0x0000000000000000-0x0000000000ffffff]
[    0.000000]   Normal   [mem 0x0000000001000000-0x000000000fffffff]
[    0.000000]   HighMem  empty
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x000000000fffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000000fffffff]
[    0.000000] percpu: Embedded 15 pages/cpu s30672 r8192 d22576 u61440
[    0.000000] Built 1 zonelists, mobility grouping on.  Total pages: 65024
[    0.000000] Kernel command line: console=ttyS0,57600n8
[    0.000000] log_buf_len individual max cpu contribution: 4096 bytes
[    0.000000] log_buf_len total cpu_extra contributions: 12288 bytes
[    0.000000] log_buf_len min size: 16384 bytes
[    0.000000] log_buf_len: 32768 bytes
[    0.000000] early log buf free: 14216(86%)
[    0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
[    0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[    0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] Writing ErrCtl register=000412fa
[    0.000000] Readback ErrCtl register=000412fa
[    0.000000] Memory: 247980K/262144K available (8061K kernel code, 892K rwdata, 1568K rodata, 280K init, 733K bss, 14164K reserved, 0K cma-reserved, 0K highmem)
[    0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[    0.000000] Hierarchical RCU implementation.
[    0.000000] NR_IRQS: 256
[    0.000000] CPU Clock: 880MHz
[    0.000000] clocksource: GIC: mask: 0xffffffffffffffff max_cycles: 0xcaf478abb4, max_idle_ns: 440795247997 ns
[    0.000000] clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 4343773742 ns
[    0.000009] sched_clock: 32 bits at 440MHz, resolution 2ns, wraps every 4880645118ns
[    0.015565] Calibrating delay loop... 583.68 BogoMIPS (lpj=1167360)
[    0.055919] pid_max: default: 4096 minimum: 301
[    0.065064] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.078089] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.093805] Hierarchical SRCU implementation.
[    0.103082] smp: Bringing up secondary CPUs ...
[    0.113986] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.113994] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.114004] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.114122] CPU1 revision is: 0001992f (MIPS 1004Kc)
[    0.140246] Synchronize counters for CPU 1: done.
[    0.205804] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.205812] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.205818] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.205878] CPU2 revision is: 0001992f (MIPS 1004Kc)
[    0.239402] Synchronize counters for CPU 2: done.
[    0.300999] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.301006] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.301013] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.301082] CPU3 revision is: 0001992f (MIPS 1004Kc)
[    0.327024] Synchronize counters for CPU 3: done.
[    0.386624] smp: Brought up 1 node, 4 CPUs
[    0.395319] devtmpfs: initialized
[    0.405072] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
[    0.424361] futex hash table entries: 16 (order: -3, 512 bytes)
[    0.436154] pinctrl core: initialized pinctrl subsystem
[    0.447323] NET: Registered protocol family 16
[    0.456700] cpuidle: using governor menu
[    0.484220] pull PCIe RST: RALINK_RSTCTRL = 4000000
[    0.794105] release PCIe RST: RALINK_RSTCTRL = 7000000
[    0.804176] ***** Xtal 40MHz *****
[    0.810918] release PCIe RST: RALINK_RSTCTRL = 7000000
[    0.821113] Port 0 N_FTS = 1b105000
[    0.828025] Port 1 N_FTS = 1b105000
[    0.834937] Port 2 N_FTS = 1b102800
[    1.992951] PCIE2 no card, disable it(RST&CLK)
[    2.001641]  -> 21007f2
[    2.006479] PCIE0 enabled
[    2.011663] PCIE1 enabled
[    2.016858] PCI host bridge /pcie@1e140000 ranges:
[    2.026367]  MEM 0x0000000060000000..0x000000006fffffff
[    2.036726]   IO 0x000000001e160000..0x000000001e16ffff
[    2.047093] PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
[    2.066762] mt7621_gpio 1e000600.gpio: registering 32 gpios
[    2.078058] mt7621_gpio 1e000600.gpio: registering 32 gpios
[    2.089214] mt7621_gpio 1e000600.gpio: registering 32 gpios
[    2.100683] vgaarb: loaded
[    2.106249] SCSI subsystem initialized
[    2.113766] usbcore: registered new interface driver usbfs
[    2.124606] usbcore: registered new interface driver hub
[    2.135129] usbcore: registered new device driver usb
[    2.145477] i2c-mt7621 1e000900.i2c: clock 100KHz, re-start not support
[    2.158813] PCI host bridge to bus 0000:00
[    2.166839] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[    2.180477] pci_bus 0000:00: root bus resource [io  0x1e160000-0x1e16ffff]
[    2.194129] pci_bus 0000:00: root bus resource [??? 0x00000000 flags 0x0]
[    2.207606] pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
[    2.224155] pci 0000:01:00.0: 2.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s x1 link at 0000:00:00.0 (capable of 4.000 Gb/s with 5 GT/s x1 link)
[    2.251939] pci 0000:02:00.0: 2.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s x1 link at 0000:00:01.0 (capable of 4.000 Gb/s with 5 GT/s x1 link)
[    2.279470] pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
[    2.292494] pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
[    2.306313] pci 0000:00:01.0: BAR 0: no space for [mem size 0x80000000]
[    2.319449] pci 0000:00:01.0: BAR 0: failed to assign [mem size 0x80000000]
[    2.333277] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    2.346755] pci 0000:00:01.0: BAR 8: assigned [mem 0x60100000-0x601fffff]
[    2.360233] pci 0000:00:00.0: BAR 1: assigned [mem 0x60200000-0x6020ffff]
[    2.373709] pci 0000:00:01.0: BAR 1: assigned [mem 0x60210000-0x6021ffff]
[    2.387208] pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x600fffff 64bit]
[    2.401712] pci 0000:00:00.0: PCI bridge to [bus 01]
[    2.411548] pci 0000:00:00.0:   bridge window [mem 0x60000000-0x600fffff]
[    2.425039] pci 0000:02:00.0: BAR 0: assigned [mem 0x60100000-0x601fffff 64bit]
[    2.439555] pci 0000:00:01.0: PCI bridge to [bus 02]
[    2.449396] pci 0000:00:01.0:   bridge window [mem 0x60100000-0x601fffff]
[    2.463554] clocksource: Switched to clocksource GIC
[    2.474753] NET: Registered protocol family 2
[    2.483561] IP idents hash table entries: 4096 (order: 3, 32768 bytes)
[    2.497245] TCP established hash table entries: 2048 (order: 1, 8192 bytes)
[    2.511002] TCP bind hash table entries: 2048 (order: 2, 16384 bytes)
[    2.523798] TCP: Hash tables configured (established 2048 bind 2048)
[    2.536493] UDP hash table entries: 128 (order: 0, 4096 bytes)
[    2.547983] UDP-Lite hash table entries: 128 (order: 0, 4096 bytes)
[    2.560657] NET: Registered protocol family 1
[    2.659518] 4 CPUs re-calibrate udelay(lpj = 1163264)
[    2.670337] workingset: timestamp_bits=30 max_order=16 bucket_order=0
[    2.689246] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[    2.709543] io scheduler noop registered
[    2.717554] io scheduler cfq registered (default)
[    2.726769] io scheduler mq-deadline registered
[    2.735774] io scheduler kyber registered
[    2.744477] mtk_hsdma 1e007000.hsdma: Using 3 as missing dma-requests property
[    2.758985] mtk_hsdma 1e007000.hsdma: MediaTek HSDMA driver registered
[    2.823896] serial8250_init
[    2.829315] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
[    2.843297] console [ttyS0] disabled
[    2.850333] 1e000c00.uartlite: ttyS0 at MMIO 0x1e000c00 (irq = 19, base_baud = 3125000) is a 16550A
[    2.868297] console [ttyS0] enabled
[    2.868297] console [ttyS0] enabled
[    2.882070] bootconsole [early0] disabled
[    2.882070] bootconsole [early0] disabled
[    2.898446] Ralink gpio driver initialized:power_gpio[8]
[    2.910106] MediaTek Nand driver init, version v2.1 Fix AHB virt2phys error
[    2.924094] Enable NFI Clock
[    2.929829] # MTK NAND # : Use HW ECC
[    2.937139] Device not found, ID: c8d1
[    2.944610] Not Support this Device!
[    2.952075] chip_mode=00000001
[    2.958157] Support this Device in MTK table! c8d1
[    2.968055] [NAND]select ecc bit:4, sparesize :64 spare_per_sector=16
[    2.980930] nand: device found, Manufacturer ID: 0xc8, Chip ID: 0xd1
[    2.993590] nand: ESMT NAND 128MiB 3,3V 8-bit
[    3.002281] nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
[    3.017376] Scanning device for bad blocks
[    3.169277] MT7621-NAND: parsing partitions cmdlinepart
[    3.180264] MT7621-NAND: got parser (null)
[    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
[    4.031931] tun: Universal TUN/TAP device driver, 1.6
[    4.042377] CHIP_ID = MT7621
[    4.048133] WAN at P4
[    4.052659] GMAC1 support rgmii
[    4.058911] GE1_RGMII_FORCE_1000
[    4.065348] GMAC2 support rgmii
[    4.071606] RGMII_AN (Internal GigaPhy)
[    4.079973] STD_v0.1  1024 rx/2048 tx descriptors allocated, mtu = 1500!
[    4.094358] set CLK_CFG_0 = 0x40a00020!!!!!!!!!!!!!!!!!!1
[    4.105108] trgmii_set_7621 Completed!!
[    4.324297] MT7530 Reset Completed!!
[    4.340754] trgmii_set_7530 Completed!!
[    4.348739] change HW-TRAP to 0x17c8f
[    4.360357] set LAN/WAN LLLLW
[    4.374100] eth3: ===> virtualif_open
[    4.381646] == MT7530 MCM ==
[    4.387482] PPP generic driver version 2.4.2
[    4.396233] PPP BSD Compression module registered
[    4.405603] PPP Deflate Compression module registered
[    4.415687] PPP MPPE Compression module registered
[    4.425229] NET: Registered protocol family 24
[    4.434106] register mt_drv
[    4.439738] bus=0x1, slot = 0x0, irq=0x0
[    4.472239]
[    4.472239] == pAd = c0181000, size = 6632704, Status=0 ==
[    4.486132] pAd->PciHif.CSRBaseAddress =0xc0080000, csr_addr=0xc0080000!
[    4.499531] RTMPInitPCIeDevice():device_id=0x7615
[    4.508911] mt_pci_chip_cfg(): HWVer=0x8a10, FWVer=0x8a10, pAd->ChipID=0x7615
[    4.523136] mt_pci_chip_cfg(): HIF_SYS_REV=0x76150001
[    4.533196] AP Driver version-5.1.0.0
[    4.540493] RtmpChipOpsHook(223): Not support for HIF_MT yet! MACVersion=0x0
[    4.554527] mt7615_init()-->
[    4.560266] Use 1st ePAeLNA default bin.
[    4.568078] Use 0st /etc/wlan/mt7615e.eeprom.bin default bin.
[    4.579550] <--mt7615_init()
[    4.589234] <-- RTMPAllocTxRxRingMemory, Status=0
[    4.599438] bus=0x2, slot = 0x1, irq=0x0
[    4.631995]
[    4.631995] == pAd = c0901000, size = 6632704, Status=0 ==
[    4.645884] pAd->PciHif.CSRBaseAddress =0xc0800000, csr_addr=0xc0800000!
[    4.659234] RTMPInitPCIeDevice():device_id=0x7615
[    4.668612] mt_pci_chip_cfg(): HWVer=0x8a10, FWVer=0x8a10, pAd->ChipID=0x7615
[    4.682820] mt_pci_chip_cfg(): HIF_SYS_REV=0x76150001
[    4.692879] AP Driver version-5.1.0.0
[    4.700175] RtmpChipOpsHook(223): Not support for HIF_MT yet! MACVersion=0x0
[    4.714208] mt7615_init()-->
[    4.719946] Use 2nd ePAeLNA default bin.
[    4.727767] Use 1st /etc/wlan/mt7615e.eeprom.bin default bin.
[    4.739224] <--mt7615_init()
[    4.748896] <-- RTMPAllocTxRxRingMemory, Status=0
[    4.759057] rdm_major = 255
[    4.765124] xhci-mtk 1e1c0000.xhci: xHCI Host Controller
[    4.775743] xhci-mtk 1e1c0000.xhci: new USB bus registered, assigned bus number 1
[    4.799681] xhci-mtk 1e1c0000.xhci: hcc params 0x01401198 hci version 0x96 quirks 0x0000000000290010
[    4.817950] xhci-mtk 1e1c0000.xhci: irq 22, io mem 0x1e1c0000
[    4.830260] hub 1-0:1.0: USB hub found
[    4.837822] hub 1-0:1.0: 2 ports detected
[    4.846283] xhci-mtk 1e1c0000.xhci: xHCI Host Controller
[    4.856910] xhci-mtk 1e1c0000.xhci: new USB bus registered, assigned bus number 2
[    4.871846] xhci-mtk 1e1c0000.xhci: Host supports USB 3.0  SuperSpeed
[    4.884885] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
[    4.901675] hub 2-0:1.0: USB hub found
[    4.909226] hub 2-0:1.0: 1 port detected
[    4.917608] usbcore: registered new interface driver usblp
[    4.928659] usbcore: registered new interface driver usb-storage
[    4.940727] usbcore: registered new interface driver usbserial
[    5.007737] rtc-pcf8563 0-0051: registered as rtc0
[    5.031606] i2c /dev entries driver
[    5.038842] Ralink APSoC Hardware Watchdog Timer
[    5.049311] usbcore: registered new interface driver usbhid
[    5.060419] usbhid: USB HID core driver
[    5.068615] u32 classifier
[    5.074005]     Performance counters on
[    5.081638]     Actions configured
[    5.088434] Netfilter messages via NETLINK v0.30.
[    5.098071] nf_conntrack version 0.5.0 (4096 buckets, 16384 max)
[    5.110321] ctnetlink v0.93: registering with nfnetlink.
[    5.121402] ipip: IPv4 and MPLS over IPv4 tunneling driver
[    5.132985] ip_tables: (C) 2000-2006 Netfilter Core Team
[    5.144546] NET: Registered protocol family 10
[    5.155889] Segment Routing with IPv6
[    5.163303] NET: Registered protocol family 17
[    5.172292] Bridge firewalling registered
[    5.180281] 8021q: 802.1Q VLAN Support v1.8
[    5.189452] registered taskstats version 1
[    5.198914] searching for nvram
[    5.279016] found nvram at 0, name:Config, contributed bytes:262144
[    5.328401] nvram empty
[    5.407013] found nvram at 1, name:Config2, contributed bytes:262144
[    5.456567] nvram empty
[    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..

```

bless,
g



Thanks,
Gagan


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
@ 2024-06-17 14:25 Gagan Sidhu
  2024-06-17 14:31 ` Richard Weinberger
  2024-06-17 15:42 ` Richard Weinberger
  0 siblings, 2 replies; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 14:25 UTC (permalink / raw)
  To: wangzhaolong1
  Cc: Artem.Bityutskiy, chengzhihao1, dpervushin, linux-kernel,
	linux-mtd, miquel.raynal, richard, vigneshr, yangerkun, yi.zhang

hi,

this patch isn’t a good one.

it must be reverted.

the problem with mr wang's changes is that it breaks expected mounting behaviour of a filesystem within an UBI, as is the case for openwrt.

i am surprised no one has raised the issue about this. i see mr raynal did raise an issue with one of mr zhihao’s fundamental errors:

https://lore.kernel.org/lkml/20231027194026.1bc32dfe@xps-13/

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

this is absolutely wrong.

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.

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.

```


3: System Boot system code via Flash.
## Booting image at bc180000 ...
  Image Name:   DD-WRT v24 Linux Kernel Imag
  Image Type:   MIPS Linux Kernel Image (lzma compressed)
  Data Size:    3875031 Bytes =  3.7 MB
  Load Address: 80001000
  Entry Point:  807d9e20
............................................................   Verifying Checksum ... OK
  Uncompressing Kernel Image ... OK
No initrd
## Transferring control to Linux (at address 807d9e20) ...
## Giving linux memsize in MB, 256

Starting kernel ...

[    0.000000] Linux version 4.14.348-openela-rt159 (
Gagan@GagansMacPro.local
) (gcc version 14.1.0 (GCC)) #5426 SMP Sat Jun 15 07:23:17 MDT 2024
[    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
[    0.000000] bootconsole [early0] enabled
[    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
[    0.000000] MIPS: machine is D-Link DIR-2640 rev. A1
[    0.000000] Determined physical RAM map:
[    0.000000]  memory: 10000000 @ 00000000 (usable)
[    0.000000] VPE topology {2,2} total 4
[    0.000000] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.000000] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.000000] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.000000] Zone ranges:
[    0.000000]   DMA      [mem 0x0000000000000000-0x0000000000ffffff]
[    0.000000]   Normal   [mem 0x0000000001000000-0x000000000fffffff]
[    0.000000]   HighMem  empty
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x0000000000000000-0x000000000fffffff]
[    0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x000000000fffffff]
[    0.000000] percpu: Embedded 15 pages/cpu s30672 r8192 d22576 u61440
[    0.000000] Built 1 zonelists, mobility grouping on.  Total pages: 65024
[    0.000000] Kernel command line: console=ttyS0,57600n8
[    0.000000] log_buf_len individual max cpu contribution: 4096 bytes
[    0.000000] log_buf_len total cpu_extra contributions: 12288 bytes
[    0.000000] log_buf_len min size: 16384 bytes
[    0.000000] log_buf_len: 32768 bytes
[    0.000000] early log buf free: 14216(86%)
[    0.000000] PID hash table entries: 1024 (order: 0, 4096 bytes)
[    0.000000] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[    0.000000] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[    0.000000] Writing ErrCtl register=000412fa
[    0.000000] Readback ErrCtl register=000412fa
[    0.000000] Memory: 247980K/262144K available (8061K kernel code, 892K rwdata, 1568K rodata, 280K init, 733K bss, 14164K reserved, 0K cma-reserved, 0K highmem)
[    0.000000] SLUB: HWalign=32, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[    0.000000] Hierarchical RCU implementation.
[    0.000000] NR_IRQS: 256
[    0.000000] CPU Clock: 880MHz
[    0.000000] clocksource: GIC: mask: 0xffffffffffffffff max_cycles: 0xcaf478abb4, max_idle_ns: 440795247997 ns
[    0.000000] clocksource: MIPS: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 4343773742 ns
[    0.000009] sched_clock: 32 bits at 440MHz, resolution 2ns, wraps every 4880645118ns
[    0.015565] Calibrating delay loop... 583.68 BogoMIPS (lpj=1167360)
[    0.055919] pid_max: default: 4096 minimum: 301
[    0.065064] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.078089] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[    0.093805] Hierarchical SRCU implementation.
[    0.103082] smp: Bringing up secondary CPUs ...
[    0.113986] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.113994] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.114004] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.114122] CPU1 revision is: 0001992f (MIPS 1004Kc)
[    0.140246] Synchronize counters for CPU 1: done.
[    0.205804] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.205812] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.205818] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.205878] CPU2 revision is: 0001992f (MIPS 1004Kc)
[    0.239402] Synchronize counters for CPU 2: done.
[    0.300999] Primary instruction cache 32kB, VIPT, 4-way, linesize 32 bytes.
[    0.301006] Primary data cache 32kB, 4-way, PIPT, no aliases, linesize 32 bytes
[    0.301013] MIPS secondary cache 256kB, 8-way, linesize 32 bytes.
[    0.301082] CPU3 revision is: 0001992f (MIPS 1004Kc)
[    0.327024] Synchronize counters for CPU 3: done.
[    0.386624] smp: Brought up 1 node, 4 CPUs
[    0.395319] devtmpfs: initialized
[    0.405072] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645041785100000 ns
[    0.424361] futex hash table entries: 16 (order: -3, 512 bytes)
[    0.436154] pinctrl core: initialized pinctrl subsystem
[    0.447323] NET: Registered protocol family 16
[    0.456700] cpuidle: using governor menu
[    0.484220] pull PCIe RST: RALINK_RSTCTRL = 4000000
[    0.794105] release PCIe RST: RALINK_RSTCTRL = 7000000
[    0.804176] ***** Xtal 40MHz *****
[    0.810918] release PCIe RST: RALINK_RSTCTRL = 7000000
[    0.821113] Port 0 N_FTS = 1b105000
[    0.828025] Port 1 N_FTS = 1b105000
[    0.834937] Port 2 N_FTS = 1b102800
[    1.992951] PCIE2 no card, disable it(RST&CLK)
[    2.001641]  -> 21007f2
[    2.006479] PCIE0 enabled
[    2.011663] PCIE1 enabled
[    2.016858] PCI host bridge /pcie@1e140000 ranges:
[    2.026367]  MEM 0x0000000060000000..0x000000006fffffff
[    2.036726]   IO 0x000000001e160000..0x000000001e16ffff
[    2.047093] PCI coherence region base: 0x60000000, mask/settings: 0xf0000002
[    2.066762] mt7621_gpio 1e000600.gpio: registering 32 gpios
[    2.078058] mt7621_gpio 1e000600.gpio: registering 32 gpios
[    2.089214] mt7621_gpio 1e000600.gpio: registering 32 gpios
[    2.100683] vgaarb: loaded
[    2.106249] SCSI subsystem initialized
[    2.113766] usbcore: registered new interface driver usbfs
[    2.124606] usbcore: registered new interface driver hub
[    2.135129] usbcore: registered new device driver usb
[    2.145477] i2c-mt7621 1e000900.i2c: clock 100KHz, re-start not support
[    2.158813] PCI host bridge to bus 0000:00
[    2.166839] pci_bus 0000:00: root bus resource [mem 0x60000000-0x6fffffff]
[    2.180477] pci_bus 0000:00: root bus resource [io  0x1e160000-0x1e16ffff]
[    2.194129] pci_bus 0000:00: root bus resource [??? 0x00000000 flags 0x0]
[    2.207606] pci_bus 0000:00: No busn resource found for root bus, will use [bus 00-ff]
[    2.224155] pci 0000:01:00.0: 2.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s x1 link at 0000:00:00.0 (capable of 4.000 Gb/s with 5 GT/s x1 link)
[    2.251939] pci 0000:02:00.0: 2.000 Gb/s available PCIe bandwidth, limited by 2.5 GT/s x1 link at 0000:00:01.0 (capable of 4.000 Gb/s with 5 GT/s x1 link)
[    2.279470] pci 0000:00:00.0: BAR 0: no space for [mem size 0x80000000]
[    2.292494] pci 0000:00:00.0: BAR 0: failed to assign [mem size 0x80000000]
[    2.306313] pci 0000:00:01.0: BAR 0: no space for [mem size 0x80000000]
[    2.319449] pci 0000:00:01.0: BAR 0: failed to assign [mem size 0x80000000]
[    2.333277] pci 0000:00:00.0: BAR 8: assigned [mem 0x60000000-0x600fffff]
[    2.346755] pci 0000:00:01.0: BAR 8: assigned [mem 0x60100000-0x601fffff]
[    2.360233] pci 0000:00:00.0: BAR 1: assigned [mem 0x60200000-0x6020ffff]
[    2.373709] pci 0000:00:01.0: BAR 1: assigned [mem 0x60210000-0x6021ffff]
[    2.387208] pci 0000:01:00.0: BAR 0: assigned [mem 0x60000000-0x600fffff 64bit]
[    2.401712] pci 0000:00:00.0: PCI bridge to [bus 01]
[    2.411548] pci 0000:00:00.0:   bridge window [mem 0x60000000-0x600fffff]
[    2.425039] pci 0000:02:00.0: BAR 0: assigned [mem 0x60100000-0x601fffff 64bit]
[    2.439555] pci 0000:00:01.0: PCI bridge to [bus 02]
[    2.449396] pci 0000:00:01.0:   bridge window [mem 0x60100000-0x601fffff]
[    2.463554] clocksource: Switched to clocksource GIC
[    2.474753] NET: Registered protocol family 2
[    2.483561] IP idents hash table entries: 4096 (order: 3, 32768 bytes)
[    2.497245] TCP established hash table entries: 2048 (order: 1, 8192 bytes)
[    2.511002] TCP bind hash table entries: 2048 (order: 2, 16384 bytes)
[    2.523798] TCP: Hash tables configured (established 2048 bind 2048)
[    2.536493] UDP hash table entries: 128 (order: 0, 4096 bytes)
[    2.547983] UDP-Lite hash table entries: 128 (order: 0, 4096 bytes)
[    2.560657] NET: Registered protocol family 1
[    2.659518] 4 CPUs re-calibrate udelay(lpj = 1163264)
[    2.670337] workingset: timestamp_bits=30 max_order=16 bucket_order=0
[    2.689246] squashfs: version 4.0 (2009/01/31) Phillip Lougher
[    2.709543] io scheduler noop registered
[    2.717554] io scheduler cfq registered (default)
[    2.726769] io scheduler mq-deadline registered
[    2.735774] io scheduler kyber registered
[    2.744477] mtk_hsdma 1e007000.hsdma: Using 3 as missing dma-requests property
[    2.758985] mtk_hsdma 1e007000.hsdma: MediaTek HSDMA driver registered
[    2.823896] serial8250_init
[    2.829315] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
[    2.843297] console [ttyS0] disabled
[    2.850333] 1e000c00.uartlite: ttyS0 at MMIO 0x1e000c00 (irq = 19, base_baud = 3125000) is a 16550A
[    2.868297] console [ttyS0] enabled
[    2.868297] console [ttyS0] enabled
[    2.882070] bootconsole [early0] disabled
[    2.882070] bootconsole [early0] disabled
[    2.898446] Ralink gpio driver initialized:power_gpio[8]
[    2.910106] MediaTek Nand driver init, version v2.1 Fix AHB virt2phys error
[    2.924094] Enable NFI Clock
[    2.929829] # MTK NAND # : Use HW ECC
[    2.937139] Device not found, ID: c8d1
[    2.944610] Not Support this Device!
[    2.952075] chip_mode=00000001
[    2.958157] Support this Device in MTK table! c8d1
[    2.968055] [NAND]select ecc bit:4, sparesize :64 spare_per_sector=16
[    2.980930] nand: device found, Manufacturer ID: 0xc8, Chip ID: 0xd1
[    2.993590] nand: ESMT NAND 128MiB 3,3V 8-bit
[    3.002281] nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
[    3.017376] Scanning device for bad blocks
[    3.169277] MT7621-NAND: parsing partitions cmdlinepart
[    3.180264] MT7621-NAND: got parser (null)
[    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
[    4.031931] tun: Universal TUN/TAP device driver, 1.6
[    4.042377] CHIP_ID = MT7621
[    4.048133] WAN at P4
[    4.052659] GMAC1 support rgmii
[    4.058911] GE1_RGMII_FORCE_1000
[    4.065348] GMAC2 support rgmii
[    4.071606] RGMII_AN (Internal GigaPhy)
[    4.079973] STD_v0.1  1024 rx/2048 tx descriptors allocated, mtu = 1500!
[    4.094358] set CLK_CFG_0 = 0x40a00020!!!!!!!!!!!!!!!!!!1
[    4.105108] trgmii_set_7621 Completed!!
[    4.324297] MT7530 Reset Completed!!
[    4.340754] trgmii_set_7530 Completed!!
[    4.348739] change HW-TRAP to 0x17c8f
[    4.360357] set LAN/WAN LLLLW
[    4.374100] eth3: ===> virtualif_open
[    4.381646] == MT7530 MCM ==
[    4.387482] PPP generic driver version 2.4.2
[    4.396233] PPP BSD Compression module registered
[    4.405603] PPP Deflate Compression module registered
[    4.415687] PPP MPPE Compression module registered
[    4.425229] NET: Registered protocol family 24
[    4.434106] register mt_drv
[    4.439738] bus=0x1, slot = 0x0, irq=0x0
[    4.472239]
[    4.472239] == pAd = c0181000, size = 6632704, Status=0 ==
[    4.486132] pAd->PciHif.CSRBaseAddress =0xc0080000, csr_addr=0xc0080000!
[    4.499531] RTMPInitPCIeDevice():device_id=0x7615
[    4.508911] mt_pci_chip_cfg(): HWVer=0x8a10, FWVer=0x8a10, pAd->ChipID=0x7615
[    4.523136] mt_pci_chip_cfg(): HIF_SYS_REV=0x76150001
[    4.533196] AP Driver version-5.1.0.0
[    4.540493] RtmpChipOpsHook(223): Not support for HIF_MT yet! MACVersion=0x0
[    4.554527] mt7615_init()-->
[    4.560266] Use 1st ePAeLNA default bin.
[    4.568078] Use 0st /etc/wlan/mt7615e.eeprom.bin default bin.
[    4.579550] <--mt7615_init()
[    4.589234] <-- RTMPAllocTxRxRingMemory, Status=0
[    4.599438] bus=0x2, slot = 0x1, irq=0x0
[    4.631995]
[    4.631995] == pAd = c0901000, size = 6632704, Status=0 ==
[    4.645884] pAd->PciHif.CSRBaseAddress =0xc0800000, csr_addr=0xc0800000!
[    4.659234] RTMPInitPCIeDevice():device_id=0x7615
[    4.668612] mt_pci_chip_cfg(): HWVer=0x8a10, FWVer=0x8a10, pAd->ChipID=0x7615
[    4.682820] mt_pci_chip_cfg(): HIF_SYS_REV=0x76150001
[    4.692879] AP Driver version-5.1.0.0
[    4.700175] RtmpChipOpsHook(223): Not support for HIF_MT yet! MACVersion=0x0
[    4.714208] mt7615_init()-->
[    4.719946] Use 2nd ePAeLNA default bin.
[    4.727767] Use 1st /etc/wlan/mt7615e.eeprom.bin default bin.
[    4.739224] <--mt7615_init()
[    4.748896] <-- RTMPAllocTxRxRingMemory, Status=0
[    4.759057] rdm_major = 255
[    4.765124] xhci-mtk 1e1c0000.xhci: xHCI Host Controller
[    4.775743] xhci-mtk 1e1c0000.xhci: new USB bus registered, assigned bus number 1
[    4.799681] xhci-mtk 1e1c0000.xhci: hcc params 0x01401198 hci version 0x96 quirks 0x0000000000290010
[    4.817950] xhci-mtk 1e1c0000.xhci: irq 22, io mem 0x1e1c0000
[    4.830260] hub 1-0:1.0: USB hub found
[    4.837822] hub 1-0:1.0: 2 ports detected
[    4.846283] xhci-mtk 1e1c0000.xhci: xHCI Host Controller
[    4.856910] xhci-mtk 1e1c0000.xhci: new USB bus registered, assigned bus number 2
[    4.871846] xhci-mtk 1e1c0000.xhci: Host supports USB 3.0  SuperSpeed
[    4.884885] usb usb2: We don't know the algorithms for LPM for this host, disabling LPM.
[    4.901675] hub 2-0:1.0: USB hub found
[    4.909226] hub 2-0:1.0: 1 port detected
[    4.917608] usbcore: registered new interface driver usblp
[    4.928659] usbcore: registered new interface driver usb-storage
[    4.940727] usbcore: registered new interface driver usbserial
[    5.007737] rtc-pcf8563 0-0051: registered as rtc0
[    5.031606] i2c /dev entries driver
[    5.038842] Ralink APSoC Hardware Watchdog Timer
[    5.049311] usbcore: registered new interface driver usbhid
[    5.060419] usbhid: USB HID core driver
[    5.068615] u32 classifier
[    5.074005]     Performance counters on
[    5.081638]     Actions configured
[    5.088434] Netfilter messages via NETLINK v0.30.
[    5.098071] nf_conntrack version 0.5.0 (4096 buckets, 16384 max)
[    5.110321] ctnetlink v0.93: registering with nfnetlink.
[    5.121402] ipip: IPv4 and MPLS over IPv4 tunneling driver
[    5.132985] ip_tables: (C) 2000-2006 Netfilter Core Team
[    5.144546] NET: Registered protocol family 10
[    5.155889] Segment Routing with IPv6
[    5.163303] NET: Registered protocol family 17
[    5.172292] Bridge firewalling registered
[    5.180281] 8021q: 802.1Q VLAN Support v1.8
[    5.189452] registered taskstats version 1
[    5.198914] searching for nvram
[    5.279016] found nvram at 0, name:Config, contributed bytes:262144
[    5.328401] nvram empty
[    5.407013] found nvram at 1, name:Config2, contributed bytes:262144
[    5.456567] nvram empty
[    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..

```

bless,
g



Thanks,
Gagan


Thanks,
Gagan


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 14:25 [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier Gagan Sidhu
@ 2024-06-17 14:31 ` Richard Weinberger
  2024-06-17 15:42 ` Richard Weinberger
  1 sibling, 0 replies; 41+ messages in thread
From: Richard Weinberger @ 2024-06-17 14:31 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: ZhaoLong Wang, Artem Bityutskiy, chengzhihao1, dpervushin,
	linux-kernel, linux-mtd, Miquel Raynal, Vignesh Raghavendra,
	yangerkun, yi zhang

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 14:25 [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier Gagan Sidhu
  2024-06-17 14:31 ` Richard Weinberger
@ 2024-06-17 15:42 ` Richard Weinberger
       [not found]   ` <14779870-BA54-4ABF-8ABF-FF1D23D172A7@mac.com>
  1 sibling, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2024-06-17 15:42 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: ZhaoLong Wang, Artem Bityutskiy, chengzhihao1, dpervushin,
	linux-kernel, linux-mtd, Miquel Raynal, Vignesh Raghavendra,
	yangerkun, yi zhang

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
       [not found]   ` <14779870-BA54-4ABF-8ABF-FF1D23D172A7@mac.com>
@ 2024-06-17 16:00     ` Richard Weinberger
  2024-06-17 16:05       ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2024-06-17 16:00 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: ZhaoLong Wang, Artem Bityutskiy, chengzhihao1, dpervushin,
	linux-kernel, linux-mtd, Miquel Raynal, Vignesh Raghavendra,
	yangerkun, yi zhang

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 16:00     ` Richard Weinberger
@ 2024-06-17 16:05       ` Gagan Sidhu
  2024-06-17 16:52         ` Richard Weinberger
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 16:05 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 16:05       ` Gagan Sidhu
@ 2024-06-17 16:52         ` Richard Weinberger
       [not found]           ` <E3E2C13C-1E52-46F2-BE2D-D2592C3369DB@mac.com>
  0 siblings, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2024-06-17 16:52 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
       [not found]           ` <E3E2C13C-1E52-46F2-BE2D-D2592C3369DB@mac.com>
@ 2024-06-17 17:33             ` Gagan Sidhu
  2024-06-17 17:48               ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 17:33 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang

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
> 


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 17:33             ` Gagan Sidhu
@ 2024-06-17 17:48               ` Gagan Sidhu
  2024-06-17 18:09                 ` Richard Weinberger
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 17:48 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 17:48               ` Gagan Sidhu
@ 2024-06-17 18:09                 ` Richard Weinberger
  2024-06-17 18:18                   ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2024-06-17 18:09 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 18:09                 ` Richard Weinberger
@ 2024-06-17 18:18                   ` Gagan Sidhu
  2024-06-17 18:32                     ` Richard Weinberger
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 18:18 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang



> 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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 18:18                   ` Gagan Sidhu
@ 2024-06-17 18:32                     ` Richard Weinberger
  2024-06-17 18:46                       ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2024-06-17 18:32 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang

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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 18:32                     ` Richard Weinberger
@ 2024-06-17 18:46                       ` Gagan Sidhu
  2024-06-17 18:52                         ` Richard Weinberger
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 18:46 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang



> 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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 18:46                       ` Gagan Sidhu
@ 2024-06-17 18:52                         ` Richard Weinberger
  2024-06-17 20:29                           ` Daniel Golle
  0 siblings, 1 reply; 41+ messages in thread
From: Richard Weinberger @ 2024-06-17 18:52 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: ZhaoLong Wang, chengzhihao1, dpervushin, linux-kernel, linux-mtd,
	Miquel Raynal, Vignesh Raghavendra, yangerkun, yi zhang, daniel

[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

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 18:52                         ` Richard Weinberger
@ 2024-06-17 20:29                           ` Daniel Golle
  2024-06-17 21:22                             ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Daniel Golle @ 2024-06-17 20:29 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Gagan Sidhu, ZhaoLong Wang, chengzhihao1, dpervushin,
	linux-kernel, linux-mtd, Miquel Raynal, Vignesh Raghavendra,
	yangerkun, yi zhang

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/

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 20:29                           ` Daniel Golle
@ 2024-06-17 21:22                             ` Gagan Sidhu
  2024-06-17 22:13                               ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 21:22 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Richard Weinberger, ZhaoLong Wang, chengzhihao1, dpervushin,
	linux-kernel, linux-mtd, Miquel Raynal, Vignesh Raghavendra,
	yangerkun, yi zhang

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/


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 21:22                             ` Gagan Sidhu
@ 2024-06-17 22:13                               ` Gagan Sidhu
  2024-06-18  4:03                                 ` Zhihao Cheng
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-17 22:13 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Richard Weinberger, ZhaoLong Wang, chengzhihao1, dpervushin,
	linux-kernel, linux-mtd, Miquel Raynal, Vignesh Raghavendra,
	yangerkun, yi zhang

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/


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-17 22:13                               ` Gagan Sidhu
@ 2024-06-18  4:03                                 ` Zhihao Cheng
  2024-06-20 22:06                                   ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Zhihao Cheng @ 2024-06-18  4:03 UTC (permalink / raw)
  To: Gagan Sidhu, Daniel Golle
  Cc: Richard Weinberger, ZhaoLong Wang, dpervushin, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

在 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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-18  4:03                                 ` Zhihao Cheng
@ 2024-06-20 22:06                                   ` Gagan Sidhu
  2024-06-21  1:59                                     ` Zhihao Cheng
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-20 22:06 UTC (permalink / raw)
  To: Zhihao Cheng
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

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
> 


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-20 22:06                                   ` Gagan Sidhu
@ 2024-06-21  1:59                                     ` Zhihao Cheng
  2024-06-21  2:09                                       ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Zhihao Cheng @ 2024-06-21  1:59 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-21  1:59                                     ` Zhihao Cheng
@ 2024-06-21  2:09                                       ` Gagan Sidhu
  2024-06-21  3:03                                         ` Zhihao Cheng
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-21  2:09 UTC (permalink / raw)
  To: Zhihao Cheng
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang


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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-21  2:09                                       ` Gagan Sidhu
@ 2024-06-21  3:03                                         ` Zhihao Cheng
  2024-06-21  4:27                                           ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Zhihao Cheng @ 2024-06-21  3:03 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-21  3:03                                         ` Zhihao Cheng
@ 2024-06-21  4:27                                           ` Gagan Sidhu
  2024-06-21  4:55                                             ` Zhihao Cheng
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-21  4:27 UTC (permalink / raw)
  To: Zhihao Cheng
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang



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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-21  4:27                                           ` Gagan Sidhu
@ 2024-06-21  4:55                                             ` Zhihao Cheng
  2024-06-21 11:36                                               ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Zhihao Cheng @ 2024-06-21  4:55 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

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



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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-21  4:55                                             ` Zhihao Cheng
@ 2024-06-21 11:36                                               ` Gagan Sidhu
  2024-06-22  2:37                                                 ` Zhihao Cheng
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-21 11:36 UTC (permalink / raw)
  To: Zhihao Cheng
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang


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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-21 11:36                                               ` Gagan Sidhu
@ 2024-06-22  2:37                                                 ` Zhihao Cheng
  2024-06-22  2:43                                                   ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Zhihao Cheng @ 2024-06-22  2:37 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-22  2:37                                                 ` Zhihao Cheng
@ 2024-06-22  2:43                                                   ` Gagan Sidhu
  2024-06-22 21:07                                                     ` Daniel Golle
  0 siblings, 1 reply; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-22  2:43 UTC (permalink / raw)
  To: Zhihao Cheng
  Cc: Daniel Golle, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang



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


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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-22  2:43                                                   ` Gagan Sidhu
@ 2024-06-22 21:07                                                     ` Daniel Golle
  2024-06-24 19:00                                                       ` Gagan Sidhu
  0 siblings, 1 reply; 41+ messages in thread
From: Daniel Golle @ 2024-06-22 21:07 UTC (permalink / raw)
  To: Gagan Sidhu
  Cc: Zhihao Cheng, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

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.

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

* Re: [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier
  2024-06-22 21:07                                                     ` Daniel Golle
@ 2024-06-24 19:00                                                       ` Gagan Sidhu
  0 siblings, 0 replies; 41+ messages in thread
From: Gagan Sidhu @ 2024-06-24 19:00 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Zhihao Cheng, Richard Weinberger, ZhaoLong Wang, linux-kernel,
	linux-mtd, Miquel Raynal, Vignesh Raghavendra, yangerkun,
	yi zhang

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.


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

end of thread, other threads:[~2024-06-24 19:00 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-17 14:25 [PATCH v2] ubi: gluebi: Fix NULL pointer dereference caused by ftl notifier Gagan Sidhu
2024-06-17 14:31 ` Richard Weinberger
2024-06-17 15:42 ` Richard Weinberger
     [not found]   ` <14779870-BA54-4ABF-8ABF-FF1D23D172A7@mac.com>
2024-06-17 16:00     ` Richard Weinberger
2024-06-17 16:05       ` Gagan Sidhu
2024-06-17 16:52         ` Richard Weinberger
     [not found]           ` <E3E2C13C-1E52-46F2-BE2D-D2592C3369DB@mac.com>
2024-06-17 17:33             ` Gagan Sidhu
2024-06-17 17:48               ` Gagan Sidhu
2024-06-17 18:09                 ` Richard Weinberger
2024-06-17 18:18                   ` Gagan Sidhu
2024-06-17 18:32                     ` Richard Weinberger
2024-06-17 18:46                       ` Gagan Sidhu
2024-06-17 18:52                         ` Richard Weinberger
2024-06-17 20:29                           ` Daniel Golle
2024-06-17 21:22                             ` Gagan Sidhu
2024-06-17 22:13                               ` Gagan Sidhu
2024-06-18  4:03                                 ` Zhihao Cheng
2024-06-20 22:06                                   ` Gagan Sidhu
2024-06-21  1:59                                     ` Zhihao Cheng
2024-06-21  2:09                                       ` Gagan Sidhu
2024-06-21  3:03                                         ` Zhihao Cheng
2024-06-21  4:27                                           ` Gagan Sidhu
2024-06-21  4:55                                             ` Zhihao Cheng
2024-06-21 11:36                                               ` Gagan Sidhu
2024-06-22  2:37                                                 ` Zhihao Cheng
2024-06-22  2:43                                                   ` Gagan Sidhu
2024-06-22 21:07                                                     ` Daniel Golle
2024-06-24 19:00                                                       ` Gagan Sidhu
  -- strict thread matches above, loose matches on Subject: below --
2024-06-17 14:21 Gagan Sidhu
2023-10-18 12:16 ZhaoLong Wang
2023-10-19  1:57 ` Zhihao Cheng
2023-10-19 20:27 ` Richard Weinberger
2023-10-20  2:27   ` Zhihao Cheng
2023-10-21 16:09     ` Richard Weinberger
2023-10-23  6:41       ` ZhaoLong Wang
2023-10-23  6:46         ` Richard Weinberger
2023-10-23  7:12           ` ZhaoLong Wang
2023-10-23  7:16             ` Richard Weinberger
2023-10-23  7:09       ` Zhihao Cheng
2023-10-23  7:15         ` Richard Weinberger
2023-10-23  7:36           ` Zhihao Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox