Messages in this thread |  | | Subject | Re: [PATCH v3] bus: fsl-mc: fix invalid free in fsl_mc_device_add | From | Laurentiu Tudor <> | Date | Fri, 10 Jul 2020 13:32:49 +0300 |
| |
On 7/9/2020 6:31 PM, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > clang static analysis flags this error > > fsl-mc-bus.c:695:2: warning: Attempt to free released memory [unix.Malloc] > kfree(mc_dev); > ^~~~~~~~~~~~~ > > The problem block of code is > > mc_bus = kzalloc(sizeof(*mc_bus), GFP_KERNEL); > if (!mc_bus) > return -ENOMEM; > > mc_dev = &mc_bus->mc_dev; > > mc_bus's structure contains a mc_dev element, > freeing it later is not appropriate. > > So check if mc_bus was allocated before freeing mc_dev > > This is a case where checkpatch > > WARNING: kfree(NULL) is safe and this check is probably not required > + if (mc_bus) > + kfree(mc_bus); > > is wrong. > > Fixes: a042fbed0290 ("staging: fsl-mc: simplify couple of deallocations") > > Signed-off-by: Tom Rix <trix@redhat.com> > --- > v1: add a comment to explain freeing uniqueness > v2: add gregkh's suggestion to comment. > > drivers/bus/fsl-mc/fsl-mc-bus.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c > index 40526da5c6a6..839d96d03f0d 100644 > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c > @@ -691,8 +691,16 @@ int fsl_mc_device_add(struct fsl_mc_obj_desc *obj_desc, > > error_cleanup_dev: > kfree(mc_dev->regions); > - kfree(mc_bus); > - kfree(mc_dev); > + /* > + * mc_bus allocates a private version of mc_dev > + * it is not appropriate to free the private version. > + * Which means we have to check the pointer before freeing it. > + * Do not remove this check. > + */ > + if (mc_bus) > + kfree(mc_bus); > + else > + kfree(mc_dev); > > return error; > } >
Thanks, Tom!
Reviewed-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
--- Best Regards, Laurentiu
|  |