Re: [PATCH 4/4] mtd: rfd_ftl: fix use-after-free

[Sean Young <sean@mess.org>]

Hi Miquel,

On Sat, Aug 07, 2021 at 12:34:09PM +0200, Miquel Raynal wrote:
> Hi Sean,
> 
> Sean Young <sean@mess.org> wrote on Sat, 7 Aug 2021 08:57:35 +0100:
> 
> > On Fri, Aug 06, 2021 at 08:21:58PM +0200, Miquel Raynal wrote:
> > > Hi Sean,
> > > 
> > > Sean Young <sean@mess.org> wrote on Tue, 13 Jul 2021 10:44:03 +0100:
> > >   
> > > > del_mtd_blktrans_dev() will kfree part, so this is a use-after-free. Use
> > > > container_of() to make it clearer what the cast is doing.
> > > > 
> > > > Signed-off-by: Sean Young <sean@mess.org>
> > > > ---
> > > >  drivers/mtd/rfd_ftl.c | 14 +++++++-------
> > > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/rfd_ftl.c b/drivers/mtd/rfd_ftl.c
> > > > index 7f5f6d247cae..af20a0a71108 100644
> > > > --- a/drivers/mtd/rfd_ftl.c
> > > > +++ b/drivers/mtd/rfd_ftl.c  
> > > 
> > > [...]
> > >   
> > > > @@ -800,10 +800,10 @@ static void rfd_ftl_remove_dev(struct  
> > > mtd_blktrans_dev *dev)  
> > > >  			part->mbd.mtd->name, i, part->blocks[i].erases);
> > > >  	}
> > > >  
> > > > -	del_mtd_blktrans_dev(dev);
> > > >  	vfree(part->sector_map);
> > > >  	kfree(part->header_cache);
> > > >  	kfree(part->blocks);
> > > > +	del_mtd_blktrans_dev(&part->mbd);  
> > > 
> > > I am not sure moving this call at the bottom of ftl_remove_dev makes
> > > sense, can we keep it where it was and just do the s/dev/part->mbd/ ?  
> > 
> > The reason for this patch is that del_mtd_blktrans_dev() kfrees its argument,
> > so both part and dev point to freed memory. This means it's a use after free.
> 
> Ok, please split this into two patches and we'll be good.

Good point.

Thank you for the review.

I'll send out v2 shortly.

Sean

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