Skip to content

Commit f53641a

Browse files
ian-abbottgregkh
authored andcommitted
comedi: comedi_test: Prevent timers rescheduling during deletion
The comedi_test devices have a couple of timers (ai_timer and ao_timer) that can be started to simulate hardware interrupts. Their expiry functions normally reschedule the timer. The driver code calls either del_timer_sync() or del_timer() to delete the timers from the queue, but does not currently prevent the timers from rescheduling themselves so synchronized deletion may be ineffective. Add a couple of boolean members (one for each timer: ai_timer_enable and ao_timer_enable) to the device private data structure to indicate whether the timers are allowed to reschedule themselves. Set the member to true when adding the timer to the queue, and to false when deleting the timer from the queue in the waveform_ai_cancel() and waveform_ao_cancel() functions. The del_timer_sync() function is also called from the waveform_detach() function, but the timer enable members will already be set to false when that function is called, so no change is needed there. Fixes: 403fe7f ("staging: comedi: comedi_test: fix timer race conditions") Cc: stable@vger.kernel.org # 4.4+ Signed-off-by: Ian Abbott <abbotti@mev.co.uk> Link: https://lore.kernel.org/r/20240214100747.16203-1-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent cfa9ba1 commit f53641a

File tree

1 file changed

+26
-4
lines changed

1 file changed

+26
-4
lines changed

drivers/comedi/drivers/comedi_test.c

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ struct waveform_private {
8787
struct comedi_device *dev; /* parent comedi device */
8888
u64 ao_last_scan_time; /* time of previous AO scan in usec */
8989
unsigned int ao_scan_period; /* AO scan period in usec */
90+
bool ai_timer_enable:1; /* should AI timer be running? */
91+
bool ao_timer_enable:1; /* should AO timer be running? */
9092
unsigned short ao_loopbacks[N_CHANS];
9193
};
9294

@@ -236,8 +238,12 @@ static void waveform_ai_timer(struct timer_list *t)
236238
time_increment = devpriv->ai_convert_time - now;
237239
else
238240
time_increment = 1;
239-
mod_timer(&devpriv->ai_timer,
240-
jiffies + usecs_to_jiffies(time_increment));
241+
spin_lock(&dev->spinlock);
242+
if (devpriv->ai_timer_enable) {
243+
mod_timer(&devpriv->ai_timer,
244+
jiffies + usecs_to_jiffies(time_increment));
245+
}
246+
spin_unlock(&dev->spinlock);
241247
}
242248

243249
overrun:
@@ -393,9 +399,12 @@ static int waveform_ai_cmd(struct comedi_device *dev,
393399
* Seem to need an extra jiffy here, otherwise timer expires slightly
394400
* early!
395401
*/
402+
spin_lock_bh(&dev->spinlock);
403+
devpriv->ai_timer_enable = true;
396404
devpriv->ai_timer.expires =
397405
jiffies + usecs_to_jiffies(devpriv->ai_convert_period) + 1;
398406
add_timer(&devpriv->ai_timer);
407+
spin_unlock_bh(&dev->spinlock);
399408
return 0;
400409
}
401410

@@ -404,6 +413,9 @@ static int waveform_ai_cancel(struct comedi_device *dev,
404413
{
405414
struct waveform_private *devpriv = dev->private;
406415

416+
spin_lock_bh(&dev->spinlock);
417+
devpriv->ai_timer_enable = false;
418+
spin_unlock_bh(&dev->spinlock);
407419
if (in_softirq()) {
408420
/* Assume we were called from the timer routine itself. */
409421
del_timer(&devpriv->ai_timer);
@@ -495,8 +507,12 @@ static void waveform_ao_timer(struct timer_list *t)
495507
unsigned int time_inc = devpriv->ao_last_scan_time +
496508
devpriv->ao_scan_period - now;
497509

498-
mod_timer(&devpriv->ao_timer,
499-
jiffies + usecs_to_jiffies(time_inc));
510+
spin_lock(&dev->spinlock);
511+
if (devpriv->ao_timer_enable) {
512+
mod_timer(&devpriv->ao_timer,
513+
jiffies + usecs_to_jiffies(time_inc));
514+
}
515+
spin_unlock(&dev->spinlock);
500516
}
501517

502518
underrun:
@@ -517,9 +533,12 @@ static int waveform_ao_inttrig_start(struct comedi_device *dev,
517533
async->inttrig = NULL;
518534

519535
devpriv->ao_last_scan_time = ktime_to_us(ktime_get());
536+
spin_lock_bh(&dev->spinlock);
537+
devpriv->ao_timer_enable = true;
520538
devpriv->ao_timer.expires =
521539
jiffies + usecs_to_jiffies(devpriv->ao_scan_period);
522540
add_timer(&devpriv->ao_timer);
541+
spin_unlock_bh(&dev->spinlock);
523542

524543
return 1;
525544
}
@@ -604,6 +623,9 @@ static int waveform_ao_cancel(struct comedi_device *dev,
604623
struct waveform_private *devpriv = dev->private;
605624

606625
s->async->inttrig = NULL;
626+
spin_lock_bh(&dev->spinlock);
627+
devpriv->ao_timer_enable = false;
628+
spin_unlock_bh(&dev->spinlock);
607629
if (in_softirq()) {
608630
/* Assume we were called from the timer routine itself. */
609631
del_timer(&devpriv->ao_timer);

0 commit comments

Comments
 (0)