@@ -1249,13 +1249,17 @@ static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev,
static void v4l2_ctrl_del_event(struct v4l2_subscribed_event *sev)
{
- struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id);
+ struct v4l2_ctrl *ctrl;
+ if (list_empty(&sev->node))
+ return;
+
+ ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id);
if (!ctrl)
return;
v4l2_ctrl_lock(ctrl);
- list_del(&sev->node);
+ list_del_init(&sev->node);
v4l2_ctrl_unlock(ctrl);
}
@@ -10,6 +10,7 @@
#include <linux/slab.h>
#include <media/v4l2-ctrls.h>
#include <media/v4l2-event.h>
+#include <media/v4l2-fh.h>
#include <media/v4l2-fwnode.h>
#include "v4l2-ctrls-priv.h"
@@ -1569,13 +1570,20 @@ void v4l2_ctrl_handler_free(struct v4l2_ctrl_handler *hdl)
list_del(&ref->node);
if (ref->p_req_array_alloc_elems)
kvfree(ref->p_req.p);
+ if (ref->ctrl->handler != hdl) {
+ mutex_lock(ref->ctrl->handler->lock);
+ list_for_each_entry_safe(sev, next_sev, &ref->ctrl->ev_subs, node)
+ if (sev->fh->ctrl_handler == hdl)
+ list_del_init(&sev->node);
+ mutex_unlock(ref->ctrl->handler->lock);
+ }
kfree(ref);
}
/* Free all controls owned by the handler */
list_for_each_entry_safe(ctrl, next_ctrl, &hdl->ctrls, node) {
list_del(&ctrl->node);
list_for_each_entry_safe(sev, next_sev, &ctrl->ev_subs, node)
- list_del(&sev->node);
+ list_del_init(&sev->node);
kvfree(ctrl->p_array);
kvfree(ctrl);
}
@@ -246,6 +246,7 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
sev->flags = sub->flags;
sev->fh = fh;
sev->ops = ops;
+ INIT_LIST_HEAD(&sev->node);
mutex_lock(&fh->subscribe_lock);
The v4l2_ctrl_handler_free() function must unlink all subscribed events of the control handler that is being freed, but it only did so for the controls owned by that control handler and not for the controls referred to by that control handler. This leaves stale data in the ev_subs list of those controls. The node list header is also properly initialized and list_del_init is called instead of list_del to ensure that list_empty() can be used to detect whether a v4l2_subscribed_event is part of a list or not. This makes v4l2_ctrl_del_event() more robust since it will not attempt to find the control if the v4l2_subscribed_event has already been unlinked from the control. Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl> Cc: stable@vger.kernel.org # 6.7.x --- This is v2 of a very old patch from 2022: https://patchwork.linuxtv.org/project/linux-media/patch/66246ea5-2bd7-6c9e-56c8-9d683ec58ffc@xs4all.nl/ It looks like I forgot to follow-up on Laurent's comments. Recently we had to upgrade to a newer kernel and apply this same patch again since it was still failing, at which point I realized that this fix wasn't in mainline. This issue only happens if you have a larger pipeline consisting of several subdevs, and one of the subdevs is forcibly unbound. Changes since v1: - fixed Laurent's comments - add locking --- drivers/media/v4l2-core/v4l2-ctrls-api.c | 8 ++++++-- drivers/media/v4l2-core/v4l2-ctrls-core.c | 10 +++++++++- drivers/media/v4l2-core/v4l2-event.c | 1 + 3 files changed, 16 insertions(+), 3 deletions(-)