Messages in this thread Patch in this message | | | Subject | Re: [PATCH] params: fix potential memory leak in add_sysfs_param() | From | David Woodhouse <> | Date | Wed, 20 Aug 2014 18:10:14 -0500 |
| |
On Wed, 2014-08-20 at 22:17 +0000, Woodhouse, David wrote: > > Except that in the failure case we *free* the old mk->mp and never free > mk->mp->grp.attrs so it *is* indeed lost. > > A simpler version of Arjun's patch might look like this: > > diff --git a/kernel/params.c b/kernel/params.c > index 34f5270..f9459bc 100644 > --- a/kernel/params.c > +++ b/kernel/params.c > @@ -613,7 +613,6 @@ static __modinit int add_sysfs_param(struct module_kobject *mk, > sizeof(*mk->mp) + sizeof(mk->mp->attrs[0]) * (num+1), > GFP_KERNEL); > if (!new) { > - kfree(attrs); > err = -ENOMEM; > goto fail; > } > @@ -653,7 +652,10 @@ static __modinit int add_sysfs_param(struct module_kobject *mk, > fail_free_new: > kfree(new); > fail: > - mk->mp = NULL; > + if (mk->mp) { > + kfree(mk->mp->grp.attrs); > + mk->mp = NULL; > + } > return err; > }
Of course this and Arjun's original *both* fail to fix the classic bug in the 'new = krealloc(mk->mp…)' bit, failing to free the old mk->mp if the realloc fails. I think my alternative fix that didn't kill the old params is probably the better way forward.
-- dwmw2
[unhandled content-type:application/x-pkcs7-signature] | |