All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] comedi: misc fixes
@ 2021-10-27  9:35 Johan Hovold
  2021-10-27  9:35 ` [PATCH v2 1/2] comedi: ni_usb6501: fix NULL-deref in command paths Johan Hovold
  2021-10-27  9:35 ` [PATCH v2 2/2] comedi: dt9812: fix DMA buffers on stack Johan Hovold
  0 siblings, 2 replies; 5+ messages in thread
From: Johan Hovold @ 2021-10-27  9:35 UTC (permalink / raw)
  To: Ian Abbott, H Hartley Sweeten
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Johan Hovold

This series fixes a number of issues in the comedi drivers that were
found through inspection.

Johan

Changes in v2:
 - drop the three patches already applied
 - move max-packet check after the ni_usb6501 endpoint lookup
 - use a single transfer buffer for both TX and RX in dt9812 (suggested
   by Ian Abbott)


Johan Hovold (2):
  comedi: ni_usb6501: fix NULL-deref in command paths
  comedi: dt9812: fix DMA buffers on stack

 drivers/comedi/drivers/dt9812.c     | 115 +++++++++++++++++++++-------
 drivers/comedi/drivers/ni_usb6501.c |  10 +++
 2 files changed, 96 insertions(+), 29 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] comedi: ni_usb6501: fix NULL-deref in command paths
  2021-10-27  9:35 [PATCH v2 0/2] comedi: misc fixes Johan Hovold
@ 2021-10-27  9:35 ` Johan Hovold
  2021-10-28  9:44   ` Ian Abbott
  2021-10-27  9:35 ` [PATCH v2 2/2] comedi: dt9812: fix DMA buffers on stack Johan Hovold
  1 sibling, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2021-10-27  9:35 UTC (permalink / raw)
  To: Ian Abbott, H Hartley Sweeten
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Johan Hovold,
	stable, Luca Ellero

The driver uses endpoint-sized USB transfer buffers but had no sanity
checks on the sizes. This can lead to zero-size-pointer dereferences or
overflowed transfer buffers in ni6501_port_command() and
ni6501_counter_command() if a (malicious) device has smaller max-packet
sizes than expected (or when doing descriptor fuzz testing).

Add the missing sanity checks to probe().

Fixes: a03bb00e50ab ("staging: comedi: add NI USB-6501 support")
Cc: stable@vger.kernel.org      # 3.18
Cc: Luca Ellero <luca.ellero@brickedbrain.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/comedi/drivers/ni_usb6501.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/comedi/drivers/ni_usb6501.c b/drivers/comedi/drivers/ni_usb6501.c
index 5b6d9d783b2f..c42987b74b1d 100644
--- a/drivers/comedi/drivers/ni_usb6501.c
+++ b/drivers/comedi/drivers/ni_usb6501.c
@@ -144,6 +144,10 @@ static const u8 READ_COUNTER_RESPONSE[]	= {0x00, 0x01, 0x00, 0x10,
 					   0x00, 0x00, 0x00, 0x02,
 					   0x00, 0x00, 0x00, 0x00};
 
+/* Largest supported packets */
+static const size_t TX_MAX_SIZE	= sizeof(SET_PORT_DIR_REQUEST);
+static const size_t RX_MAX_SIZE	= sizeof(READ_PORT_RESPONSE);
+
 enum commands {
 	READ_PORT,
 	WRITE_PORT,
@@ -501,6 +505,12 @@ static int ni6501_find_endpoints(struct comedi_device *dev)
 	if (!devpriv->ep_rx || !devpriv->ep_tx)
 		return -ENODEV;
 
+	if (usb_endpoint_maxp(devpriv->ep_rx) < RX_MAX_SIZE)
+		return -ENODEV;
+
+	if (usb_endpoint_maxp(devpriv->ep_tx) < TX_MAX_SIZE)
+		return -ENODEV;
+
 	return 0;
 }
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] comedi: dt9812: fix DMA buffers on stack
  2021-10-27  9:35 [PATCH v2 0/2] comedi: misc fixes Johan Hovold
  2021-10-27  9:35 ` [PATCH v2 1/2] comedi: ni_usb6501: fix NULL-deref in command paths Johan Hovold
@ 2021-10-27  9:35 ` Johan Hovold
  2021-10-28  9:45   ` Ian Abbott
  1 sibling, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2021-10-27  9:35 UTC (permalink / raw)
  To: Ian Abbott, H Hartley Sweeten
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Johan Hovold, stable

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
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/comedi/drivers/dt9812.c | 115 ++++++++++++++++++++++++--------
 1 file changed, 86 insertions(+), 29 deletions(-)

