Em Fri, 11 Sep 2015 17:20:47 +0200
Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
> On 09/06/2015 07:30 PM, Mauro Carvalho Chehab wrote:
> > Right now, if something gets wrong at dvb_create_media_entity()
> > or at dvb_create_media_graph(), the device will still be
> > registered.
> >
> > Change the logic to properly handle it and free all media graph
> > objects if something goes wrong at dvb_register_device().
> >
> > Also, change the logic at dvb_create_media_graph() to return
> > an error code if something goes wrong. It is up to the
> > caller to implement the right logic and to call
> > dvb_unregister_device() to unregister the already-created
> > objects.
> >
> > While here, add a missing logic to unregister the created
> > interfaces.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
>
> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
I did a lot of tests today with KASAN and I found some issues on
this patch. So, I'm folding it with the enclosed changes:
commit e919f3fc27a329814c064b4c3f110a4a4e80284c
Author: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
Date: Thu Dec 10 10:48:00 2015 -0200
fixup
Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 5c51084a331a..bc650c637fc0 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -187,21 +187,32 @@ static void dvb_media_device_free(struct dvb_device *dvbdev)
{
#if defined(CONFIG_MEDIA_CONTROLLER_DVB)
if (dvbdev->entity) {
+ media_device_unregister_entity(dvbdev->entity);
+ kfree(dvbdev->entity);
+ kfree(dvbdev->pads);
+ dvbdev->entity = NULL;
+ dvbdev->pads = NULL;
+ }
+
+ if (dvbdev->tsout_entity) {
int i;
for (i = 0; i < dvbdev->tsout_num_entities; i++) {
media_device_unregister_entity(&dvbdev->tsout_entity[i]);
kfree(dvbdev->tsout_entity[i].name);
}
- media_device_unregister_entity(dvbdev->entity);
-
- kfree(dvbdev->entity);
- kfree(dvbdev->pads);
kfree(dvbdev->tsout_entity);
kfree(dvbdev->tsout_pads);
+ dvbdev->tsout_entity = NULL;
+ dvbdev->tsout_pads = NULL;
+
+ dvbdev->tsout_num_entities = 0;
}
- if (dvbdev->intf_devnode)
+
+ if (dvbdev->intf_devnode) {
media_devnode_remove(dvbdev->intf_devnode);
+ dvbdev->intf_devnode = NULL;
+ }
#endif
}
@@ -221,13 +232,15 @@ static int dvb_create_tsout_entity(struct dvb_device *dvbdev,
if (!dvbdev->tsout_entity)
return -ENOMEM;
+ dvbdev->tsout_num_entities = npads;
+
for (i = 0; i < npads; i++) {
struct media_pad *pads = &dvbdev->tsout_pads[i];
struct media_entity *entity = &dvbdev->tsout_entity[i];
entity->name = kasprintf(GFP_KERNEL, "%s #%d", name, i);
if (!entity->name)
- return ret;
+ return -ENOMEM;
entity->type = MEDIA_ENT_T_DVB_TSOUT;
pads->flags = MEDIA_PAD_FL_SINK;
@@ -318,9 +331,11 @@ static int dvb_create_media_entity(struct dvb_device *dvbdev,
dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
break;
default:
+ /* Should never happen, as the first switch prevents it */
kfree(dvbdev->entity);
kfree(dvbdev->pads);
dvbdev->entity = NULL;
+ dvbdev->pads = NULL;
return 0;
}
@@ -423,24 +438,13 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
return -ENFILE;
}
- *pdvbdev = dvbdev = kmalloc(sizeof(struct dvb_device), GFP_KERNEL);
+ *pdvbdev = dvbdev = kzalloc(sizeof(*dvbdev), GFP_KERNEL);
if (!dvbdev){
mutex_unlock(&dvbdev_register_lock);
return -ENOMEM;
}
- ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads);
- if (ret) {
- printk(KERN_ERR
- "%s: dvb_register_media_device failed to create the mediagraph\n",
- __func__);
-
- dvb_media_device_free(dvbdev);
- mutex_unlock(&dvbdev_register_lock);
- return ret;
- }
-
dvbdevfops = kzalloc(sizeof(struct file_operations), GFP_KERNEL);
if (!dvbdevfops){
@@ -483,6 +487,20 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
dvb_minors[minor] = dvbdev;
up_write(&minor_rwsem);
+ ret = dvb_register_media_device(dvbdev, type, minor, demux_sink_pads);
+ if (ret) {
+ printk(KERN_ERR
+ "%s: dvb_register_media_device failed to create the mediagraph\n",
+ __func__);
+
+ dvb_media_device_free(dvbdev);
+ kfree(dvbdevfops);
+ kfree(dvbdev);
+ up_write(&minor_rwsem);
+ mutex_unlock(&dvbdev_register_lock);
+ return ret;
+ }
+
mutex_unlock(&dvbdev_register_lock);
clsdev = device_create(dvb_class, adap->device,
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 96700843b1e4..c7d97190a67e 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -576,6 +576,10 @@ void media_device_unregister(struct media_device *mdev)
struct media_entity *next;
struct media_interface *intf, *tmp_intf;
+ /* Remove all entities from the media device */
+ list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
+ media_device_unregister_entity(entity);
+
/* Remove all interfaces from the media device */
spin_lock(&mdev->lock);
list_for_each_entry_safe(intf, tmp_intf, &mdev->interfaces,
@@ -586,9 +590,6 @@ void media_device_unregister(struct media_device *mdev)
}
spin_unlock(&mdev->lock);
- list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
- media_device_unregister_entity(entity);
-
device_remove_file(&mdev->devnode.dev, &dev_attr_model);
media_devnode_unregister(&mdev->devnode);
@@ -654,8 +655,7 @@ void media_device_unregister_entity(struct media_entity *entity)
/* 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 (media_type(link->gobj1) == MEDIA_GRAPH_ENTITY
- && link->entity == entity)
+ if (link->entity == entity)
__media_remove_intf_link(link);
}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
