diff options
| author | Ian Abbott <abbotti@mev.co.uk> | 2019-03-04 09:33:54 -0500 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2019-03-18 02:57:58 -0400 |
| commit | bafd9c64056cd034a1174dcadb65cd3b294ff8f6 (patch) | |
| tree | 3251b6a4f0f14c9be41e5ed14d17fa9f29f20b9a | |
| parent | 8bce6dcede65139a087ff240127e3f3c01363eed (diff) | |
staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest
`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
Reported-by: Ivan Vasilyev <grabesstimme@gmail.com>
Tested-by: Ivan Vasilyev <grabesstimme@gmail.com>
Fixes: f164cbf98fa8 ("staging: comedi: ni_mio_common: add finite regeneration to dio output")
Cc: <stable@vger.kernel.org> # 4.6+
Cc: Spencer E. Olson <olsonse@umich.edu>
Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
| -rw-r--r-- | drivers/staging/comedi/comedidev.h | 2 | ||||
| -rw-r--r-- | drivers/staging/comedi/drivers.c | 33 | ||||
| -rw-r--r-- | drivers/staging/comedi/drivers/ni_mio_common.c | 10 |
3 files changed, 38 insertions, 7 deletions
diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h index a7d569cfca5d..0dff1ac057cd 100644 --- a/drivers/staging/comedi/comedidev.h +++ b/drivers/staging/comedi/comedidev.h | |||
| @@ -1001,6 +1001,8 @@ int comedi_dio_insn_config(struct comedi_device *dev, | |||
| 1001 | unsigned int mask); | 1001 | unsigned int mask); |
| 1002 | unsigned int comedi_dio_update_state(struct comedi_subdevice *s, | 1002 | unsigned int comedi_dio_update_state(struct comedi_subdevice *s, |
| 1003 | unsigned int *data); | 1003 | unsigned int *data); |
| 1004 | unsigned int comedi_bytes_per_scan_cmd(struct comedi_subdevice *s, | ||
| 1005 | struct comedi_cmd *cmd); | ||
| 1004 | unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s); | 1006 | unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s); |
| 1005 | unsigned int comedi_nscans_left(struct comedi_subdevice *s, | 1007 | unsigned int comedi_nscans_left(struct comedi_subdevice *s, |
| 1006 | unsigned int nscans); | 1008 | unsigned int nscans); |
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c index eefa62f42c0f..5a32b8fc000e 100644 --- a/drivers/staging/comedi/drivers.c +++ b/drivers/staging/comedi/drivers.c | |||
| @@ -394,11 +394,13 @@ unsigned int comedi_dio_update_state(struct comedi_subdevice *s, | |||
| 394 | EXPORT_SYMBOL_GPL(comedi_dio_update_state); | 394 | EXPORT_SYMBOL_GPL(comedi_dio_update_state); |
| 395 | 395 | ||
| 396 | /** | 396 | /** |
| 397 | * comedi_bytes_per_scan() - Get length of asynchronous command "scan" in bytes | 397 | * comedi_bytes_per_scan_cmd() - Get length of asynchronous command "scan" in |
| 398 | * bytes | ||
| 398 | * @s: COMEDI subdevice. | 399 | * @s: COMEDI subdevice. |
| 400 | * @cmd: COMEDI command. | ||
| 399 | * | 401 | * |
| 400 | * Determines the overall scan length according to the subdevice type and the | 402 | * Determines the overall scan length according to the subdevice type and the |
| 401 | * number of channels in the scan. | 403 | * number of channels in the scan for the specified command. |
| 402 | * | 404 | * |
| 403 | * For digital input, output or input/output subdevices, samples for | 405 | * For digital input, output or input/output subdevices, samples for |
| 404 | * multiple channels are assumed to be packed into one or more unsigned | 406 | * multiple channels are assumed to be packed into one or more unsigned |
| @@ -408,9 +410,9 @@ EXPORT_SYMBOL_GPL(comedi_dio_update_state); | |||
| 408 | * | 410 | * |
| 409 | * Returns the overall scan length in bytes. | 411 | * Returns the overall scan length in bytes. |
| 410 | */ | 412 | */ |
| 411 | unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s) | 413 | unsigned int comedi_bytes_per_scan_cmd(struct comedi_subdevice *s, |
| 414 | struct comedi_cmd *cmd) | ||
| 412 | { | 415 | { |
| 413 | struct comedi_cmd *cmd = &s->async->cmd; | ||
| 414 | unsigned int num_samples; | 416 | unsigned int num_samples; |
| 415 | unsigned int bits_per_sample; | 417 | unsigned int bits_per_sample; |
| 416 | 418 | ||
| @@ -427,6 +429,29 @@ unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s) | |||
| 427 | } | 429 | } |
| 428 | return comedi_samples_to_bytes(s, num_samples); | 430 | return comedi_samples_to_bytes(s, num_samples); |
| 429 | } | 431 | } |
| 432 | EXPORT_SYMBOL_GPL(comedi_bytes_per_scan_cmd); | ||
| 433 | |||
| 434 | /** | ||
| 435 | * comedi_bytes_per_scan() - Get length of asynchronous command "scan" in bytes | ||
| 436 | * @s: COMEDI subdevice. | ||
| 437 | * | ||
| 438 | * Determines the overall scan length according to the subdevice type and the | ||
| 439 | * number of channels in the scan for the current command. | ||
| 440 | * | ||
| 441 | * For digital input, output or input/output subdevices, samples for | ||
| 442 | * multiple channels are assumed to be packed into one or more unsigned | ||
| 443 | * short or unsigned int values according to the subdevice's %SDF_LSAMPL | ||
| 444 | * flag. For other types of subdevice, samples are assumed to occupy a | ||
| 445 | * whole unsigned short or unsigned int according to the %SDF_LSAMPL flag. | ||
| 446 | * | ||
| 447 | * Returns the overall scan length in bytes. | ||
| 448 | */ | ||
| 449 | unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s) | ||
| 450 | { | ||
| 451 | struct comedi_cmd *cmd = &s->async->cmd; | ||
| 452 | |||
| 453 | return comedi_bytes_per_scan_cmd(s, cmd); | ||
| 454 | } | ||
| 430 | EXPORT_SYMBOL_GPL(comedi_bytes_per_scan); | 455 | EXPORT_SYMBOL_GPL(comedi_bytes_per_scan); |
| 431 | 456 | ||
| 432 | static unsigned int __comedi_nscans_left(struct comedi_subdevice *s, | 457 | static unsigned int __comedi_nscans_left(struct comedi_subdevice *s, |
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index 5edf59ac6706..b04dad8c7092 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c | |||
| @@ -3545,6 +3545,7 @@ static int ni_cdio_cmdtest(struct comedi_device *dev, | |||
| 3545 | struct comedi_subdevice *s, struct comedi_cmd *cmd) | 3545 | struct comedi_subdevice *s, struct comedi_cmd *cmd) |
| 3546 | { | 3546 | { |
| 3547 | struct ni_private *devpriv = dev->private; | 3547 | struct ni_private *devpriv = dev->private; |
| 3548 | unsigned int bytes_per_scan; | ||
| 3548 | int err = 0; | 3549 | int err = 0; |
| 3549 | 3550 | ||
| 3550 | /* Step 1 : check if triggers are trivially valid */ | 3551 | /* Step 1 : check if triggers are trivially valid */ |
| @@ -3579,9 +3580,12 @@ static int ni_cdio_cmdtest(struct comedi_device *dev, | |||
| 3579 | err |= comedi_check_trigger_arg_is(&cmd->convert_arg, 0); | 3580 | err |= comedi_check_trigger_arg_is(&cmd->convert_arg, 0); |
| 3580 | err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg, | 3581 | err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg, |
| 3581 | cmd->chanlist_len); | 3582 | cmd->chanlist_len); |
| 3582 | err |= comedi_check_trigger_arg_max(&cmd->stop_arg, | 3583 | bytes_per_scan = comedi_bytes_per_scan_cmd(s, cmd); |
| 3583 | s->async->prealloc_bufsz / | 3584 | if (bytes_per_scan) { |
| 3584 | comedi_bytes_per_scan(s)); | 3585 | err |= comedi_check_trigger_arg_max(&cmd->stop_arg, |
| 3586 | s->async->prealloc_bufsz / | ||
| 3587 | bytes_per_scan); | ||
| 3588 | } | ||
| 3585 | 3589 | ||
| 3586 | if (err) | 3590 | if (err) |
| 3587 | return 3; | 3591 | return 3; |
