Hi Mauro,
On Mon, Jun 21, 2021 at 04:34:08PM +0200, Mauro Carvalho Chehab wrote:
Em Mon, 21 Jun 2021 17:11:46 +0300 Laurent Pinchart escreveu:
On Mon, Jun 21, 2021 at 03:40:19PM +0200, Mauro Carvalho Chehab wrote:
As warned by smatch: drivers/media/usb/uvc/uvc_v4l2.c:911 uvc_ioctl_g_input() error: doing dma on the stack (&i) drivers/media/usb/uvc/uvc_v4l2.c:943 uvc_ioctl_s_input() error: doing dma on the stack (&i)
those two functions call uvc_query_ctrl passing a pointer to a data at the DMA stack. those are used to send URBs via usb_control_msg(). Using DMA stack is not supported and should not work anymore on modern Linux versions.
So, use a kmalloc'ed buffer.
Cc: stable@vger.kernel.org # Kernel 4.9 and upper Signed-off-by: Mauro Carvalho Chehab mchehab+huawei@kernel.org
drivers/media/usb/uvc/uvc_v4l2.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 252136cc885c..a95bf7318848 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -899,8 +899,8 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, unsigned int *input) { struct uvc_fh *handle = fh; struct uvc_video_chain *chain = handle->chain;
- u8 *buf; int ret;
- u8 i;
if (chain->selector == NULL || (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) { @@ -908,13 +908,20 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, unsigned int *input) return 0; }
- buf = kmalloc(1, GFP_KERNEL);
- if (!buf)
return -ENOMEM;- ret = uvc_query_ctrl(chain->dev, UVC_GET_CUR, chain->selector->id, chain->dev->intfnum, UVC_SU_INPUT_SELECT_CONTROL,
&i, 1);
buf, 1);Memory leak :-)
Argh ;-)
Clearly, I'm needing more caffeine today, but it is too damn hot here...
if (!ret) *input = *buf - 1;
kfree(buf);
return ret;
- *input = i - 1;
- *input = *buf - 1;
- kfree(buf);
- return 0;
} @@ -922,8 +929,8 @@ static int uvc_ioctl_s_input(struct file *file, void *fh, unsigned int input) { struct uvc_fh *handle = fh; struct uvc_video_chain *chain = handle->chain;
- char *buf;
u8 *buf;
With these two changes,
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Thanks!
Do I need to take the patch in my tree ?
It is up to you.
I suspect that it would be easier to just merge it at media_stage, together with the other patches from the smatch series, but it is up to you.
Just let me know if you prefer to merge it via your tree, and I'll drop it from my queue, or otherwise I'll merge directly at media_stage, after waiting for a while on feedbacks on the remaining patches.
Please merge it directly, it's less work for me :-)