All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity
@ 2019-08-19  1:51 zhengbin
  2019-09-04  8:46 ` zhengbin (A)
  2019-09-06  9:12 ` Laurent Pinchart
  0 siblings, 2 replies; 6+ messages in thread
From: zhengbin @ 2019-08-19  1:51 UTC (permalink / raw)
  To: sakari.ailus, laurent.pinchart, mchehab, linux-media; +Cc: yi.zhang, zhengbin13

In media_device_register_entity, if media_graph_walk_init fails,
need to free the previously memory.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: zhengbin <zhengbin13@huawei.com>
---
 drivers/media/mc/mc-device.c | 65 ++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index e19df51..da80883 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -575,6 +575,38 @@ static void media_device_release(struct media_devnode *devnode)
 	dev_dbg(devnode->parent, "Media device released\n");
 }

+static void __media_device_unregister_entity(struct media_entity *entity)
+{
+	struct media_device *mdev = entity->graph_obj.mdev;
+	struct media_link *link, *tmp;
+	struct media_interface *intf;
+	unsigned int i;
+
+	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
+
+	/* Remove all interface links pointing to this entity */
+	list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
+		list_for_each_entry_safe(link, tmp, &intf->links, list) {
+			if (link->entity == entity)
+				__media_remove_intf_link(link);
+		}
+	}
+
+	/* Remove all data links that belong to this entity */
+	__media_entity_remove_links(entity);
+
+	/* Remove all pads that belong to this entity */
+	for (i = 0; i < entity->num_pads; i++)
+		media_gobj_destroy(&entity->pads[i].graph_obj);
+
+	/* Remove the entity */
+	media_gobj_destroy(&entity->graph_obj);
+
+	/* invoke entity_notify callbacks to handle entity removal?? */
+
+	entity->graph_obj.mdev = NULL;
+}
+
 /**
  * media_device_register_entity - Register an entity with a media device
  * @mdev:	The media device
@@ -632,6 +664,7 @@ int __must_check media_device_register_entity(struct media_device *mdev,
 		 */
 		ret = media_graph_walk_init(&new, mdev);
 		if (ret) {
+			__media_device_unregister_entity(entity);
 			mutex_unlock(&mdev->graph_mutex);
 			return ret;
 		}
@@ -644,38 +677,6 @@ int __must_check media_device_register_entity(struct media_device *mdev,
 }
 EXPORT_SYMBOL_GPL(media_device_register_entity);

