Skip to content

Commit 5149525

Browse files
ian-abbottgregkh
authored andcommitted
comedi: Use reference count for asynchronous command functions
For interrupts from badly behaved hardware (as emulated by Syzbot), it is possible for the Comedi core functions that manage the progress of asynchronous data acquisition to be called from driver ISRs while no asynchronous command has been set up, which can cause problems such as invalid pointer dereferencing or dividing by zero. Change those functions in the Comedi core to use this pattern: if `comedi_get_is_subdevice_running(s)` returns `true` then call a safe version of the function with the same name prefixed with an underscore, followed by a call to `comedi_put_is_subdevice_running(s)`, otherwise take some default action. `comedi_get_is_subdevice_running(s)` returning `true` ensures that the details of the asynchronous command will not be destroyed before the matching call to `comedi_put_is_subdevice_running(s)`. Replace calls to those functions from elsewhere in the Comedi core with calls to the safe versions of the functions. The modified functions are: `comedi_buf_read_alloc()`, `comedi_buf_read_free()`, `comedi_buf_read_n_available()`, `comedi_buf_read_samples()`, `comedi_buf_write_alloc()`, `comedi_buf_write_free()`, `comedi_buf_write_samples()`, `comedi_bytes_per_scan()`, `comedi_event()`, `comedi_handle_events()`, `comedi_inc_scan_progress()`, `comedi_nsamples_left()`, `comedi_nscans_left()`. Signed-off-by: Ian Abbott <abbotti@mev.co.uk> Link: https://patch.msgid.link/20251023133001.8439-3-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 4e1da51 commit 5149525

4 files changed

Lines changed: 323 additions & 152 deletions

File tree

drivers/comedi/comedi_buf.c

Lines changed: 182 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -273,19 +273,8 @@ unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s)
273273
return free_end - async->buf_write_count;
274274
}
275275

