Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Fri, 27 Oct 2023 19:40:26 +0200
From:   Miquel Raynal <miquel.raynal@bootlin.com>
To:     ZhaoLong Wang <wangzhaolong1@huawei.com>
Cc:     <richard@nod.at>, <vigneshr@ti.com>, <Artem.Bityutskiy@nokia.com>,
        <dpervushin@embeddedalley.com>, <linux-mtd@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <chengzhihao1@huawei.com>,
        <yi.zhang@huawei.com>, <yangerkun@huawei.com>
Subject: Re: [PATCH v3] mtd: Fix gluebi NULL pointer dereference caused by
 ftl notifier
Message-ID: <20231027194026.1bc32dfe@xps-13>
In-Reply-To: <20231027012033.50280-1-wangzhaolong1@huawei.com>
References: <20231027012033.50280-1-wangzhaolong1@huawei.com>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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
> =E2=80=98gluebi->desc=E2=80=99 in gluebi_read().
>=20
> 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
>=20
> Detailed reproduction information available at the link[1],
>=20
> 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=3D217992 [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(-)
>=20
> 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;
> =20
> -	if (mtd->type =3D=3D MTD_ABSENT)
> +	if (mtd->type =3D=3D MTD_ABSENT || mtd->type =3D=3D MTD_UBIVOLUME)
>  		return;
> =20
>  	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 !=3D MTD_ABSENT)
> +		if (mtd->type !=3D MTD_ABSENT && mtd->type !=3D MTD_UBIVOLUME)
>  			tr->add_mtd(tr, mtd);
>  	mutex_unlock(&mtd_table_mutex);
>  	return 0;


Thanks,
Miqu=C3=A8l