-static void __media_device_unregister_entity(struct media_entity *entity)
-{
-	struct media_device *mdev = entity->graph_obj.mdev;
-	struct media_link *link, *tmp;
-	struct media_interface *intf;
-	unsigned int i;
-
-	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
-
-	/* Remove all interface links pointing to this entity */
-	list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
-		list_for_each_entry_safe(link, tmp, &intf->links, list) {
-			if (link->entity == entity)
-				__media_remove_intf_link(link);
-		}
-	}
-
-	/* Remove all data links that belong to this entity */
-	__media_entity_remove_links(entity);
-
-	/* Remove all pads that belong to this entity */
-	for (i = 0; i < entity->num_pads; i++)
-		media_gobj_destroy(&entity->pads[i].graph_obj);
-
-	/* Remove the entity */
-	media_gobj_destroy(&entity->graph_obj);
-
-	/* invoke entity_notify callbacks to handle entity removal?? */
-
-	entity->graph_obj.mdev = NULL;
-}
-
 void media_device_unregister_entity(struct media_entity *entity)
 {
 	struct media_device *mdev = entity->graph_obj.mdev;
--
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity
  2019-08-19  1:51 [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity zhengbin
@ 2019-09-04  8:46 ` zhengbin (A)
  2019-09-06  9:12 ` Laurent Pinchart
  1 sibling, 0 replies; 6+ messages in thread
From: zhengbin (A) @ 2019-09-04  8:46 UTC (permalink / raw)
  To: sakari.ailus, laurent.pinchart, mchehab, linux-media; +Cc: yi.zhang

ping

On 2019/8/19 9:51, zhengbin wrote:
> In media_device_register_entity, if media_graph_walk_init fails,
> need to free the previously memory.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: zhengbin <zhengbin13@huawei.com>
> ---
>  drivers/media/mc/mc-device.c | 65 ++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
> index e19df51..da80883 100644
> --- a/drivers/media/mc/mc-device.c
> +++ b/drivers/media/mc/mc-device.c
> @@ -575,6 +575,38 @@ static void media_device_release(struct media_devnode *devnode)
>  	dev_dbg(devnode->parent, "Media device released\n");
>  }
>
> +static void __media_device_unregister_entity(struct media_entity *entity)
> +{
> +	struct media_device *mdev = entity->graph_obj.mdev;
> +	struct media_link *link, *tmp;
> +	struct media_interface *intf;
> +	unsigned int i;
> +
> +	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
> +
> +	/* Remove all interface links pointing to this entity */
> +	list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
> +		list_for_each_entry_safe(link, tmp, &intf->links, list) {
> +			if (link->entity == entity)
> +				__media_remove_intf_link(link);
> +		}
> +	}
> +
> +	/* Remove all data links that belong to this entity */
> +	__media_entity_remove_links(entity);
> +
> +	/* Remove all pads that belong to this entity */
> +	for (i = 0; i < entity->num_pads; i++)
> +		media_gobj_destroy(&entity->pads[i].graph_obj);
> +
> +	/* Remove the entity */
> +	media_gobj_destroy(&entity->graph_obj);
> +
> +	/* invoke entity_notify callbacks to handle entity removal?? */
> +
> +	entity->graph_obj.mdev = NULL;
> +}
> +
>  /**
>   * media_device_register_entity - Register an entity with a media device
>   * @mdev:	The media device
> @@ -632,6 +664,7 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  		 */
>  		ret = media_graph_walk_init(&new, mdev);
>  		if (ret) {
> +			__media_device_unregister_entity(entity);
>  			mutex_unlock(&mdev->graph_mutex);
>  			return ret;
>  		}
> @@ -644,38 +677,6 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  }
>  EXPORT_SYMBOL_GPL(media_device_register_entity);
>
> -static void __media_device_unregister_entity(struct media_entity *entity)
> -{
> -	struct media_device *mdev = entity->graph_obj.mdev;
> -	struct media_link *link, *tmp;
> -	struct media_interface *intf;
> -	unsigned int i;
> -
> -	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
> -
> -	/* Remove all interface links pointing to this entity */
> -	list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
> -		list_for_each_entry_safe(link, tmp, &intf->links, list) {
> -			if (link->entity == entity)
> -				__media_remove_intf_link(link);
> -		}
> -	}
> -
> -	/* Remove all data links that belong to this entity */
> -	__media_entity_remove_links(entity);
> -
> -	/* Remove all pads that belong to this entity */
> -	for (i = 0; i < entity->num_pads; i++)
> -		media_gobj_destroy(&entity->pads[i].graph_obj);
> -
> -	/* Remove the entity */
> -	media_gobj_destroy(&entity->graph_obj);
> -
> -	/* invoke entity_notify callbacks to handle entity removal?? */
> -
> -	entity->graph_obj.mdev = NULL;
> -}
> -
>  void media_device_unregister_entity(struct media_entity *entity)
>  {
>  	struct media_device *mdev = entity->graph_obj.mdev;
> --
> 2.7.4
>
>
> .
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity
  2019-08-19  1:51 [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity zhengbin
  2019-09-04  8:46 ` zhengbin (A)
@ 2019-09-06  9:12 ` Laurent Pinchart
  2019-09-06 10:11   ` Sakari Ailus
  1 sibling, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2019-09-06  9:12 UTC (permalink / raw)
  To: zhengbin; +Cc: sakari.ailus, mchehab, linux-media, yi.zhang

Hello Zhengbin,

On Mon, Aug 19, 2019 at 09:51:30AM +0800, zhengbin wrote:
> In media_device_register_entity, if media_graph_walk_init fails,
> need to free the previously memory.
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: zhengbin <zhengbin13@huawei.com>

This looks good to me.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

and applied to my tree, for v5.5.

> ---
>  drivers/media/mc/mc-device.c | 65 ++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
> index e19df51..da80883 100644
> --- a/drivers/media/mc/mc-device.c
> +++ b/drivers/media/mc/mc-device.c
> @@ -575,6 +575,38 @@ static void media_device_release(struct media_devnode *devnode)
>  	dev_dbg(devnode->parent, "Media device released\n");
>  }
> 
> +static void __media_device_unregister_entity(struct media_entity *entity)
> +{
> +	struct media_device *mdev = entity->graph_obj.mdev;
> +	struct media_link *link, *tmp;
> +	struct media_interface *intf;
> +	unsigned int i;
> +
> +	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
> +
> +	/* Remove all interface links pointing to this entity */
> +	list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
> +		list_for_each_entry_safe(link, tmp, &intf->links, list) {
> +			if (link->entity == entity)
> +				__media_remove_intf_link(link);
> +		}
> +	}
> +
> +	/* Remove all data links that belong to this entity */
> +	__media_entity_remove_links(entity);
> +
> +	/* Remove all pads that belong to this entity */
> +	for (i = 0; i < entity->num_pads; i++)
> +		media_gobj_destroy(&entity->pads[i].graph_obj);
> +
> +	/* Remove the entity */
> +	media_gobj_destroy(&entity->graph_obj);
> +
> +	/* invoke entity_notify callbacks to handle entity removal?? */
> +
> +	entity->graph_obj.mdev = NULL;
> +}
> +
>  /**
>   * media_device_register_entity - Register an entity with a media device
>   * @mdev:	The media device
> @@ -632,6 +664,7 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  		 */
>  		ret = media_graph_walk_init(&new, mdev);
>  		if (ret) {
> +			__media_device_unregister_entity(entity);
>  			mutex_unlock(&mdev->graph_mutex);
>  			return ret;
>  		}
> @@ -644,38 +677,6 @@ int __must_check media_device_register_entity(struct media_device *mdev,
>  }
>  EXPORT_SYMBOL_GPL(media_device_register_entity);
> 
> -static void __media_device_unregister_entity(struct media_entity *entity)
> -{
> -	struct media_device *mdev = entity->graph_obj.mdev;
> -	struct media_link *link, *tmp;
> -	struct media_interface *intf;
> -	unsigned int i;
> -
> -	ida_free(&mdev->entity_internal_idx, entity->internal_idx);
> -
> -	/* Remove all interface links pointing to this entity */
> -	list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
> -		list_for_each_entry_safe(link, tmp, &intf->links, list) {
> -			if (link->entity == entity)
> -				__media_remove_intf_link(link);
> -		}
> -	}
> -
> -	/* Remove all data links that belong to this entity */
> -	__media_entity_remove_links(entity);
> -
> -	/* Remove all pads that belong to this entity */
> -	for (i = 0; i < entity->num_pads; i++)
> -		media_gobj_destroy(&entity->pads[i].graph_obj);
> -
> -	/* Remove the entity */
> -	media_gobj_destroy(&entity->graph_obj);
> -
> -	/* invoke entity_notify callbacks to handle entity removal?? */
> -
> -	entity->graph_obj.mdev = NULL;
> -}
> -
>  void media_device_unregister_entity(struct media_entity *entity)
>  {
>  	struct media_device *mdev = entity->graph_obj.mdev;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity
  2019-09-06  9:12 ` Laurent Pinchart
@ 2019-09-06 10:11   ` Sakari Ailus
  2019-09-06 10:36     ` Laurent Pinchart
  0 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2019-09-06 10:11 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: zhengbin, mchehab, linux-media, yi.zhang

On Fri, Sep 06, 2019 at 12:12:03PM +0300, Laurent Pinchart wrote:
> Hello Zhengbin,
> 
> On Mon, Aug 19, 2019 at 09:51:30AM +0800, zhengbin wrote:
> > In media_device_register_entity, if media_graph_walk_init fails,
> > need to free the previously memory.
> > 
> > Reported-by: Hulk Robot <hulkci@huawei.com>
> > Signed-off-by: zhengbin <zhengbin13@huawei.com>
> 
> This looks good to me.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> and applied to my tree, for v5.5.

Hmm. This is in my tree as well. Would you like to drop it from yours? :-)

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity
  2019-09-06 10:11   ` Sakari Ailus
@ 2019-09-06 10:36     ` Laurent Pinchart
  2019-09-06 10:59       ` Sakari Ailus
  0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2019-09-06 10:36 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: zhengbin, mchehab, linux-media, yi.zhang

Hi Sakari,

On Fri, Sep 06, 2019 at 01:11:34PM +0300, Sakari Ailus wrote:
> On Fri, Sep 06, 2019 at 12:12:03PM +0300, Laurent Pinchart wrote:
> > On Mon, Aug 19, 2019 at 09:51:30AM +0800, zhengbin wrote:
> > > In media_device_register_entity, if media_graph_walk_init fails,
> > > need to free the previously memory.
> > > 
> > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > Signed-off-by: zhengbin <zhengbin13@huawei.com>
> > 
> > This looks good to me.
> > 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > and applied to my tree, for v5.5.
> 
> Hmm. This is in my tree as well. Would you like to drop it from yours? :-)

Sure :-)

I wonder if we should setup a shared git tree for this.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity
  2019-09-06 10:36     ` Laurent Pinchart
@ 2019-09-06 10:59       ` Sakari Ailus
  0 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2019-09-06 10:59 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: zhengbin, mchehab, linux-media, yi.zhang

On Fri, Sep 06, 2019 at 01:36:02PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Fri, Sep 06, 2019 at 01:11:34PM +0300, Sakari Ailus wrote:
> > On Fri, Sep 06, 2019 at 12:12:03PM +0300, Laurent Pinchart wrote:
> > > On Mon, Aug 19, 2019 at 09:51:30AM +0800, zhengbin wrote:
> > > > In media_device_register_entity, if media_graph_walk_init fails,
> > > > need to free the previously memory.
> > > > 
> > > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > > Signed-off-by: zhengbin <zhengbin13@huawei.com>
> > > 
> > > This looks good to me.
> > > 
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > and applied to my tree, for v5.5.
> > 
> > Hmm. This is in my tree as well. Would you like to drop it from yours? :-)
> 
> Sure :-)
> 
> I wonder if we should setup a shared git tree for this.

I think following the patchwork status should be enough for now.

I marked it as accepted but forgot to assign it myself in this case. I'll
try assign them as well. It'd be actually nice if Patchwork did that by
default, as this is generally what needs to be done.

What do you think?

If there would be more patches, it'd make sense to rethink this IMO. But
two people eagerly merging patches is much better than none at all. ;)

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-09-06 11:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-19  1:51 [PATCH v2] media: mc-device.c: fix memleak in media_device_register_entity zhengbin
2019-09-04  8:46 ` zhengbin (A)
2019-09-06  9:12 ` Laurent Pinchart
2019-09-06 10:11   ` Sakari Ailus
2019-09-06 10:36     ` Laurent Pinchart
2019-09-06 10:59       ` Sakari Ailus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.