276-
/**
277-
* comedi_buf_write_alloc() - Reserve buffer space for writing
278-
* @s: COMEDI subdevice.
279-
* @nbytes: Maximum space to reserve in bytes.
280-
*
281-
* Reserve up to @nbytes bytes of space to be written in the COMEDI acquisition
282-
* data buffer associated with the subdevice. The amount reserved is limited
283-
* by the space available.
284-
*
285-
* Return: The amount of space reserved in bytes.
286-
*/
287-
unsigned int comedi_buf_write_alloc(struct comedi_subdevice *s,
288-
unsigned int nbytes)
276+
unsigned int _comedi_buf_write_alloc(struct comedi_subdevice *s,
277+
unsigned int nbytes)
289278
{
290279
struct comedi_async *async = s->async;
291280
unsigned int unalloc = comedi_buf_write_n_unalloc(s);
@@ -303,6 +292,29 @@ unsigned int comedi_buf_write_alloc(struct comedi_subdevice *s,
303292

304293
return nbytes;
305294
}
295+
296+
/**
297+
* comedi_buf_write_alloc() - Reserve buffer space for writing
298+
* @s: COMEDI subdevice.
299+
* @nbytes: Maximum space to reserve in bytes.
300+
*
301+
* Reserve up to @nbytes bytes of space to be written in the COMEDI acquisition
302+
* data buffer associated with the subdevice. The amount reserved is limited
303+
* by the space available.
304+
*
305+
* Return: The amount of space reserved in bytes.
306+
*/
307+
unsigned int comedi_buf_write_alloc(struct comedi_subdevice *s,
308+
unsigned int nbytes)
309+
{
310+
if (comedi_get_is_subdevice_running(s)) {
311+
nbytes = _comedi_buf_write_alloc(s, nbytes);
312+
comedi_put_is_subdevice_running(s);
313+
} else {
314+
nbytes = 0;
315+
}
316+
return nbytes;
317+
}
306318
EXPORT_SYMBOL_GPL(comedi_buf_write_alloc);
307319

308320
/*
@@ -362,6 +374,24 @@ unsigned int comedi_buf_write_n_allocated(struct comedi_subdevice *s)
362374
return async->buf_write_alloc_count - async->buf_write_count;
363375
}
364376

377+
unsigned int _comedi_buf_write_free(struct comedi_subdevice *s,
378+
unsigned int nbytes)
379+
{
380+
struct comedi_async *async = s->async;
381+
unsigned int allocated = comedi_buf_write_n_allocated(s);
382+
383+
if (nbytes > allocated)
384+
nbytes = allocated;
385+
386+
async->buf_write_count += nbytes;
387+
async->buf_write_ptr += nbytes;
388+
comedi_buf_munge(s, async->buf_write_count - async->munge_count);
389+
if (async->buf_write_ptr >= async->prealloc_bufsz)
390+
async->buf_write_ptr %= async->prealloc_bufsz;
391+
392+
return nbytes;
393+
}
394+
365395
/**
366396
* comedi_buf_write_free() - Free buffer space after it is written
367397
* @s: COMEDI subdevice.
@@ -379,22 +409,35 @@ unsigned int comedi_buf_write_n_allocated(struct comedi_subdevice *s)
379409
*/
380410
unsigned int comedi_buf_write_free(struct comedi_subdevice *s,
381411
unsigned int nbytes)
412+
{
413+
if (comedi_get_is_subdevice_running(s)) {
414+
nbytes = _comedi_buf_write_free(s, nbytes);
415+
comedi_put_is_subdevice_running(s);
416+
} else {
417+
nbytes = 0;
418+
}
419+
return nbytes;
420+
}
421+
EXPORT_SYMBOL_GPL(comedi_buf_write_free);
422+
423+
unsigned int _comedi_buf_read_n_available(struct comedi_subdevice *s)
382424
{
383425
struct comedi_async *async = s->async;
384-
unsigned int allocated = comedi_buf_write_n_allocated(s);
426+
unsigned int num_bytes;
385427

386-
if (nbytes > allocated)
387-
nbytes = allocated;
428+
if (!async)
429+
return 0;
388430

389-
async->buf_write_count += nbytes;
390-
async->buf_write_ptr += nbytes;
391-
comedi_buf_munge(s, async->buf_write_count - async->munge_count);
392-
if (async->buf_write_ptr >= async->prealloc_bufsz)
393-
async->buf_write_ptr %= async->prealloc_bufsz;
431+
num_bytes = async->munge_count - async->buf_read_count;
394432

395-
return nbytes;
433+
/*
434+
* ensure the async buffer 'counts' are read before we
435+
* attempt to read data from the buffer
436+
*/
437+
smp_rmb();
438+
439+
return num_bytes;
396440
}
397-
EXPORT_SYMBOL_GPL(comedi_buf_write_free);
398441

399442
/**
400443
* comedi_buf_read_n_available() - Determine amount of readable buffer space
@@ -409,23 +452,38 @@ EXPORT_SYMBOL_GPL(comedi_buf_write_free);
409452
*/
410453
unsigned int comedi_buf_read_n_available(struct comedi_subdevice *s)
411454
{
412-
struct comedi_async *async = s->async;
413455
unsigned int num_bytes;
414456

415-
if (!async)
416-
return 0;
457+
if (comedi_get_is_subdevice_running(s)) {
458+
num_bytes = _comedi_buf_read_n_available(s);
459+
comedi_put_is_subdevice_running(s);
460+
} else {
461+
num_bytes = 0;
462+
}
463+
return num_bytes;
464+
}
465+
EXPORT_SYMBOL_GPL(comedi_buf_read_n_available);
417466

418-
num_bytes = async->munge_count - async->buf_read_count;
467+
unsigned int _comedi_buf_read_alloc(struct comedi_subdevice *s,
468+
unsigned int nbytes)
469+
{
470+
struct comedi_async *async = s->async;
471+
unsigned int available;
472+
473+
available = async->munge_count - async->buf_read_alloc_count;
474+
if (nbytes > available)
475+
nbytes = available;
476+
477+
async->buf_read_alloc_count += nbytes;
419478

420479
/*
421480
* ensure the async buffer 'counts' are read before we
422-
* attempt to read data from the buffer
481+
* attempt to read data from the read-alloc'ed buffer space
423482
*/
424483
smp_rmb();
425484

426-
return num_bytes;
485+
return nbytes;
427486
}
428-
EXPORT_SYMBOL_GPL(comedi_buf_read_n_available);
429487

430488
/**
431489
* comedi_buf_read_alloc() - Reserve buffer space for reading
@@ -445,21 +503,12 @@ EXPORT_SYMBOL_GPL(comedi_buf_read_n_available);
445503
unsigned int comedi_buf_read_alloc(struct comedi_subdevice *s,
446504
unsigned int nbytes)
447505
{
448-
struct comedi_async *async = s->async;
449-
unsigned int available;
450-
451-
available = async->munge_count - async->buf_read_alloc_count;
452-
if (nbytes > available)
453-
nbytes = available;
454-
455-
async->buf_read_alloc_count += nbytes;
456-
457-
/*
458-
* ensure the async buffer 'counts' are read before we
459-
* attempt to read data from the read-alloc'ed buffer space
460-
*/
461-
smp_rmb();
462-
506+
if (comedi_get_is_subdevice_running(s)) {
507+
nbytes = _comedi_buf_read_alloc(s, nbytes);
508+
comedi_put_is_subdevice_running(s);
509+
} else {
510+
nbytes = 0;
511+
}
463512
return nbytes;
464513
}
465514
EXPORT_SYMBOL_GPL(comedi_buf_read_alloc);
@@ -469,21 +518,8 @@ static unsigned int comedi_buf_read_n_allocated(struct comedi_async *async)
469518
return async->buf_read_alloc_count - async->buf_read_count;
470519
}
471520

