[PATCH v3] staging: most: video: prevent probes during component exit

[PATCH v3] staging: most: video: prevent probes during component exit

[Pavan Kumar Yalagada]

When comp_exit() runs, comp_probe_channel() could still add new devices
to video_devices, creating a race and potentially leaving the list in
an inconsistent state.

This patch prevents new devices from being added while exiting by:

 - comp_exiting is set under lock to prevent new devices being added.
 - Early exit paths free allocated memory (kfree) to avoid leaks.
 - comp_probe_channel() checks comp_exiting before modifying video_devices.
 - Removing WARN/BUG as it becomes unnecessary.

Signed-off-by: Pavan Kumar Yalagada <pavankumaryalagada@gmail.com>

---

v3:
 - comp_exiting flag update and memory cleanup for early exits.
 - Commit message clarified for reviewers.
---
 drivers/staging/most/video/video.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 32f71d9a9cf7..f257fb179813 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -60,6 +60,8 @@ static inline struct comp_fh *to_comp_fh(struct file *filp)
 static LIST_HEAD(video_devices);
 static DEFINE_SPINLOCK(list_lock);

+static bool comp_exiting;
+
 static inline bool data_ready(struct most_video_dev *mdev)
 {
 	return !list_empty(&mdev->pending_mbos);
@@ -498,13 +500,22 @@ static int comp_probe_channel(struct most_interface *iface, int channel_idx,
 		goto err_unreg;

 	spin_lock_irq(&list_lock);
+	if (comp_exiting) {
+		spin_unlock_irq(&list_lock);
+		goto err_cleanup;
+	}
 	list_add(&mdev->list, &video_devices);
 	spin_unlock_irq(&list_lock);
 	return 0;

+err_cleanup:
+	kfree(mdev);
+	return -EBUSY;
+
 err_unreg:
 	v4l2_device_disconnect(&mdev->v4l2_dev);
 	v4l2_device_put(&mdev->v4l2_dev);
+	kfree(mdev);
 	return ret;
 }

@@ -553,8 +564,13 @@ static int __init comp_init(void)

 static void __exit comp_exit(void)
 {
+	unsigned long flags;
 	struct most_video_dev *mdev, *tmp;

+	spin_lock_irqsave(&list_lock, flags);
+	comp_exiting = true;
+	spin_unlock_irqrestore(&list_lock, flags);
+
 	/*
 	 * As the mostcore currently doesn't call disconnect_channel()
 	 * for linked channels while we call most_deregister_component()
@@ -569,13 +585,14 @@ static void __exit comp_exit(void)
 		comp_unregister_videodev(mdev);
 		v4l2_device_disconnect(&mdev->v4l2_dev);
 		v4l2_device_put(&mdev->v4l2_dev);
+		kfree(mdev);
+
 		spin_lock_irq(&list_lock);
 	}
 	spin_unlock_irq(&list_lock);

 	most_deregister_configfs_subsys(&comp);
 	most_deregister_component(&comp);
-	BUG_ON(!list_empty(&video_devices));
 }

 module_init(comp_init);
--
2.47.3

Re: [PATCH v3] staging: most: video: prevent probes during component exit

[Dan Carpenter]

On Fri, Nov 28, 2025 at 11:50:33AM -0500, Pavan Kumar Yalagada wrote:
> When comp_exit() runs, comp_probe_channel() could still add new devices
> to video_devices, creating a race and potentially leaving the list in
> an inconsistent state.
> 
> This patch prevents new devices from being added while exiting by:
> 
>  - comp_exiting is set under lock to prevent new devices being added.
>  - Early exit paths free allocated memory (kfree) to avoid leaks.
>  - comp_probe_channel() checks comp_exiting before modifying video_devices.
>  - Removing WARN/BUG as it becomes unnecessary.
> 
> Signed-off-by: Pavan Kumar Yalagada <pavankumaryalagada@gmail.com>
> 
> ---
> 
> v3:
>  - comp_exiting flag update and memory cleanup for early exits.
>  - Commit message clarified for reviewers.
> ---
>  drivers/staging/most/video/video.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
> index 32f71d9a9cf7..f257fb179813 100644
> --- a/drivers/staging/most/video/video.c
> +++ b/drivers/staging/most/video/video.c
> @@ -60,6 +60,8 @@ static inline struct comp_fh *to_comp_fh(struct file *filp)
>  static LIST_HEAD(video_devices);
>  static DEFINE_SPINLOCK(list_lock);
> 
> +static bool comp_exiting;
> +
>  static inline bool data_ready(struct most_video_dev *mdev)
>  {
>  	return !list_empty(&mdev->pending_mbos);
> @@ -498,13 +500,22 @@ static int comp_probe_channel(struct most_interface *iface, int channel_idx,
>  		goto err_unreg;
> 
>  	spin_lock_irq(&list_lock);
> +	if (comp_exiting) {
> +		spin_unlock_irq(&list_lock);
> +		goto err_cleanup;

Why do you not need to unregister?  I don't think this error handling
is correct.

> +	}
>  	list_add(&mdev->list, &video_devices);
>  	spin_unlock_irq(&list_lock);
>  	return 0;
> 
> +err_cleanup:
> +	kfree(mdev);
> +	return -EBUSY;
> +
>  err_unreg:
>  	v4l2_device_disconnect(&mdev->v4l2_dev);
>  	v4l2_device_put(&mdev->v4l2_dev);
> +	kfree(mdev);
>  	return ret;
>  }
> 
> @@ -553,8 +564,13 @@ static int __init comp_init(void)
> 
>  static void __exit comp_exit(void)
>  {
> +	unsigned long flags;
>  	struct most_video_dev *mdev, *tmp;
> 
> +	spin_lock_irqsave(&list_lock, flags);
> +	comp_exiting = true;
> +	spin_unlock_irqrestore(&list_lock, flags);
> +

It doesn't make sense to call spin_unlock_irqrestore()
and the spin_lock_irq() on the next line.

So the issue is, we want a write barrier so that the write in comp_exit()
happens before the read in comp_probe_channel(), which the spinlocks
do accomplish, I suppose.

>  	/*
>  	 * As the mostcore currently doesn't call disconnect_channel()
>  	 * for linked channels while we call most_deregister_component()
> @@ -569,13 +585,14 @@ static void __exit comp_exit(void)
>  		comp_unregister_videodev(mdev);
>  		v4l2_device_disconnect(&mdev->v4l2_dev);
>  		v4l2_device_put(&mdev->v4l2_dev);
> +		kfree(mdev);

No no.  This isn't related to race conditions and it's probably
wrong as well.

regards,
dan carpenter