| From 536de747bc48262225889a533db6650731ab25d3 Mon Sep 17 00:00:00 2001 |
| From: Johan Hovold <johan@kernel.org> |
| Date: Wed, 27 Oct 2021 11:35:29 +0200 |
| Subject: comedi: dt9812: fix DMA buffers on stack |
| |
| From: Johan Hovold <johan@kernel.org> |
| |
| commit 536de747bc48262225889a533db6650731ab25d3 upstream. |
| |
| USB transfer buffers are typically mapped for DMA and must not be |
| allocated on the stack or transfers will fail. |
| |
| Allocate proper transfer buffers in the various command helpers and |
| return an error on short transfers instead of acting on random stack |
| data. |
| |
| Note that this also fixes a stack info leak on systems where DMA is not |
| used as 32 bytes are always sent to the device regardless of how short |
| the command is. |
| |
| Fixes: 63274cd7d38a ("Staging: comedi: add usb dt9812 driver") |
| Cc: stable@vger.kernel.org # 2.6.29 |
| Reviewed-by: Ian Abbott <abbotti@mev.co.uk> |
| Signed-off-by: Johan Hovold <johan@kernel.org> |
| Link: https://lore.kernel.org/r/20211027093529.30896-3-johan@kernel.org |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/staging/comedi/drivers/dt9812.c | 115 +++++++++++++++++++++++--------- |
| 1 file changed, 86 insertions(+), 29 deletions(-) |
| |
| --- a/drivers/staging/comedi/drivers/dt9812.c |
| +++ b/drivers/staging/comedi/drivers/dt9812.c |
| @@ -41,6 +41,7 @@ |
| #include <linux/kernel.h> |
| #include <linux/module.h> |
| #include <linux/errno.h> |
| +#include <linux/slab.h> |
| #include <linux/uaccess.h> |
| |
| #include "../comedi_usb.h" |
| @@ -246,22 +247,42 @@ static int dt9812_read_info(struct comed |
| { |
| struct usb_device *usb = comedi_to_usb_dev(dev); |
| struct dt9812_private *devpriv = dev->private; |
| - struct dt9812_usb_cmd cmd; |
| + struct dt9812_usb_cmd *cmd; |
| + size_t tbuf_size; |
| int count, ret; |
| + void *tbuf; |
| |
| - cmd.cmd = cpu_to_le32(DT9812_R_FLASH_DATA); |
| - cmd.u.flash_data_info.address = |
| + tbuf_size = max(sizeof(*cmd), buf_size); |
| + |
| + tbuf = kzalloc(tbuf_size, GFP_KERNEL); |
| + if (!tbuf) |
| + return -ENOMEM; |
| + |
| + cmd = tbuf; |
| + |
| + cmd->cmd = cpu_to_le32(DT9812_R_FLASH_DATA); |
| + cmd->u.flash_data_info.address = |
| cpu_to_le16(DT9812_DIAGS_BOARD_INFO_ADDR + offset); |
| - cmd.u.flash_data_info.numbytes = cpu_to_le16(buf_size); |
| + cmd->u.flash_data_info.numbytes = cpu_to_le16(buf_size); |
| |
| /* DT9812 only responds to 32 byte writes!! */ |
| ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), |
| - &cmd, 32, &count, DT9812_USB_TIMEOUT); |
| + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); |
| if (ret) |
| - return ret; |
| + goto out; |
| + |
| + ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), |
| + tbuf, buf_size, &count, DT9812_USB_TIMEOUT); |
| + if (!ret) { |
| + if (count == buf_size) |
| + memcpy(buf, tbuf, buf_size); |
| + else |
| + ret = -EREMOTEIO; |
| + } |
| +out: |
| + kfree(tbuf); |
| |
| - return usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), |
| - buf, buf_size, &count, DT9812_USB_TIMEOUT); |
| + return ret; |
| } |
| |
| static int dt9812_read_multiple_registers(struct comedi_device *dev, |
| @@ -270,22 +291,42 @@ static int dt9812_read_multiple_register |
| { |
| struct usb_device *usb = comedi_to_usb_dev(dev); |
| struct dt9812_private *devpriv = dev->private; |
| - struct dt9812_usb_cmd cmd; |
| + struct dt9812_usb_cmd *cmd; |
| int i, count, ret; |
| + size_t buf_size; |
| + void *buf; |
| + |
| + buf_size = max_t(size_t, sizeof(*cmd), reg_count); |
| + |
| + buf = kzalloc(buf_size, GFP_KERNEL); |
| + if (!buf) |
| + return -ENOMEM; |
| + |
| + cmd = buf; |
| |
| - cmd.cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG); |
| - cmd.u.read_multi_info.count = reg_count; |
| + cmd->cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG); |
| + cmd->u.read_multi_info.count = reg_count; |
| for (i = 0; i < reg_count; i++) |
| - cmd.u.read_multi_info.address[i] = address[i]; |
| + cmd->u.read_multi_info.address[i] = address[i]; |
| |
| /* DT9812 only responds to 32 byte writes!! */ |
| ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), |
| - &cmd, 32, &count, DT9812_USB_TIMEOUT); |
| + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); |
| if (ret) |
| - return ret; |
| + goto out; |
| |
| - return usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), |
| - value, reg_count, &count, DT9812_USB_TIMEOUT); |
| + ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr), |
| + buf, reg_count, &count, DT9812_USB_TIMEOUT); |
| + if (!ret) { |
| + if (count == reg_count) |
| + memcpy(value, buf, reg_count); |
| + else |
| + ret = -EREMOTEIO; |
| + } |
| +out: |
| + kfree(buf); |
| + |
| + return ret; |
| } |
| |
| static int dt9812_write_multiple_registers(struct comedi_device *dev, |
| @@ -294,19 +335,27 @@ static int dt9812_write_multiple_registe |
| { |
| struct usb_device *usb = comedi_to_usb_dev(dev); |
| struct dt9812_private *devpriv = dev->private; |
| - struct dt9812_usb_cmd cmd; |
| + struct dt9812_usb_cmd *cmd; |
| int i, count; |
| + int ret; |
| |
| - cmd.cmd = cpu_to_le32(DT9812_W_MULTI_BYTE_REG); |
| - cmd.u.read_multi_info.count = reg_count; |
| + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL); |
| + if (!cmd) |
| + return -ENOMEM; |
| + |
| + cmd->cmd = cpu_to_le32(DT9812_W_MULTI_BYTE_REG); |
| + cmd->u.read_multi_info.count = reg_count; |
| for (i = 0; i < reg_count; i++) { |
| - cmd.u.write_multi_info.write[i].address = address[i]; |
| - cmd.u.write_multi_info.write[i].value = value[i]; |
| + cmd->u.write_multi_info.write[i].address = address[i]; |
| + cmd->u.write_multi_info.write[i].value = value[i]; |
| } |
| |
| /* DT9812 only responds to 32 byte writes!! */ |
| - return usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), |
| - &cmd, 32, &count, DT9812_USB_TIMEOUT); |
| + ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), |
| + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); |
| + kfree(cmd); |
| + |
| + return ret; |
| } |
| |
| static int dt9812_rmw_multiple_registers(struct comedi_device *dev, |
| @@ -315,17 +364,25 @@ static int dt9812_rmw_multiple_registers |
| { |
| struct usb_device *usb = comedi_to_usb_dev(dev); |
| struct dt9812_private *devpriv = dev->private; |
| - struct dt9812_usb_cmd cmd; |
| + struct dt9812_usb_cmd *cmd; |
| int i, count; |
| + int ret; |
| + |
| + cmd = kzalloc(sizeof(*cmd), GFP_KERNEL); |
| + if (!cmd) |
| + return -ENOMEM; |
| |
| - cmd.cmd = cpu_to_le32(DT9812_RMW_MULTI_BYTE_REG); |
| - cmd.u.rmw_multi_info.count = reg_count; |
| + cmd->cmd = cpu_to_le32(DT9812_RMW_MULTI_BYTE_REG); |
| + cmd->u.rmw_multi_info.count = reg_count; |
| for (i = 0; i < reg_count; i++) |
| - cmd.u.rmw_multi_info.rmw[i] = rmw[i]; |
| + cmd->u.rmw_multi_info.rmw[i] = rmw[i]; |
| |
| /* DT9812 only responds to 32 byte writes!! */ |
| - return usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), |
| - &cmd, 32, &count, DT9812_USB_TIMEOUT); |
| + ret = usb_bulk_msg(usb, usb_sndbulkpipe(usb, devpriv->cmd_wr.addr), |
| + cmd, sizeof(*cmd), &count, DT9812_USB_TIMEOUT); |
| + kfree(cmd); |
| + |
| + return ret; |
| } |
| |
| static int dt9812_digital_in(struct comedi_device *dev, u8 *bits) |