Commit d7e43844 authored by Matthias Dahl's avatar Matthias Dahl Committed by Mauro Carvalho Chehab

V4L/DVB (9054): implement proper locking in the dvb ca en50221 driver

Concurrent access to a single DVB CA 50221 interface slot is generally
discouraged. The underlying drivers (budget-av, budget-ci) do not implement
proper locking and thus two transactions could (and do) interfere with on
another.

This fixes the following problems seen by others and myself:

 - sudden i/o errors when writing to the ci device which usually would
   result in an undefined state of the hw and require a software restart

 - errors about the CAM trying to send a buffer larger than the agreed size
   usually also resulting in an undefined state of the hw

Due the to design of the DVB CA 50221 driver, implementing the locks in the
underlying drivers would not be enough and still leave some race conditions,
even though they were harder to trigger.
Signed-off-by: default avatarMatthias Dahl <devel@mortal-soul.de>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent 0c37dd7a
...@@ -93,6 +93,9 @@ struct dvb_ca_slot { ...@@ -93,6 +93,9 @@ struct dvb_ca_slot {
/* current state of the CAM */ /* current state of the CAM */
int slot_state; int slot_state;
/* mutex used for serializing access to one CI slot */
struct mutex slot_lock;
/* Number of CAMCHANGES that have occurred since last processing */ /* Number of CAMCHANGES that have occurred since last processing */
atomic_t camchange_count; atomic_t camchange_count;
...@@ -711,14 +714,20 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot, u8 * b ...@@ -711,14 +714,20 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot, u8 * b
dprintk("%s\n", __func__); dprintk("%s\n", __func__);
// sanity check /* sanity check */
if (bytes_write > ca->slot_info[slot].link_buf_size) if (bytes_write > ca->slot_info[slot].link_buf_size)
return -EINVAL; return -EINVAL;
/* check if interface is actually waiting for us to read from it, or if a read is in progress */ /* it is possible we are dealing with a single buffer implementation,
thus if there is data available for read or if there is even a read
already in progress, we do nothing but awake the kernel thread to
process the data if necessary. */
if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) < 0) if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) < 0)
goto exitnowrite; goto exitnowrite;
if (status & (STATUSREG_DA | STATUSREG_RE)) { if (status & (STATUSREG_DA | STATUSREG_RE)) {
if (status & STATUSREG_DA)
dvb_ca_en50221_thread_wakeup(ca);
status = -EAGAIN; status = -EAGAIN;
goto exitnowrite; goto exitnowrite;
} }
...@@ -987,6 +996,8 @@ static int dvb_ca_en50221_thread(void *data) ...@@ -987,6 +996,8 @@ static int dvb_ca_en50221_thread(void *data)
/* go through all the slots processing them */ /* go through all the slots processing them */
for (slot = 0; slot < ca->slot_count; slot++) { for (slot = 0; slot < ca->slot_count; slot++) {
mutex_lock(&ca->slot_info[slot].slot_lock);
// check the cam status + deal with CAMCHANGEs // check the cam status + deal with CAMCHANGEs
while (dvb_ca_en50221_check_camstatus(ca, slot)) { while (dvb_ca_en50221_check_camstatus(ca, slot)) {
/* clear down an old CI slot if necessary */ /* clear down an old CI slot if necessary */
...@@ -1122,7 +1133,7 @@ static int dvb_ca_en50221_thread(void *data) ...@@ -1122,7 +1133,7 @@ static int dvb_ca_en50221_thread(void *data)
case DVB_CA_SLOTSTATE_RUNNING: case DVB_CA_SLOTSTATE_RUNNING:
if (!ca->open) if (!ca->open)
continue; break;
// poll slots for data // poll slots for data
pktcount = 0; pktcount = 0;
...@@ -1146,6 +1157,8 @@ static int dvb_ca_en50221_thread(void *data) ...@@ -1146,6 +1157,8 @@ static int dvb_ca_en50221_thread(void *data)
} }
break; break;
} }
mutex_unlock(&ca->slot_info[slot].slot_lock);
} }
} }
...@@ -1181,6 +1194,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct inode *inode, struct file *file, ...@@ -1181,6 +1194,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct inode *inode, struct file *file,
switch (cmd) { switch (cmd) {
case CA_RESET: case CA_RESET:
for (slot = 0; slot < ca->slot_count; slot++) { for (slot = 0; slot < ca->slot_count; slot++) {
mutex_lock(&ca->slot_info[slot].slot_lock);
if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_NONE) { if (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_NONE) {
dvb_ca_en50221_slot_shutdown(ca, slot); dvb_ca_en50221_slot_shutdown(ca, slot);
if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE) if (ca->flags & DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE)
...@@ -1188,6 +1202,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct inode *inode, struct file *file, ...@@ -1188,6 +1202,7 @@ static int dvb_ca_en50221_io_do_ioctl(struct inode *inode, struct file *file,
slot, slot,
DVB_CA_EN50221_CAMCHANGE_INSERTED); DVB_CA_EN50221_CAMCHANGE_INSERTED);
} }
mutex_unlock(&ca->slot_info[slot].slot_lock);
} }
ca->next_read_slot = 0; ca->next_read_slot = 0;
dvb_ca_en50221_thread_wakeup(ca); dvb_ca_en50221_thread_wakeup(ca);
...@@ -1308,7 +1323,9 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file, ...@@ -1308,7 +1323,9 @@ static ssize_t dvb_ca_en50221_io_write(struct file *file,
goto exit; goto exit;
} }
mutex_lock(&ca->slot_info[slot].slot_lock);
status = dvb_ca_en50221_write_data(ca, slot, fragbuf, fraglen + 2); status = dvb_ca_en50221_write_data(ca, slot, fragbuf, fraglen + 2);
mutex_unlock(&ca->slot_info[slot].slot_lock);
if (status == (fraglen + 2)) { if (status == (fraglen + 2)) {
written = 1; written = 1;
break; break;
...@@ -1664,6 +1681,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter, ...@@ -1664,6 +1681,7 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
ca->slot_info[i].slot_state = DVB_CA_SLOTSTATE_NONE; ca->slot_info[i].slot_state = DVB_CA_SLOTSTATE_NONE;
atomic_set(&ca->slot_info[i].camchange_count, 0); atomic_set(&ca->slot_info[i].camchange_count, 0);
ca->slot_info[i].camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED; ca->slot_info[i].camchange_type = DVB_CA_EN50221_CAMCHANGE_REMOVED;
mutex_init(&ca->slot_info[i].slot_lock);
} }
if (signal_pending(current)) { if (signal_pending(current)) {
......
...@@ -45,8 +45,10 @@ struct dvb_ca_en50221 { ...@@ -45,8 +45,10 @@ struct dvb_ca_en50221 {
/* the module owning this structure */ /* the module owning this structure */
struct module* owner; struct module* owner;
/* NOTE: the read_*, write_* and poll_slot_status functions must use locks as /* NOTE: the read_*, write_* and poll_slot_status functions will be
* they may be called from several threads at once */ * called for different slots concurrently and need to use locks where
* and if appropriate. There will be no concurrent access to one slot.
*/
/* functions for accessing attribute memory on the CAM */ /* functions for accessing attribute memory on the CAM */
int (*read_attribute_mem)(struct dvb_ca_en50221* ca, int slot, int address); int (*read_attribute_mem)(struct dvb_ca_en50221* ca, int slot, int address);
......
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