[PATCH 1/2] mtd/nand : don't free the global data fsl_lbc_ctrl_dev->nand in fsl_elbc_chip_remove()
Artem Bityutskiy
dedekind1 at gmail.com
Thu Jun 30 07:53:13 EDT 2011
On Wed, 2011-06-29 at 11:45 -0500, Scott Wood wrote:
> On Wed, 29 Jun 2011 09:20:25 +0300
> Artem Bityutskiy <dedekind1 at gmail.com> wrote:
>
> > On Tue, 2011-06-28 at 09:50 +0800, b35362 at freescale.com wrote:
> > > From: Liu Shuo <b35362 at freescale.com>
> > >
> > > The global data fsl_lbc_ctrl_dev->nand don't have to be freed in
> > > fsl_elbc_chip_remove(). The right place to do that is in fsl_elbc_nand_remove()
> > > if elbc_fcm_ctrl->counter is zero.
> > >
> > > Signed-off-by: Liu Shuo <b35362 at freescale.com>
> > > ---
> > > drivers/mtd/nand/fsl_elbc_nand.c | 1 -
> > > 1 files changed, 0 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/fsl_elbc_nand.c b/drivers/mtd/nand/fsl_elbc_nand.c
> > > index 0bb254c..a212116 100644
> > > --- a/drivers/mtd/nand/fsl_elbc_nand.c
> > > +++ b/drivers/mtd/nand/fsl_elbc_nand.c
> > > @@ -829,7 +829,6 @@ static int fsl_elbc_chip_remove(struct fsl_elbc_mtd *priv)
> > >
> > > elbc_fcm_ctrl->chips[priv->bank] = NULL;
> > > kfree(priv);
> > > - kfree(elbc_fcm_ctrl);
> > > return 0;
> > > }
> >
> > Do we have to assign fsl_lbc_ctrl_dev->nand to NULL in
> > fsl_elbc_nand_remove() then? I think that assignment can be killed then.
> >
> > if (!elbc_fcm_ctrl->counter) {
> > fsl_lbc_ctrl_dev->nand = NULL;
> > kfree(elbc_fcm_ctrl);
> > }
> >
>
> If we're freeing fsl_lbc_ctrl, we'd better get rid of references to it...
Yes, on the one hand this is a good defensive programming practice, on
the other hand it hides double-free bugs. Like this patch fixes a
double-free bug, and why it was noticed before? I thought may be because
of this NULL assignment?
I do not insist though, that was just a suggestion/question.
--
Best Regards,
Artem Bityutskiy
More information about the linux-mtd
mailing list