[Patch] hdac_sysfs: Fix a memory leaking bug in sound/hda/hdac_sysfs.c file of Linux 5.1

[Patch] hdac_sysfs: Fix a memory leaking bug in sound/hda/hdac_sysfs.c file of Linux 5.1

[Gen Zhang]

tree->root and tree->nodes are allocated by memory allocation 
functions. And tree is also an allocated memory. When allocation of 
tree->root and tree->nodes fails, not freeing tree will leak memory. 
Thus we should free tree in this situation.

Signed-off-by: Gen Zhang <blackgod016574@gmail.com>

---
diff --git a/sound/hda/hdac_sysfs.c b/sound/hda/hdac_sysfs.c
index fb2aa34..5d8a939 100644
--- a/sound/hda/hdac_sysfs.c
+++ b/sound/hda/hdac_sysfs.c
@@ -370,12 +370,12 @@ static int widget_tree_create(struct hdac_device *codec)
 
 	tree->root = kobject_create_and_add("widgets", &codec->dev.kobj);
 	if (!tree->root)
-		return -ENOMEM;
+		goto free_tree;
 
 	tree->nodes = kcalloc(codec->num_nodes + 1, sizeof(*tree->nodes),
 			      GFP_KERNEL);
 	if (!tree->nodes)
-		return -ENOMEM;
+		goto free_tree;
 
 	for (i = 0, nid = codec->start_nid; i < codec->num_nodes; i++, nid++) {
 		err = add_widget_node(tree->root, nid, &widget_node_group,
@@ -393,6 +393,9 @@ static int widget_tree_create(struct hdac_device *codec)
 
 	kobject_uevent(tree->root, KOBJ_CHANGE);
 	return 0;
+free_tree:
+	kfree(tree);
+	return -ENOMEM;
 }
 
 int hda_widget_sysfs_init(struct hdac_device *codec)
---

Re: [Patch] hdac_sysfs: Fix a memory leaking bug in sound/hda/hdac_sysfs.c file of Linux 5.1

[Takashi Iwai]

On Thu, 16 May 2019 10:40:03 +0200,
Gen Zhang wrote:
> 
> tree->root and tree->nodes are allocated by memory allocation 
> functions. And tree is also an allocated memory. When allocation of 
> tree->root and tree->nodes fails, not freeing tree will leak memory. 
> Thus we should free tree in this situation.

No, the leftover is freed in the caller side by calling
widget_tree_free().


thanks,

Takashi

> 
> Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> 
> ---
> diff --git a/sound/hda/hdac_sysfs.c b/sound/hda/hdac_sysfs.c
> index fb2aa34..5d8a939 100644
> --- a/sound/hda/hdac_sysfs.c
> +++ b/sound/hda/hdac_sysfs.c
> @@ -370,12 +370,12 @@ static int widget_tree_create(struct hdac_device *codec)
>  
>  	tree->root = kobject_create_and_add("widgets", &codec->dev.kobj);
>  	if (!tree->root)
> -		return -ENOMEM;
> +		goto free_tree;
>  
>  	tree->nodes = kcalloc(codec->num_nodes + 1, sizeof(*tree->nodes),
>  			      GFP_KERNEL);
>  	if (!tree->nodes)
> -		return -ENOMEM;
> +		goto free_tree;
>  
>  	for (i = 0, nid = codec->start_nid; i < codec->num_nodes; i++, nid++) {
>  		err = add_widget_node(tree->root, nid, &widget_node_group,
> @@ -393,6 +393,9 @@ static int widget_tree_create(struct hdac_device *codec)
>  
>  	kobject_uevent(tree->root, KOBJ_CHANGE);
>  	return 0;
> +free_tree:
> +	kfree(tree);
> +	return -ENOMEM;
>  }
>  
>  int hda_widget_sysfs_init(struct hdac_device *codec)
> ---
> 

Re: [Patch] hdac_sysfs: Fix a memory leaking bug in sound/hda/hdac_sysfs.c file of Linux 5.1

[Gen Zhang]

On Thu, May 16, 2019 at 10:49:43AM +0200, Takashi Iwai wrote:
> On Thu, 16 May 2019 10:40:03 +0200,
> Gen Zhang wrote:
> > 
> > tree->root and tree->nodes are allocated by memory allocation 
> > functions. And tree is also an allocated memory. When allocation of 
> > tree->root and tree->nodes fails, not freeing tree will leak memory. 
> > Thus we should free tree in this situation.
> 
> No, the leftover is freed in the caller side by calling
> widget_tree_free().
> 
> 
> thanks,
> 
> Takashi
> 
> > 
> > Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> > 
> > ---
> > diff --git a/sound/hda/hdac_sysfs.c b/sound/hda/hdac_sysfs.c
> > index fb2aa34..5d8a939 100644
> > --- a/sound/hda/hdac_sysfs.c
> > +++ b/sound/hda/hdac_sysfs.c
> > @@ -370,12 +370,12 @@ static int widget_tree_create(struct hdac_device *codec)
> >  
> >  	tree->root = kobject_create_and_add("widgets", &codec->dev.kobj);
> >  	if (!tree->root)
> > -		return -ENOMEM;
> > +		goto free_tree;
> >  
> >  	tree->nodes = kcalloc(codec->num_nodes + 1, sizeof(*tree->nodes),
> >  			      GFP_KERNEL);
> >  	if (!tree->nodes)
> > -		return -ENOMEM;
> > +		goto free_tree;
> >  
> >  	for (i = 0, nid = codec->start_nid; i < codec->num_nodes; i++, nid++) {
> >  		err = add_widget_node(tree->root, nid, &widget_node_group,
> > @@ -393,6 +393,9 @@ static int widget_tree_create(struct hdac_device *codec)
> >  
> >  	kobject_uevent(tree->root, KOBJ_CHANGE);
> >  	return 0;
> > +free_tree:
> > +	kfree(tree);
> > +	return -ENOMEM;
> >  }
> >  
> >  int hda_widget_sysfs_init(struct hdac_device *codec)
> > ---
> > 
It is freed in the caller site :)
But it is really a rare free usage.
Thanks
Gen