[PATCH 2/3 v4] P4080/mtd: Only make elbc nand driver detect nand flash partitions

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru at gmail.com]
> Sent: Monday, September 20, 2010 21:19 PM
> To: Zang Roy-R61911
> Cc: linux-mtd at lists.infradead.org; dwmw2 at infradead.org; dedekind1 at gmail.com;
> akpm at linux-foundation.org; Lan Chunhe-B25806; Wood Scott-B07421; Gala Kumar-
> B11780; linuxppc-dev at ozlabs.org
> Subject: Re: [PATCH 2/3 v4] P4080/mtd: Only make elbc nand driver detect nand
> flash partitions
> 
> On Fri, Sep 17, 2010 at 03:01:08PM +0800, Roy Zang wrote:
> [...]
> > +static struct mutex fsl_elbc_nand_mutex;
> > +
> > +static int __devinit fsl_elbc_nand_probe(struct platform_device *dev)
> >  {
> > -	struct fsl_lbc_regs __iomem *lbc = ctrl->regs;
> > +	struct fsl_lbc_regs __iomem *lbc;
> >  	struct fsl_elbc_mtd *priv;
> >  	struct resource res;
> > +	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = NULL;
> 
> No need for = NULL.
> 
> [...]
> > -	ctrl->chips[bank] = priv;
> > +	mutex_init(&fsl_elbc_nand_mutex);
> 
> This may cause all sorts of misbehaviours, e.g.
> 
> A: mutex_init(foo)
> A: mutex_lock(foo)
> B: mutex_init(foo)   <- destroyed "A"-context mutex.
> A: mutex_unlock(foo) <- oops
> 
> Instead of dynamically initializing the mutex, just define it
> with DEFINE_MUTEX() above.
> 
> (Btw, #include <linux/mutex.h> is needed.)
> 
> > +
> > +	mutex_lock(&fsl_elbc_nand_mutex);
> [...]
> > -static int __devinit fsl_elbc_ctrl_init(struct fsl_elbc_ctrl *ctrl)
> > +static int fsl_elbc_nand_remove(struct platform_device *dev)
> [...]
> > +	struct fsl_elbc_fcm_ctrl *elbc_fcm_ctrl = fsl_lbc_ctrl_dev->nand;
> [...]
> > +		if (elbc_fcm_ctrl->chips[i])
> > +			fsl_elbc_chip_remove(elbc_fcm_ctrl->chips[i]);
> [...]
> > +	fsl_lbc_ctrl_dev->nand = NULL;
> > +	kfree(elbc_fcm_ctrl);
> 
> Will cause NULL dereference and/or use-after-free for other
> elbc nand instances. To avoid that, reference counting for
> elbc_fcm_ctrl is required.
Make sense.
will update.
Roy