As pointed by Dan, the patch f8fd4c61b5ae: "[media] media-entity:
protect object creation/removal using spin lock" leads to the following
static checker warning:
drivers/media/media-entity.c:781 media_remove_intf_link()
error: dereferencing freed memory 'link'
drivers/media/media-entity.c
777 void media_remove_intf_link(struct media_link *link)
778 {
779 spin_lock(&link->graph_obj.mdev->lock);
780 __media_remove_intf_link(link);
781 spin_unlock(&link->graph_obj.mdev->lock);
In practice, I didn't see any troubles even with KASAN enabled.
I guess gcc optimizer internally cached the mdev reference,
instead of getting it twice. Yet, it is a very bad idea to rely
on such optimization. So, let's fix the code.
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/media-entity.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 29810f9b86ce..6c3f4041797c 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -580,13 +580,15 @@ EXPORT_SYMBOL_GPL(__media_entity_remove_links);
void media_entity_remove_links(struct media_entity *entity)
{
+ struct media_device *mdev = entity->graph_obj.mdev;
+
/* Do nothing if the entity is not registered. */
- if (entity->graph_obj.mdev == NULL)
+ if (mdev == NULL)
return;
- spin_lock(&entity->graph_obj.mdev->lock);
+ spin_lock(&mdev->lock);
__media_entity_remove_links(entity);
- spin_unlock(&entity->graph_obj.mdev->lock);
+ spin_unlock(&mdev->lock);
}
EXPORT_SYMBOL_GPL(media_entity_remove_links);
@@ -781,9 +783,15 @@ EXPORT_SYMBOL_GPL(__media_remove_intf_link);
void media_remove_intf_link(struct media_link *link)
{
- spin_lock(&link->graph_obj.mdev->lock);
+ struct media_device *mdev = link->graph_obj.mdev;
+
+ /* Do nothing if the intf is not registered. */
+ if (mdev == NULL)
+ return;
+
+ spin_lock(&mdev->lock);
__media_remove_intf_link(link);
- spin_unlock(&link->graph_obj.mdev->lock);
+ spin_unlock(&mdev->lock);
}
EXPORT_SYMBOL_GPL(media_remove_intf_link);
@@ -799,12 +807,14 @@ EXPORT_SYMBOL_GPL(__media_remove_intf_links);
void media_remove_intf_links(struct media_interface *intf)
{
+ struct media_device *mdev = intf->graph_obj.mdev;
+
/* Do nothing if the intf is not registered. */
- if (intf->graph_obj.mdev == NULL)
+ if (mdev == NULL)
return;
- spin_lock(&intf->graph_obj.mdev->lock);
+ spin_lock(&mdev->lock);
__media_remove_intf_links(intf);
- spin_unlock(&intf->graph_obj.mdev->lock);
+ spin_unlock(&mdev->lock);
}
EXPORT_SYMBOL_GPL(media_remove_intf_links);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
