au0828_enable_source() starts the media pipeline and saves the link, its owner and the source and sink entities in struct au0828_dev. The matching au0828_disable_source() is only called from the frontend open and release paths. When the device is disconnected, au0828_dvb_unregister() removes the frontend entity through dvb_unregister_frontend(). dvb_remove_device() frees the entity unconditionally and __media_entity_remove_links() frees every link of it, but the source is not released: the pipeline is never stopped and dev->active_link, dev->active_link_owner, dev->active_source and dev->active_sink keep pointing at freed memory. struct au0828_dev stays alive until the last V4L2 file descriptor is closed, because the video device holds a reference to v4l2_device. A later close therefore reaches au0828_disable_source() through v4l2_fh_exit() and dereferences the freed link and entity. Stop the pipeline and deactivate the link in au0828_usb_disconnect(), before the frontend entity is unregistered and while holding graph_mutex. Move the cleanup tail of au0828_disable_source() to au0828_release_active_link() so that both paths share it; the disconnect path must not be skipped by the dev->users > 1 check, which only applies to a video owner. Nothing has to be released when no link was activated, so the call is guarded by dev->active_link_owner, and the guard is evaluated again under graph_mutex: a close that runs in between can clear the field before the disconnect path acquires the lock. au0828_disable_source() stops the pipeline itself on the way to its deactivate_link label, so the label deactivates the link and clears the saved state only (au0828_deactivate_link()), while the disconnect path calls au0828_release_active_link(), which stops the pipeline first. Calling __media_pipeline_stop() a second time for the same pipeline trips its WARN_ON(!pipe) check. au0828_enable_source() refuses to start a pipeline once DEV_DISCONNECTED is set. Without that check a frontend open that happens while au0828_dvb_unregister() shuts the rest of the device down can reactivate the link after the disconnect path released it, and the frontend entity is then freed with an active pipeline. Fixes: c94903f1f7cc ("[media] media: au0828 add enable, disable source handlers") Cc: stable@vger.kernel.org Co-developed-by: Bin Yu Signed-off-by: Bin Yu Co-developed-by: Cheng Wen Signed-off-by: Cheng Wen Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn> --- Changes in v2: - Re-check dev->active_link_owner under graph_mutex. A close running on another CPU can clear it while the disconnect path waits for the lock, and au0828_release_active_link() would then dereference NULL. - Split the shared cleanup into au0828_deactivate_link(), so the deactivate_link label of au0828_disable_source() does not stop the pipeline a second time. __media_pipeline_stop() warns on an unbalanced call. - Reject au0828_enable_source() after DEV_DISCONNECTED, so a frontend open cannot reactivate the link while the device is being torn down. - The NULL entity that dvb_frontend_release() passes after dvb_media_device_free() is a dvb-core problem, fixed separately by https://lore.kernel.org/r/20261102143415.26942-1-25031212351@stu.xidian.edu.cn --- drivers/media/usb/au0828/au0828-core.c | 64 ++++++++++++++++++++------ 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index 445cbeb7abae..0d4528a26c7d 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -120,6 +120,7 @@ static int recv_control_msg(struct au0828_dev *dev, u16 request, u32 value, #ifdef CONFIG_MEDIA_CONTROLLER static void au0828_media_graph_notify(struct media_entity *new, void *notify_data); +static void au0828_release_active_link(struct au0828_dev *dev); #endif static void au0828_unregister_media_device(struct au0828_dev *dev) @@ -175,6 +176,19 @@ static void au0828_usb_disconnect(struct usb_interface *interface) set_bit(DEV_DISCONNECTED, &dev->dev_state); au0828_rc_unregister(dev); +#ifdef CONFIG_MEDIA_CONTROLLER + /* + * The frontend entity can own the active link. Its entity is released + * by au0828_dvb_unregister() below, so stop the pipeline and deactivate + * the link while the entity is still registered. + */ + if (dev->media_dev) { + mutex_lock(&dev->media_dev->graph_mutex); + if (dev->active_link_owner) + au0828_release_active_link(dev); + mutex_unlock(&dev->media_dev->graph_mutex); + } +#endif /* Digital TV */ au0828_dvb_unregister(dev); @@ -300,6 +314,10 @@ static int au0828_enable_source(struct media_entity *entity, dev = mdev->source_priv; + /* Do not start a new pipeline on a device that is going away */ + if (test_bit(DEV_DISCONNECTED, &dev->dev_state)) + return -ENODEV; + /* * For Audio and V4L2 entity, find the link to which decoder * is the sink. Look for an active link between decoder and @@ -438,6 +456,37 @@ static int au0828_enable_source(struct media_entity *entity, return ret; } +/* Callers should hold graph_mutex */ +static void au0828_deactivate_link(struct au0828_dev *dev) +{ + int ret; + + ret = __media_entity_setup_link(dev->active_link, 0); + if (ret) + pr_err("Deactivate link Error %d\n", ret); + + pr_info("Disabled Source: %s->%s->%s Ret %d\n", + dev->active_source->name, dev->active_sink->name, + dev->active_link_owner->name, ret); + + dev->active_link = NULL; + dev->active_link_owner = NULL; + dev->active_source = NULL; + dev->active_sink = NULL; + dev->active_link_shared = false; + dev->active_link_user = NULL; + dev->active_link_user_pipe = NULL; +} + +/* Callers should hold graph_mutex */ +static void au0828_release_active_link(struct au0828_dev *dev) +{ + __media_pipeline_stop(dev->active_link_owner->pads); + pr_debug("Pipeline stop for %s\n", dev->active_link_owner->name); + + au0828_deactivate_link(dev); +} + /* Callers should hold graph_mutex */ static void au0828_disable_source(struct media_entity *entity) { @@ -537,20 +586,7 @@ static void au0828_disable_source(struct media_entity *entity) dev->active_link_owner->name); deactivate_link: - ret = __media_entity_setup_link(dev->active_link, 0); - if (ret) - pr_err("Deactivate link Error %d\n", ret); - - pr_info("Disabled Source: %s->%s->%s Ret %d\n", - dev->active_source->name, dev->active_sink->name, - dev->active_link_owner->name, ret); - - dev->active_link = NULL; - dev->active_link_owner = NULL; - dev->active_source = NULL; - dev->active_sink = NULL; - dev->active_link_shared = false; - dev->active_link_user = NULL; + au0828_deactivate_link(dev); } } #endif -- 2.34.1