diff --git a/drivers/comedi/drivers/dt9812.c b/drivers/comedi/drivers/dt9812.c
index 634f57730c1e..704b04d2980d 100644
--- a/drivers/comedi/drivers/dt9812.c
+++ b/drivers/comedi/drivers/dt9812.c
@@ -32,6 +32,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"
@@ -237,22 +238,42 @@ static int dt9812_read_info(struct comedi_device *dev,
 {
 	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,
@@ -261,22 +282,42 @@ static int dt9812_read_multiple_registers(struct comedi_device *dev,
 {
 	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;
 
-	cmd.cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG);
-	cmd.u.read_multi_info.count = reg_count;
+	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;
 	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;
+
+	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 usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr),
-			    value, reg_count, &count, DT9812_USB_TIMEOUT);
+	return ret;
 }
 
 static int dt9812_write_multiple_registers(struct comedi_device *dev,
@@ -285,19 +326,27 @@ static int dt9812_write_multiple_registers(struct comedi_device *dev,
 {
 	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_W_MULTI_BYTE_REG);
-	cmd.u.read_multi_info.count = reg_count;
+	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,
@@ -306,17 +355,25 @@ static int dt9812_rmw_multiple_registers(struct comedi_device *dev,
 {
 	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)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] comedi: ni_usb6501: fix NULL-deref in command paths
  2021-10-27  9:35 ` [PATCH v2 1/2] comedi: ni_usb6501: fix NULL-deref in command paths Johan Hovold
@ 2021-10-28  9:44   ` Ian Abbott
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Abbott @ 2021-10-28  9:44 UTC (permalink / raw)
  To: Johan Hovold, H Hartley Sweeten
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, stable, Luca Ellero

On 27/10/2021 10:35, Johan Hovold wrote:
> The driver uses endpoint-sized USB transfer buffers but had no sanity
> checks on the sizes. This can lead to zero-size-pointer dereferences or
> overflowed transfer buffers in ni6501_port_command() and
> ni6501_counter_command() if a (malicious) device has smaller max-packet
> sizes than expected (or when doing descriptor fuzz testing).
> 
> Add the missing sanity checks to probe().
> 
> Fixes: a03bb00e50ab ("staging: comedi: add NI USB-6501 support")
> Cc: stable@vger.kernel.org      # 3.18
> Cc: Luca Ellero <luca.ellero@brickedbrain.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>   drivers/comedi/drivers/ni_usb6501.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/comedi/drivers/ni_usb6501.c b/drivers/comedi/drivers/ni_usb6501.c
> index 5b6d9d783b2f..c42987b74b1d 100644
> --- a/drivers/comedi/drivers/ni_usb6501.c
> +++ b/drivers/comedi/drivers/ni_usb6501.c
> @@ -144,6 +144,10 @@ static const u8 READ_COUNTER_RESPONSE[]	= {0x00, 0x01, 0x00, 0x10,
>   					   0x00, 0x00, 0x00, 0x02,
>   					   0x00, 0x00, 0x00, 0x00};
>   
> +/* Largest supported packets */
> +static const size_t TX_MAX_SIZE	= sizeof(SET_PORT_DIR_REQUEST);
> +static const size_t RX_MAX_SIZE	= sizeof(READ_PORT_RESPONSE);
> +
>   enum commands {
>   	READ_PORT,
>   	WRITE_PORT,
> @@ -501,6 +505,12 @@ static int ni6501_find_endpoints(struct comedi_device *dev)
>   	if (!devpriv->ep_rx || !devpriv->ep_tx)
>   		return -ENODEV;
>   
> +	if (usb_endpoint_maxp(devpriv->ep_rx) < RX_MAX_SIZE)
> +		return -ENODEV;
> +
> +	if (usb_endpoint_maxp(devpriv->ep_tx) < TX_MAX_SIZE)
> +		return -ENODEV;
> +
>   	return 0;
>   }
>   
> 

Looks good, thanks!

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] comedi: dt9812: fix DMA buffers on stack
  2021-10-27  9:35 ` [PATCH v2 2/2] comedi: dt9812: fix DMA buffers on stack Johan Hovold
@ 2021-10-28  9:45   ` Ian Abbott
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Abbott @ 2021-10-28  9:45 UTC (permalink / raw)
  To: Johan Hovold, H Hartley Sweeten
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, stable

On 27/10/2021 10:35, Johan Hovold wrote:
> 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
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>   drivers/comedi/drivers/dt9812.c | 115 ++++++++++++++++++++++++--------
>   1 file changed, 86 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/comedi/drivers/dt9812.c b/drivers/comedi/drivers/dt9812.c
> index 634f57730c1e..704b04d2980d 100644
> --- a/drivers/comedi/drivers/dt9812.c
> +++ b/drivers/comedi/drivers/dt9812.c
> @@ -32,6 +32,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"
> @@ -237,22 +238,42 @@ static int dt9812_read_info(struct comedi_device *dev,
>   {
>   	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,
> @@ -261,22 +282,42 @@ static int dt9812_read_multiple_registers(struct comedi_device *dev,
>   {
>   	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;
>   
> -	cmd.cmd = cpu_to_le32(DT9812_R_MULTI_BYTE_REG);
> -	cmd.u.read_multi_info.count = reg_count;
> +	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;
>   	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;
> +
> +	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 usb_bulk_msg(usb, usb_rcvbulkpipe(usb, devpriv->cmd_rd.addr),
> -			    value, reg_count, &count, DT9812_USB_TIMEOUT);
> +	return ret;
>   }
>   
>   static int dt9812_write_multiple_registers(struct comedi_device *dev,
> @@ -285,19 +326,27 @@ static int dt9812_write_multiple_registers(struct comedi_device *dev,
>   {
>   	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_W_MULTI_BYTE_REG);
> -	cmd.u.read_multi_info.count = reg_count;
> +	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,
> @@ -306,17 +355,25 @@ static int dt9812_rmw_multiple_registers(struct comedi_device *dev,
>   {
>   	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)
> 

Looks great, thanks!

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company  )=-
-=( registered in England & Wales.  Regd. number: 02862268.  )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-10-28  9:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-27  9:35 [PATCH v2 0/2] comedi: misc fixes Johan Hovold
2021-10-27  9:35 ` [PATCH v2 1/2] comedi: ni_usb6501: fix NULL-deref in command paths Johan Hovold
2021-10-28  9:44   ` Ian Abbott
2021-10-27  9:35 ` [PATCH v2 2/2] comedi: dt9812: fix DMA buffers on stack Johan Hovold
2021-10-28  9:45   ` Ian Abbott

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.