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

[Zhihao Cheng]

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