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

From: Anton Vorontsov <[email-masked]>
Date: Mon, 20 Sep 2010 17:19:07 +0400

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.

Thanks,

-- 
Anton Vorontsov
email: [email-masked]
irc://irc.freenode.net/bd2