472-
/**
473-
* comedi_buf_read_free() - Free buffer space after it has been read
474-
* @s: COMEDI subdevice.
475-
* @nbytes: Maximum space to free in bytes.
476-
*
477-
* Free up to @nbytes bytes of buffer space previously reserved for reading in
478-
* the COMEDI acquisition data buffer associated with the subdevice. The
479-
* amount of space freed is limited to the amount that was reserved.
480-
*
481-
* The freed space becomes available for allocation by the writer.
482-
*
483-
* Return: The amount of space freed in bytes.
484-
*/
485-
unsigned int comedi_buf_read_free(struct comedi_subdevice *s,
486-
unsigned int nbytes)
521+
unsigned int _comedi_buf_read_free(struct comedi_subdevice *s,
522+
unsigned int nbytes)
487523
{
488524
struct comedi_async *async = s->async;
489525
unsigned int allocated;
@@ -503,6 +539,31 @@ unsigned int comedi_buf_read_free(struct comedi_subdevice *s,
503539
async->buf_read_ptr %= async->prealloc_bufsz;
504540
return nbytes;
505541
}
542+
543+
/**
544+
* comedi_buf_read_free() - Free buffer space after it has been read
545+
* @s: COMEDI subdevice.
546+
* @nbytes: Maximum space to free in bytes.
547+
*
548+
* Free up to @nbytes bytes of buffer space previously reserved for reading in
549+
* the COMEDI acquisition data buffer associated with the subdevice. The
550+
* amount of space freed is limited to the amount that was reserved.
551+
*
552+
* The freed space becomes available for allocation by the writer.
553+
*
554+
* Return: The amount of space freed in bytes.
555+
*/
556+
unsigned int comedi_buf_read_free(struct comedi_subdevice *s,
557+
unsigned int nbytes)
558+
{
559+
if (comedi_get_is_subdevice_running(s)) {
560+
nbytes = _comedi_buf_read_free(s, nbytes);
561+
comedi_put_is_subdevice_running(s);
562+
} else {
563+
nbytes = 0;
564+
}
565+
return nbytes;
566+
}
506567
EXPORT_SYMBOL_GPL(comedi_buf_read_free);
507568

508569
static void comedi_buf_memcpy_to(struct comedi_subdevice *s,
@@ -558,6 +619,38 @@ static void comedi_buf_memcpy_from(struct comedi_subdevice *s,
558619
}
559620
}
560621

622+
static unsigned int _comedi_buf_write_samples(struct comedi_subdevice *s,
623+
const void *data,
624+
unsigned int nsamples)
625+
{
626+
unsigned int max_samples;
627+
unsigned int nbytes;
628+
629+
/*
630+
* Make sure there is enough room in the buffer for all the samples.
631+
* If not, clamp the nsamples to the number that will fit, flag the
632+
* buffer overrun and add the samples that fit.
633+
*/
634+
max_samples = comedi_bytes_to_samples(s, comedi_buf_write_n_unalloc(s));
635+
if (nsamples > max_samples) {
636+
dev_warn(s->device->class_dev, "buffer overrun\n");
637+
s->async->events |= COMEDI_CB_OVERFLOW;
638+
nsamples = max_samples;
639+
}
640+
641+
if (nsamples == 0)
642+
return 0;
643+
644+
nbytes = comedi_samples_to_bytes(s, nsamples);
645+
nbytes = _comedi_buf_write_alloc(s, nbytes);
646+
comedi_buf_memcpy_to(s, data, nbytes);
647+
_comedi_buf_write_free(s, nbytes);
648+
_comedi_inc_scan_progress(s, nbytes);
649+
s->async->events |= COMEDI_CB_BLOCK;
650+
651+
return nbytes;
652+
}
653+
561654
/**
562655
* comedi_buf_write_samples() - Write sample data to COMEDI buffer
563656
* @s: COMEDI subdevice.
@@ -577,35 +670,43 @@ static void comedi_buf_memcpy_from(struct comedi_subdevice *s,
577670
*/
578671
unsigned int comedi_buf_write_samples(struct comedi_subdevice *s,
579672
const void *data, unsigned int nsamples)
673+
{
674+
unsigned int nbytes;
675+
676+
if (comedi_get_is_subdevice_running(s)) {
677+
nbytes = _comedi_buf_write_samples(s, data, nsamples);
678+
comedi_put_is_subdevice_running(s);
679+
} else {
680+
nbytes = 0;
681+
}
682+
return nbytes;
683+
}
684+
EXPORT_SYMBOL_GPL(comedi_buf_write_samples);
685+
686+
static unsigned int _comedi_buf_read_samples(struct comedi_subdevice *s,
687+
void *data, unsigned int nsamples)
580688
{
581689
unsigned int max_samples;
582690
unsigned int nbytes;
583691

584-
/*
585-
* Make sure there is enough room in the buffer for all the samples.
586-
* If not, clamp the nsamples to the number that will fit, flag the
587-
* buffer overrun and add the samples that fit.
588-
*/
589-
max_samples = comedi_bytes_to_samples(s, comedi_buf_write_n_unalloc(s));
590-
if (nsamples > max_samples) {
591-
dev_warn(s->device->class_dev, "buffer overrun\n");
592-
s->async->events |= COMEDI_CB_OVERFLOW;
692+
/* clamp nsamples to the number of full samples available */
693+
max_samples = comedi_bytes_to_samples(s,
694+
_comedi_buf_read_n_available(s));
695+
if (nsamples > max_samples)
593696
nsamples = max_samples;
594-
}
595697

596698
if (nsamples == 0)
597699
return 0;
598700

599-
nbytes = comedi_buf_write_alloc(s,
701+
nbytes = _comedi_buf_read_alloc(s,
600702
comedi_samples_to_bytes(s, nsamples));
601-
comedi_buf_memcpy_to(s, data, nbytes);
602-
comedi_buf_write_free(s, nbytes);
603-
comedi_inc_scan_progress(s, nbytes);
703+
comedi_buf_memcpy_from(s, data, nbytes);
704+
_comedi_buf_read_free(s, nbytes);
705+
_comedi_inc_scan_progress(s, nbytes);
604706
s->async->events |= COMEDI_CB_BLOCK;
605707

606708
return nbytes;
607709
}
608-
EXPORT_SYMBOL_GPL(comedi_buf_write_samples);
609710

610711
/**
611712
* comedi_buf_read_samples() - Read sample data from COMEDI buffer
@@ -624,25 +725,14 @@ EXPORT_SYMBOL_GPL(comedi_buf_write_samples);
624725
unsigned int comedi_buf_read_samples(struct comedi_subdevice *s,
625726
void *data, unsigned int nsamples)
626727
{
627-
unsigned int max_samples;
628728
unsigned int nbytes;
629729

630-
/* clamp nsamples to the number of full samples available */
631-
max_samples = comedi_bytes_to_samples(s,
632-
comedi_buf_read_n_available(s));
633-
if (nsamples > max_samples)
634-
nsamples = max_samples;
635-
636-
if (nsamples == 0)
637-
return 0;
638-
639-
nbytes = comedi_buf_read_alloc(s,
640-
comedi_samples_to_bytes(s, nsamples));
641-
comedi_buf_memcpy_from(s, data, nbytes);
642-
comedi_buf_read_free(s, nbytes);
643-
comedi_inc_scan_progress(s, nbytes);
644-
s->async->events |= COMEDI_CB_BLOCK;
645-
730+
if (comedi_get_is_subdevice_running(s)) {
731+
nbytes = _comedi_buf_read_samples(s, data, nsamples);
732+
comedi_put_is_subdevice_running(s);
733+
} else {
734+
nbytes = 0;
735+
}
646736
return nbytes;
647737
}
648738
EXPORT_SYMBOL_GPL(comedi_buf_read_samples);

0 commit comments

Comments
 (0)