Messages in this thread | | | Subject | Re: [PATCH 1/2] of: overlay: fix properties memory leak | From | Frank Rowand <> | Date | Thu, 21 Nov 2019 10:55:26 -0600 |
| |
Hi Vincent,
On 11/18/19 7:28 AM, Vincent Whitchurch wrote: > No changeset entries are created for #address-cells and #size-cells > properties, but the duplicated properies are never freed. This results > in a memory leak which is detected by kmemleak: > > unreferenced object 0x85887180 (size 64): > backtrace: > kmem_cache_alloc_trace+0x1fb/0x1fc > __of_prop_dup+0x25/0x7c > add_changeset_property+0x17f/0x370 > build_changeset_next_level+0x29/0x20c > of_overlay_fdt_apply+0x32b/0x6b4 > ... > > Fixes: 6f75118800acf77f8 ("of: overlay: validate overlay properties #address-cells and #size-cells") > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > --- > drivers/of/overlay.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > index c423e94baf0f..5f8869e2a8b3 100644 > --- a/drivers/of/overlay.c > +++ b/drivers/of/overlay.c > @@ -360,7 +360,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs, > pr_err("WARNING: memory leak will occur if overlay removed, property: %pOF/%s\n", > target->np, new_prop->name); > > - if (ret) { > + if (ret || !check_for_non_overlay_node) { > kfree(new_prop->name); > kfree(new_prop->value); > kfree(new_prop); >
Thanks for finding and proposing a fix for the memory leak.
The proposed patch conveniently uses check_for_non_overlay_node which leads to a nice small patch. But ends up adding an additional hidden meaning to the variable, resulting in more fragile code.
I will propose a different solution and ask you to test it to make sure it also solves the memory leak.
-Frank
| |