patch "staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest" added to staging-linus
Subsystems (2)
This is a note to let you know that I've just added the patch titled
staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest
to my staging git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
in the staging-linus branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
From bafd9c64056cd034a1174dcadb65cd3b294ff8f6 Mon Sep 17 00:00:00 2001
From: Ian Abbott <[email protected]>
Date: Mon, 4 Mar 2019 14:33:54 +0000
Subject: 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
--- 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(-)
drivers/staging/comedi/comedidev.h +2 -0
@@ -1001,6 +1001,8 @@ int comedi_dio_insn_config(struct comedi_device *dev, unsigned int mask);unsigned int comedi_dio_update_state (struct comedi_subdevice *s,unsigned int *data); +unsigned int comedi_bytes_per_scan_cmd (struct comedi_subdevice *s, +struct comedi_cmd *cmd);unsigned int comedi_bytes_per_scan (struct comedi_subdevice *s);unsigned int comedi_nscans_left (struct comedi_subdevice *s,unsigned int nscans);
drivers/staging/comedi/drivers.c +29 -4
@@ -394,11 +394,13 @@ unsigned int comedi_dio_update_state(struct comedi_subdevice *s,EXPORT_SYMBOL_GPL (comedi_dio_update_state);/** -* comedi_bytes_per_scan() - Get length of asynchronous command "scan" in bytes +* comedi_bytes_per_scan_cmd() - Get length of asynchronous command "scan" in +* bytes * @s: COMEDI subdevice. +* @cmd: COMEDI command. * * Determines the overall scan length according to the subdevice type and the -* number of channels in the scan. +* number of channels in the scan for the specified command. * * For digital input, output or input/output subdevices, samples for * multiple channels are assumed to be packed into one or more unsigned
@@ -408,9 +410,9 @@ EXPORT_SYMBOL_GPL(comedi_dio_update_state);* * Returns the overall scan length in bytes. */ -unsigned int comedi_bytes_per_scan (struct comedi_subdevice *s) +unsigned int comedi_bytes_per_scan_cmd (struct comedi_subdevice *s, +struct comedi_cmd *cmd) { -struct comedi_cmd *cmd = &s->async->cmd;unsigned int num_samples;unsigned int bits_per_sample;
@@ -427,6 +429,29 @@ unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s) }return comedi_samples_to_bytes (s, num_samples); } +EXPORT_SYMBOL_GPL (comedi_bytes_per_scan_cmd); + +/** +* comedi_bytes_per_scan() - Get length of asynchronous command "scan" in bytes +* @s: COMEDI subdevice. +* +* Determines the overall scan length according to the subdevice type and the +* number of channels in the scan for the current command. +* +* For digital input, output or input/output subdevices, samples for +* multiple channels are assumed to be packed into one or more unsigned +* short or unsigned int values according to the subdevice's %SDF_LSAMPL +* flag. For other types of subdevice, samples are assumed to occupy a +* whole unsigned short or unsigned int according to the %SDF_LSAMPL flag. +* +* Returns the overall scan length in bytes. +*/ +unsigned int comedi_bytes_per_scan (struct comedi_subdevice *s) +{ +struct comedi_cmd *cmd = &s->async->cmd; + +return comedi_bytes_per_scan_cmd (s, cmd); +}EXPORT_SYMBOL_GPL (comedi_bytes_per_scan); static unsigned int __comedi_nscans_left(struct comedi_subdevice *s,
drivers/staging/comedi/drivers/ni_mio_common.c +7 -3
@@ -3545,6 +3545,7 @@ static int ni_cdio_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s, struct comedi_cmd *cmd) { struct ni_private *devpriv = dev->private; +unsigned int bytes_per_scan;int err =0 ;/* Step 1 : check if triggers are trivially valid */
@@ -3579,9 +3580,12 @@ static int ni_cdio_cmdtest(struct comedi_device *dev, err |=comedi_check_trigger_arg_is (&cmd->convert_arg,0 ); err |=comedi_check_trigger_arg_is (&cmd->scan_end_arg, cmd->chanlist_len); - err |=comedi_check_trigger_arg_max (&cmd->stop_arg, - s->async->prealloc_bufsz / -comedi_bytes_per_scan (s)); + bytes_per_scan =comedi_bytes_per_scan_cmd (s, cmd); +if (bytes_per_scan) { + err |=comedi_check_trigger_arg_max (&cmd->stop_arg, + s->async->prealloc_bufsz / + bytes_per_scan); + }if (err)return 3 ;
Signature · 1 line
2.21.0