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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20190318065426.GA11577@kroah.com>
Date:   Mon, 18 Mar 2019 07:54:26 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Ian Abbott <abbotti@....co.uk>
Cc:     devel@...verdev.osuosl.org,
        H Hartley Sweeten <hsweeten@...ionengravers.com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        "Spencer E . Olson" <olsonse@...ch.edu>
Subject: Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for
 DIO cmdtest

On Wed, Mar 13, 2019 at 06:57:17PM +0000, Ian Abbott wrote:
> 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@...il.com>
> Tested-by: Ivan Vasilyev <grabesstimme@...il.com>

Not too late, I'll go add it now, thanks.

greg k-h