kpriv/linux-hardened
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

staging: pi433: cleanup tx_fifo locking

Browse source 
pi433_write requires locking due to multiple writers.  After acquiring
the lock check if enough free space is available in the kfifo to write
the whole message. This check should prevent partial writes to tx_fifo
so kfifo_reset is not needed anymore.

pi433_tx_thread is the only reader so it does not require locking
after kfifo_reset is removed.

Signed-off-by: Valentin Vidic <Valentin.Vidic@CARNet.hr>
Reviewed-by: Marcus Wolf <marcus.wolf@wolf-entwicklungen.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Valentin Vidic 2018-04-19 12:25:30 +02:00 committed by Greg Kroah-Hartman
parent 1dab154ef1
commit 5451dab9b7
1 changed files with 14 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
drivers/staging/pi433/pi433_if.c
View file

@ -87,7 +87,7 @@ struct pi433_device {
/* tx related values */ /* tx related values */
STRUCT_KFIFO_REC_1(MSG_FIFO_SIZE) tx_fifo; STRUCT_KFIFO_REC_1(MSG_FIFO_SIZE) tx_fifo;
struct mutex tx_fifo_lock; // TODO: check, whether necessary or obsolete struct mutex tx_fifo_lock; /* serialize userspace writers */
struct task_struct *tx_task_struct; struct task_struct *tx_task_struct;
wait_queue_head_t tx_wait_queue; wait_queue_head_t tx_wait_queue;
u8 free_in_fifo; u8 free_in_fifo;
@ -589,19 +589,15 @@ pi433_tx_thread(void *data)
* - size of message * - size of message
* - message * - message
*/ */
mutex_lock(&device->tx_fifo_lock);
retval = kfifo_out(&device->tx_fifo, &tx_cfg, sizeof(tx_cfg)); retval = kfifo_out(&device->tx_fifo, &tx_cfg, sizeof(tx_cfg));
if (retval != sizeof(tx_cfg)) { if (retval != sizeof(tx_cfg)) {
dev_dbg(device->dev, "reading tx_cfg from fifo failed: got %d byte(s), expected %d", retval, (unsigned int)sizeof(tx_cfg)); dev_dbg(device->dev, "reading tx_cfg from fifo failed: got %d byte(s), expected %d", retval, (unsigned int)sizeof(tx_cfg));
mutex_unlock(&device->tx_fifo_lock);
continue; continue;
} }
retval = kfifo_out(&device->tx_fifo, &size, sizeof(size_t)); retval = kfifo_out(&device->tx_fifo, &size, sizeof(size_t));
if (retval != sizeof(size_t)) { if (retval != sizeof(size_t)) {
dev_dbg(device->dev, "reading msg size from fifo failed: got %d, expected %d", retval, (unsigned int)sizeof(size_t)); dev_dbg(device->dev, "reading msg size from fifo failed: got %d, expected %d", retval, (unsigned int)sizeof(size_t));
mutex_unlock(&device->tx_fifo_lock);
continue; continue;
} }
@ -634,7 +630,6 @@ pi433_tx_thread(void *data)
sizeof(device->buffer) - position); sizeof(device->buffer) - position);
dev_dbg(device->dev, dev_dbg(device->dev,
"read %d message byte(s) from fifo queue.", retval); "read %d message byte(s) from fifo queue.", retval);
mutex_unlock(&device->tx_fifo_lock);
/* if rx is active, we need to interrupt the waiting for /* if rx is active, we need to interrupt the waiting for
* incoming telegrams, to be able to send something. * incoming telegrams, to be able to send something.
@ -818,7 +813,7 @@ pi433_write(struct file *filp, const char __user *buf,
struct pi433_instance *instance; struct pi433_instance *instance;
struct pi433_device *device; struct pi433_device *device;
int retval; int retval;
unsigned int copied; unsigned int required, available, copied;
instance = filp->private_data; instance = filp->private_data;
device = instance->device; device = instance->device;
@ -833,6 +828,16 @@ pi433_write(struct file *filp, const char __user *buf,
* - message * - message
*/ */
mutex_lock(&device->tx_fifo_lock); mutex_lock(&device->tx_fifo_lock);
required = sizeof(instance->tx_cfg) + sizeof(size_t) + count;
available = kfifo_avail(&device->tx_fifo);
if (required > available) {
dev_dbg(device->dev, "write to fifo failed: %d bytes required but %d available",
required, available);
mutex_unlock(&device->tx_fifo_lock);
return -EAGAIN;
}
retval = kfifo_in(&device->tx_fifo, &instance->tx_cfg, retval = kfifo_in(&device->tx_fifo, &instance->tx_cfg,
sizeof(instance->tx_cfg)); sizeof(instance->tx_cfg));
if (retval != sizeof(instance->tx_cfg)) if (retval != sizeof(instance->tx_cfg))
@ -855,8 +860,8 @@ pi433_write(struct file *filp, const char __user *buf,
return copied; return copied;
abort: abort:
dev_dbg(device->dev, "write to fifo failed: 0x%x", retval); dev_warn(device->dev,
kfifo_reset(&device->tx_fifo); // TODO: maybe find a solution, not to discard already stored, valid entries "write to fifo failed, non recoverable: 0x%x", retval);
mutex_unlock(&device->tx_fifo_lock); mutex_unlock(&device->tx_fifo_lock);
return -EAGAIN; return -EAGAIN;
} }