* Re: [PATCH] media: mc-device.c: fix memleak in media_device_register_entity
2019-08-16 3:33 [PATCH] media: mc-device.c: fix memleak in media_device_register_entity zhengbin
@ 2019-08-16 9:49 ` Sakari Ailus
2019-08-16 11:57 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2019-08-16 9:49 UTC (permalink / raw)
To: zhengbin; +Cc: laurent.pinchart, mchehab, linux-media, yi.zhang
Hi Zhenbin,
On Fri, Aug 16, 2019 at 11:33:02AM +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>
> ---
> drivers/media/mc/mc-device.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
> index e19df51..939be00 100644
> --- a/drivers/media/mc/mc-device.c
> +++ b/drivers/media/mc/mc-device.c
> @@ -632,6 +632,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;
> }
This does not compile; the function is defined after it's used here.
Could you move the function definition up, just above this function,
please?
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] media: mc-device.c: fix memleak in media_device_register_entity
2019-08-16 3:33 [PATCH] media: mc-device.c: fix memleak in media_device_register_entity zhengbin
2019-08-16 9:49 ` Sakari Ailus
@ 2019-08-16 11:57 ` kbuild test robot
1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2019-08-16 11:57 UTC (permalink / raw)
To: zhengbin
Cc: kbuild-all, sakari.ailus, laurent.pinchart, mchehab, linux-media,
yi.zhang, zhengbin13
[-- Attachment #1: Type: text/plain, Size: 5929 bytes --]
Hi zhengbin,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[cannot apply to v5.3-rc4 next-20190814]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/zhengbin/media-mc-device-c-fix-memleak-in-media_device_register_entity/20190816-191628
base: git://linuxtv.org/media_tree.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
drivers/media//mc/mc-device.c: In function 'media_device_register_entity':
>> drivers/media//mc/mc-device.c:635:4: error: implicit declaration of function '__media_device_unregister_entity'; did you mean 'media_device_unregister_entity'? [-Werror=implicit-function-declaration]
__media_device_unregister_entity(entity);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
media_device_unregister_entity
drivers/media//mc/mc-device.c: At top level:
>> drivers/media//mc/mc-device.c:648:13: warning: conflicting types for '__media_device_unregister_entity'
static void __media_device_unregister_entity(struct media_entity *entity)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/media//mc/mc-device.c:648:13: error: static declaration of '__media_device_unregister_entity' follows non-static declaration
drivers/media//mc/mc-device.c:635:4: note: previous implicit declaration of '__media_device_unregister_entity' was here
__media_device_unregister_entity(entity);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +635 drivers/media//mc/mc-device.c
577
578 /**
579 * media_device_register_entity - Register an entity with a media device
580 * @mdev: The media device
581 * @entity: The entity
582 */
583 int __must_check media_device_register_entity(struct media_device *mdev,
584 struct media_entity *entity)
585 {
586 struct media_entity_notify *notify, *next;
587 unsigned int i;
588 int ret;
589
590 if (entity->function == MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN ||
591 entity->function == MEDIA_ENT_F_UNKNOWN)
592 dev_warn(mdev->dev,
593 "Entity type for entity %s was not initialized!\n",
594 entity->name);
595
596 /* Warn if we apparently re-register an entity */
597 WARN_ON(entity->graph_obj.mdev != NULL);
598 entity->graph_obj.mdev = mdev;
599 INIT_LIST_HEAD(&entity->links);
600 entity->num_links = 0;
601 entity->num_backlinks = 0;
602
603 ret = ida_alloc_min(&mdev->entity_internal_idx, 1, GFP_KERNEL);
604 if (ret < 0)
605 return ret;
606 entity->internal_idx = ret;
607
608 mutex_lock(&mdev->graph_mutex);
609 mdev->entity_internal_idx_max =
610 max(mdev->entity_internal_idx_max, entity->internal_idx);
611
612 /* Initialize media_gobj embedded at the entity */
613 media_gobj_create(mdev, MEDIA_GRAPH_ENTITY, &entity->graph_obj);
614
615 /* Initialize objects at the pads */
616 for (i = 0; i < entity->num_pads; i++)
617 media_gobj_create(mdev, MEDIA_GRAPH_PAD,
618 &entity->pads[i].graph_obj);
619
620 /* invoke entity_notify callbacks */
621 list_for_each_entry_safe(notify, next, &mdev->entity_notify, list)
622 notify->notify(entity, notify->notify_data);
623
624 if (mdev->entity_internal_idx_max
625 >= mdev->pm_count_walk.ent_enum.idx_max) {
626 struct media_graph new = { .top = 0 };
627
628 /*
629 * Initialise the new graph walk before cleaning up
630 * the old one in order not to spoil the graph walk
631 * object of the media device if graph walk init fails.
632 */
633 ret = media_graph_walk_init(&new, mdev);
634 if (ret) {
> 635 __media_device_unregister_entity(entity);
636 mutex_unlock(&mdev->graph_mutex);
637 return ret;
638 }
639 media_graph_walk_cleanup(&mdev->pm_count_walk);
640 mdev->pm_count_walk = new;
641 }
642 mutex_unlock(&mdev->graph_mutex);
643
644 return 0;
645 }
646 EXPORT_SYMBOL_GPL(media_device_register_entity);
647
> 648 static void __media_device_unregister_entity(struct media_entity *entity)
649 {
650 struct media_device *mdev = entity->graph_obj.mdev;
651 struct media_link *link, *tmp;
652 struct media_interface *intf;
653 unsigned int i;
654
655 ida_free(&mdev->entity_internal_idx, entity->internal_idx);
656
657 /* Remove all interface links pointing to this entity */
658 list_for_each_entry(intf, &mdev->interfaces, graph_obj.list) {
659 list_for_each_entry_safe(link, tmp, &intf->links, list) {
660 if (link->entity == entity)
661 __media_remove_intf_link(link);
662 }
663 }
664
665 /* Remove all data links that belong to this entity */
666 __media_entity_remove_links(entity);
667
668 /* Remove all pads that belong to this entity */
669 for (i = 0; i < entity->num_pads; i++)
670 media_gobj_destroy(&entity->pads[i].graph_obj);
671
672 /* Remove the entity */
673 media_gobj_destroy(&entity->graph_obj);
674
675 /* invoke entity_notify callbacks to handle entity removal?? */
676
677 entity->graph_obj.mdev = NULL;
678 }
679
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58651 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread