Commit 2ab25d35 authored by Hans Verkuil's avatar Hans Verkuil Committed by Mauro Carvalho Chehab

[media] cec: improve locking

- The global lock was used in cec_get_device when it should have
  used the devnode lock.
- cec_put_device also took the global lock, but since the release
  function takes that lock as well this could lead to a deadlock.
  Just don't take the lock here since there is no reason for it.
- cec_devnode_register() should take the global lock when clearing
  the bit in the global bitmap.
- In cec_devnode_unregister() place the devnode->(un)register tests
  and assignments under the devnode lock as well: this has to be
  in a critical block.
Signed-off-by: default avatarHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@s-opensource.com>
parent 62148f09
...@@ -51,31 +51,29 @@ int cec_get_device(struct cec_devnode *devnode) ...@@ -51,31 +51,29 @@ int cec_get_device(struct cec_devnode *devnode)
{ {
/* /*
* Check if the cec device is available. This needs to be done with * Check if the cec device is available. This needs to be done with
* the cec_devnode_lock held to prevent an open/unregister race: * the devnode->lock held to prevent an open/unregister race:
* without the lock, the device could be unregistered and freed between * without the lock, the device could be unregistered and freed between
* the devnode->registered check and get_device() calls, leading to * the devnode->registered check and get_device() calls, leading to
* a crash. * a crash.
*/ */
mutex_lock(&cec_devnode_lock); mutex_lock(&devnode->lock);
/* /*
* return ENXIO if the cec device has been removed * return ENXIO if the cec device has been removed
* already or if it is not registered anymore. * already or if it is not registered anymore.
*/ */
if (!devnode->registered) { if (!devnode->registered) {
mutex_unlock(&cec_devnode_lock); mutex_unlock(&devnode->lock);
return -ENXIO; return -ENXIO;
} }
/* and increase the device refcount */ /* and increase the device refcount */
get_device(&devnode->dev); get_device(&devnode->dev);
mutex_unlock(&cec_devnode_lock); mutex_unlock(&devnode->lock);
return 0; return 0;
} }
void cec_put_device(struct cec_devnode *devnode) void cec_put_device(struct cec_devnode *devnode)
{ {
mutex_lock(&cec_devnode_lock);
put_device(&devnode->dev); put_device(&devnode->dev);
mutex_unlock(&cec_devnode_lock);
} }
/* Called when the last user of the cec device exits. */ /* Called when the last user of the cec device exits. */
...@@ -84,11 +82,10 @@ static void cec_devnode_release(struct device *cd) ...@@ -84,11 +82,10 @@ static void cec_devnode_release(struct device *cd)
struct cec_devnode *devnode = to_cec_devnode(cd); struct cec_devnode *devnode = to_cec_devnode(cd);
mutex_lock(&cec_devnode_lock); mutex_lock(&cec_devnode_lock);
/* Mark device node number as free */ /* Mark device node number as free */
clear_bit(devnode->minor, cec_devnode_nums); clear_bit(devnode->minor, cec_devnode_nums);
mutex_unlock(&cec_devnode_lock); mutex_unlock(&cec_devnode_lock);
cec_delete_adapter(to_cec_adapter(devnode)); cec_delete_adapter(to_cec_adapter(devnode));
} }
...@@ -160,7 +157,9 @@ static int __must_check cec_devnode_register(struct cec_devnode *devnode, ...@@ -160,7 +157,9 @@ static int __must_check cec_devnode_register(struct cec_devnode *devnode,
cdev_del: cdev_del:
cdev_del(&devnode->cdev); cdev_del(&devnode->cdev);
clr_bit: clr_bit:
mutex_lock(&cec_devnode_lock);
clear_bit(devnode->minor, cec_devnode_nums); clear_bit(devnode->minor, cec_devnode_nums);
mutex_unlock(&cec_devnode_lock);
return ret; return ret;
} }
...@@ -177,17 +176,21 @@ static void cec_devnode_unregister(struct cec_devnode *devnode) ...@@ -177,17 +176,21 @@ static void cec_devnode_unregister(struct cec_devnode *devnode)
{ {
struct cec_fh *fh; struct cec_fh *fh;
mutex_lock(&devnode->lock);
/* Check if devnode was never registered or already unregistered */ /* Check if devnode was never registered or already unregistered */
if (!devnode->registered || devnode->unregistered) if (!devnode->registered || devnode->unregistered) {
mutex_unlock(&devnode->lock);
return; return;
}
mutex_lock(&devnode->lock);
list_for_each_entry(fh, &devnode->fhs, list) list_for_each_entry(fh, &devnode->fhs, list)
wake_up_interruptible(&fh->wait); wake_up_interruptible(&fh->wait);
mutex_unlock(&devnode->lock);
devnode->registered = false; devnode->registered = false;
devnode->unregistered = true; devnode->unregistered = true;
mutex_unlock(&devnode->lock);
device_del(&devnode->dev); device_del(&devnode->dev);
cdev_del(&devnode->cdev); cdev_del(&devnode->cdev);
put_device(&devnode->dev); put_device(&devnode->dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment