[PATCH v2] staging: comedi: drivers: usbduxsigma.c: fix DMA buffers on stack

[PATCH v2] staging: comedi: drivers: usbduxsigma.c: fix DMA buffers on stack

[Kumar Amit Mehta]

This patch fixes an instance of DMA buffer on stack(being passed to
usb_control_msg)for the USB-DUXsigma Board driver. Found using smatch.

Signed-off-by: Kumar Amit Mehta <gmate.amit@gmail.com>
---
 drivers/staging/comedi/drivers/usbduxsigma.c |   27 ++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c b/drivers/staging/comedi/drivers/usbduxsigma.c
index dc6b017..9e99a4b 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -681,7 +681,11 @@ static void usbduxsub_ao_IsocIrq(struct urb *urb)
 static int usbduxsub_start(struct usbduxsub *usbduxsub)
 {
 	int errcode = 0;
-	uint8_t local_transfer_buffer[16];
+	uint8_t *local_transfer_buffer;
+
+	local_transfer_buffer = kmalloc(16, GFP_KERNEL);
+	if (!local_transfer_buffer)
+		return -ENOMEM;
 
 	/* 7f92 to zero */
 	local_transfer_buffer[0] = 0;
@@ -702,19 +706,22 @@ static int usbduxsub_start(struct usbduxsub *usbduxsub)
 				  1,
 				  /* Timeout */
 				  BULK_TIMEOUT);
-	if (errcode < 0) {
+	if (errcode < 0)
 		dev_err(&usbduxsub->interface->dev,
 			"comedi_: control msg failed (start)\n");
-		return errcode;
-	}
-	return 0;
+
+	kfree(local_transfer_buffer);
+	return errcode;
 }
 
 static int usbduxsub_stop(struct usbduxsub *usbduxsub)
 {
 	int errcode = 0;
+	uint8_t *local_transfer_buffer;
 
-	uint8_t local_transfer_buffer[16];
+	local_transfer_buffer = kmalloc(16, GFP_KERNEL);
+	if (!local_transfer_buffer)
+		return -ENOMEM;
 
 	/* 7f92 to one */
 	local_transfer_buffer[0] = 1;
@@ -732,12 +739,12 @@ static int usbduxsub_stop(struct usbduxsub *usbduxsub)
 				  1,
 				  /* Timeout */
 				  BULK_TIMEOUT);
-	if (errcode < 0) {
+	if (errcode < 0)
 		dev_err(&usbduxsub->interface->dev,
 			"comedi_: control msg failed (stop)\n");
-		return errcode;
-	}
-	return 0;
+
+	kfree(local_transfer_buffer);
+	return errcode;
 }
 
 static int usbduxsub_upload(struct usbduxsub *usbduxsub,
-- 
1.7.9.5

Re: [PATCH v2] staging: comedi: drivers: usbduxsigma.c: fix DMA buffers on stack

[Dan Carpenter]

Looks good.

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH v2] staging: comedi: drivers: usbduxsigma.c: fix DMA buffers on stack

[Ian Abbott]

On 2013-02-22 06:07, Kumar Amit Mehta wrote:
> This patch fixes an instance of DMA buffer on stack(being passed to
> usb_control_msg)for the USB-DUXsigma Board driver. Found using smatch.
>

Looks good here too.

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

Would you mind doing the same for usbdux.c and usbduxfast.c?

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH v2] staging: comedi: drivers: usbduxsigma.c: fix DMA buffers on stack

[Kumar amit mehta]

On Fri, Feb 22, 2013 at 10:02:44AM +0000, Ian Abbott wrote:
> On 2013-02-22 06:07, Kumar Amit Mehta wrote:
> >This patch fixes an instance of DMA buffer on stack(being passed to
> >usb_control_msg)for the USB-DUXsigma Board driver. Found using smatch.
> >
> 
> Looks good here too.
> 
> Reviewed-by: Ian Abbott <abbotti@mev.co.uk>
> 
> Would you mind doing the same for usbdux.c and usbduxfast.c?
Sure. 
As pointed out by Dan, Do you really want the buffer to be of 16 bytes, as we
are using only the first byte and passing the length as '1' in:
usbduxsub_start, usbduxsub_stop, usbduxfastsub_start, usbduxfastsub_stop ?

~Amit

Re: [PATCH v2] staging: comedi: drivers: usbduxsigma.c: fix DMA buffers on stack

[Ian Abbott]

On 2013-02-22 14:26, Kumar amit mehta wrote:
> On Fri, Feb 22, 2013 at 10:02:44AM +0000, Ian Abbott wrote:
>> On 2013-02-22 06:07, Kumar Amit Mehta wrote:
>>> This patch fixes an instance of DMA buffer on stack(being passed to
>>> usb_control_msg)for the USB-DUXsigma Board driver. Found using smatch.
>>>
>>
>> Looks good here too.
>>
>> Reviewed-by: Ian Abbott <abbotti@mev.co.uk>
>>
>> Would you mind doing the same for usbdux.c and usbduxfast.c?
> Sure.
> As pointed out by Dan, Do you really want the buffer to be of 16 bytes, as we
> are using only the first byte and passing the length as '1' in:
> usbduxsub_start, usbduxsub_stop, usbduxfastsub_start, usbduxfastsub_stop ?

A buffer size of 1 should be fine.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-