Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/03/2019 14:33, Ian Abbott wrote:
`ni_cdio_cmdtest()` validates Comedi asynchronous commands for the DIO
subdevice (subdevice 2) of supported National Instruments M-series
cards.  It is called when handling the `COMEDI_CMD` and `COMEDI_CMDTEST`
ioctls for this subdevice.  There are two causes for a possible
divide-by-zero error when validating that the `stop_arg` member of the
passed-in command is not too large.

The first cause for the divide-by-zero is that calls to
`comedi_bytes_per_scan()` are only valid once the command has been
copied to `s->async->cmd`, but that copy is only done for the
`COMEDI_CMD` ioctl.  For the `COMEDI_CMDTEST` ioctl, it will use
whatever was left there by the previous `COMEDI_CMD` ioctl, if any.
(This is very likely, as it is usual for the application to use
`COMEDI_CMDTEST` before `COMEDI_CMD`.) If there has been no previous,
valid `COMEDI_CMD` for this subdevice, then `comedi_bytes_per_scan()`
will return 0, so the subsequent division in `ni_cdio_cmdtest()` of
`s->async->prealloc_bufsz / comedi_bytes_per_scan(s)` will be a
divide-by-zero error.  To fix this error, call a new function
`comedi_bytes_per_scan_cmd(s, cmd)`, based on the existing
`comedi_bytes_per_scan(s)` but using a specified `struct comedi_cmd` for
its calculations.  (Also refactor `comedi_bytes_per_scan()` to call the
new function.)

Once the first cause for the divide-by-zero has been fixed, the second
cause is that `comedi_bytes_per_scan_cmd()` can legitimately return 0 if
the `scan_end_arg` member of the `struct comedi_cmd` being tested is 0.
Fix it by only performing the division (and validating that `stop_arg`
is no more than the maximum value) if `comedi_bytes_per_scan_cmd()`
returns a non-zero value.

The problem was reported on the COMEDI mailing list here:
https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM

Fixes: f164cbf98fa8 ("staging: comedi: ni_mio_common: add finite regeneration to dio output")

Greg,
If it's not too late, it would be nice if the following "Reported-by:" and "Tested-by:" lines could be added (or I can resend with these lines included if necessary). It's no big deal if this is too late. I'll live with it. Thanks.

Reported-by: Ivan Vasilyev <grabesstimme@xxxxxxxxx>
Tested-by: Ivan Vasilyev <grabesstimme@xxxxxxxxx>

Cc: <stable@xxxxxxxxxxxxxxx> # 4.6+
Cc: Spencer E. Olson <olsonse@xxxxxxxxx>
Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
---
  drivers/staging/comedi/comedidev.h            |  2 ++
  drivers/staging/comedi/drivers.c              | 33 ++++++++++++++++---
  .../staging/comedi/drivers/ni_mio_common.c    | 10 ++++--
  3 files changed, 38 insertions(+), 7 deletions(-)

--
-=( Ian Abbott <abbotti@xxxxxxxxx> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux