From f0fc97cfc4292b4e5d5998805c17707b73b05289 Mon Sep 17 00:00:00 2001 From: Shaoen-Lin Date: Mon, 1 Jun 2026 17:45:12 +0800 Subject: [PATCH] Fix device removal locking control_iocontrol_destroy_device() reads a vcam_devices entry before taking vcam_devices_lock, but later removes entries from the same array while holding the lock. Concurrent destroy requests can therefore use a stale index or device pointer after another request shifts the array. Take vcam_devices_lock before validating the index and reading the device pointer. Remove the device from vcam_devices while still holding the lock, then release the lock before destroying the device. The removal loop shifts vcam_devices entries left by reading vcam_devices[i + 1]. The old loop allowed i to reach vcam_device_count - 1, so the last iteration read one entry past the valid array range. Stop the loop before i + 1 reaches vcam_device_count. Signed-off-by: Shaoen-Lin --- control.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/control.c b/control.c index 5d3ba3d..3102168 100644 --- a/control.c +++ b/control.c @@ -102,27 +102,31 @@ static int control_iocontrol_modify_input_setting( static int control_iocontrol_destroy_device(struct vcam_device_spec *dev_spec) { struct vcam_device *dev; - unsigned long flags = 0; + unsigned long ctldev_flags = 0; + unsigned long dev_flags = 0; int i; - if (ctldev->vcam_device_count <= dev_spec->idx) + spin_lock_irqsave(&ctldev->vcam_devices_lock, ctldev_flags); + if (ctldev->vcam_device_count <= dev_spec->idx) { + spin_unlock_irqrestore(&ctldev->vcam_devices_lock, ctldev_flags); return -EINVAL; + } dev = ctldev->vcam_devices[dev_spec->idx]; - spin_lock_irqsave(&dev->in_fh_slock, flags); + spin_lock_irqsave(&dev->in_fh_slock, dev_flags); if (dev->fb_isopen || vb2_is_busy(&dev->vb_out_vidq)) { - spin_unlock_irqrestore(&dev->in_fh_slock, flags); + spin_unlock_irqrestore(&dev->in_fh_slock, dev_flags); + spin_unlock_irqrestore(&ctldev->vcam_devices_lock, ctldev_flags); return -EBUSY; } dev->fb_isopen = true; - spin_unlock_irqrestore(&dev->in_fh_slock, flags); + spin_unlock_irqrestore(&dev->in_fh_slock, dev_flags); - spin_lock_irqsave(&ctldev->vcam_devices_lock, flags); - for (i = dev_spec->idx; i < (ctldev->vcam_device_count); i++) + for (i = dev_spec->idx; i + 1 < (ctldev->vcam_device_count); i++) ctldev->vcam_devices[i] = ctldev->vcam_devices[i + 1]; ctldev->vcam_devices[--ctldev->vcam_device_count] = NULL; - spin_unlock_irqrestore(&ctldev->vcam_devices_lock, flags); + spin_unlock_irqrestore(&ctldev->vcam_devices_lock, ctldev_flags); destroy_vcam_device(dev);