[PATCH] [media] usbtv: don't do DMA on stack

[PATCH] [media] usbtv: don't do DMA on stack

[Mauro Carvalho Chehab]

As reported by smatch:
	drivers/media/usb/usbtv/usbtv-video.c:716 usbtv_s_ctrl() error: doing dma on the stack (data)
	drivers/media/usb/usbtv/usbtv-video.c:758 usbtv_s_ctrl() error: doing dma on the stack (data)

We should not do it, as it won't work on Kernels 4.9 and upper.
So, alloc a buffer for it.

Fixes: c53a846c48f2 ("[media] usbtv: add video controls")
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
---
 drivers/media/usb/usbtv/usbtv-video.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/usbtv/usbtv-video.c b/drivers/media/usb/usbtv/usbtv-video.c
index 86ffbf8780f2..d3b6d3dfaa09 100644
--- a/drivers/media/usb/usbtv/usbtv-video.c
+++ b/drivers/media/usb/usbtv/usbtv-video.c
@@ -704,10 +704,14 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct usbtv *usbtv = container_of(ctrl->handler, struct usbtv,
 								ctrl);
-	u8 data[3];
+	u8 *data;
 	u16 index, size;
 	int ret;
 
+	data = kmalloc(3, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
 	/*
 	 * Read in the current brightness/contrast registers. We need them
 	 * both, because the values are for some reason interleaved.
@@ -717,6 +721,8 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
 			usb_sndctrlpipe(usbtv->udev, 0), USBTV_CONTROL_REG,
 			USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 			0, USBTV_BASE + 0x0244, (void *)data, 3, 0);
+		if (ret < 0)
+			goto error;
 	}
 
 	switch (ctrl->id) {
@@ -752,6 +758,7 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
 		}
 		break;
 	default:
+		kfree(data);
 		return -EINVAL;
 	}
 
@@ -759,12 +766,13 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
 			USBTV_CONTROL_REG,
 			USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 			0, index, (void *)data, size, 0);
-	if (ret < 0) {
+
+error:
+	if (ret < 0)
 		dev_warn(usbtv->dev, "Failed to submit a control request.\n");
-		return ret;
-	}
 
-	return 0;
+	kfree(data);
+	return ret;
 }
 
 static const struct v4l2_ctrl_ops usbtv_ctrl_ops = {
-- 
2.7.4

Re: [PATCH] [media] usbtv: don't do DMA on stack

[Lubomir Rintel]

On Wed, 2016-11-16 at 13:15 -0200, Mauro Carvalho Chehab wrote:
> As reported by smatch:
> 	drivers/media/usb/usbtv/usbtv-video.c:716 usbtv_s_ctrl() error:
> doing dma on the stack (data)
> 	drivers/media/usb/usbtv/usbtv-video.c:758 usbtv_s_ctrl() error:
> doing dma on the stack (data)
> 
> We should not do it, as it won't work on Kernels 4.9 and upper.
> So, alloc a buffer for it.
> 
> Fixes: c53a846c48f2 ("[media] usbtv: add video controls")
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  drivers/media/usb/usbtv/usbtv-video.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/usb/usbtv/usbtv-video.c
> b/drivers/media/usb/usbtv/usbtv-video.c
> index 86ffbf8780f2..d3b6d3dfaa09 100644
> --- a/drivers/media/usb/usbtv/usbtv-video.c
> +++ b/drivers/media/usb/usbtv/usbtv-video.c
> @@ -704,10 +704,14 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct usbtv *usbtv = container_of(ctrl->handler, struct
> usbtv,
>  								ctrl
> );
> -	u8 data[3];
> +	u8 *data;
>  	u16 index, size;
>  	int ret;
>  
> +	data = kmalloc(3, GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
>  	/*
>  	 * Read in the current brightness/contrast registers. We
> need them
>  	 * both, because the values are for some reason interleaved.
> @@ -717,6 +721,8 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
>  			usb_sndctrlpipe(usbtv->udev, 0),
> USBTV_CONTROL_REG,
>  			USB_DIR_OUT | USB_TYPE_VENDOR |
> USB_RECIP_DEVICE,
>  			0, USBTV_BASE + 0x0244, (void *)data, 3, 0);
> +		if (ret < 0)
> +			goto error;
>  	}
>  
>  	switch (ctrl->id) {
> @@ -752,6 +758,7 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
>  		}
>  		break;
>  	default:
> +		kfree(data);
>  		return -EINVAL;
>  	}
>  
> @@ -759,12 +766,13 @@ static int usbtv_s_ctrl(struct v4l2_ctrl *ctrl)
>  			USBTV_CONTROL_REG,
>  			USB_DIR_OUT | USB_TYPE_VENDOR |
> USB_RECIP_DEVICE,
>  			0, index, (void *)data, size, 0);
> -	if (ret < 0) {
> +
> +error:
> +	if (ret < 0)
>  		dev_warn(usbtv->dev, "Failed to submit a control
> request.\n");
> -		return ret;
> -	}
>  
> -	return 0;
> +	kfree(data);
> +	return ret;
>  }
>  
>  static const struct v4l2_ctrl_ops usbtv_ctrl_ops = {

Reviewed-by: Lubomir Rintel <lkundrak@v3.sk>

Thank you,
Lubo