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

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi ZhaoLong,

wangzhaolong1@huawei.com wrote on Fri, 27 Oct 2023 09:20:33 +0800:

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

flt ?					notifier

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

Glitches?

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

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

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

This sentence sounds wrong :)

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


Thanks,
Miquèl