5.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Ian Abbott abbotti@mev.co.uk
[ Upstream commit 46d8c744136ce2454aa4c35c138cc06817f92b8e ]
Some Comedi subdevice instruction handlers are known to access instruction data elements beyond the first `insn->n` elements in some cases. The `do_insn_ioctl()` and `do_insnlist_ioctl()` functions allocate at least `MIN_SAMPLES` (16) data elements to deal with this, but they do not initialize all of that. For Comedi instruction codes that write to the subdevice, the first `insn->n` data elements are copied from user-space, but the remaining elements are left uninitialized. That could be a problem if the subdevice instruction handler reads the uninitialized data. Ensure that the first `MIN_SAMPLES` elements are initialized before calling these instruction handlers, filling the uncopied elements with 0. For `do_insnlist_ioctl()`, the same data buffer elements are used for handling a list of instructions, so ensure the first `MIN_SAMPLES` elements are initialized for each instruction that writes to the subdevice.
Fixes: ed9eccbe8970 ("Staging: add comedi core") Cc: stable@vger.kernel.org # 5.13+ Signed-off-by: Ian Abbott abbotti@mev.co.uk Link: https://lore.kernel.org/r/20250707161439.88385-1-abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ Reworked for before commit bac42fb21259 ("comedi: get rid of compat_alloc_user_space() mess in COMEDI_CMD{,TEST} compat") ] Signed-off-by: Ian Abbott abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- v2: Fixed a patch application error due to fixing up the wrong commit. Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/staging/comedi/comedi_fops.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
--- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -1584,21 +1584,27 @@ static int do_insnlist_ioctl(struct come }
for (i = 0; i < insnlist.n_insns; ++i) { + unsigned int n = insns[i].n; + if (insns[i].insn & INSN_MASK_WRITE) { if (copy_from_user(data, insns[i].data, - insns[i].n * sizeof(unsigned int))) { + n * sizeof(unsigned int))) { dev_dbg(dev->class_dev, "copy_from_user failed\n"); ret = -EFAULT; goto error; } + if (n < MIN_SAMPLES) { + memset(&data[n], 0, (MIN_SAMPLES - n) * + sizeof(unsigned int)); + } } ret = parse_insn(dev, insns + i, data, file); if (ret < 0) goto error; if (insns[i].insn & INSN_MASK_READ) { if (copy_to_user(insns[i].data, data, - insns[i].n * sizeof(unsigned int))) { + n * sizeof(unsigned int))) { dev_dbg(dev->class_dev, "copy_to_user failed\n"); ret = -EFAULT; @@ -1665,6 +1671,10 @@ static int do_insn_ioctl(struct comedi_d ret = -EFAULT; goto error; } + if (insn.n < MIN_SAMPLES) { + memset(&data[insn.n], 0, + (MIN_SAMPLES - insn.n) * sizeof(unsigned int)); + } } ret = parse_insn(dev, &insn, data, file); if (ret < 0)