linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] media: uvcvideo: Fix OOB read
@ 2023-07-20 17:46 Ricardo Ribalda
  2023-07-20 21:47 ` Sergey Senozhatsky
  2023-07-25 21:34 ` Laurent Pinchart
  0 siblings, 2 replies; 11+ messages in thread
From: Ricardo Ribalda @ 2023-07-20 17:46 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Sergey Senozhatsky, stable,
	Zubin Mithra, Ricardo Ribalda

If the index provided by the user is bigger than the mask size, we might do an
out of bound read.

CC: stable@kernel.org
Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
Reported-by: Zubin Mithra <zsm@chromium.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Avoid reading index >= 31
---
Changes in v2:
- Use BITS_PER_TYPE instead of 32 (thanks Sergey).
- Add Reported-by tag.
- Link to v1: https://lore.kernel.org/r/20230717-uvc-oob-v1-1-f5b9b4aba3b4@chromium.org
---
 drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 5e9d3da862dd..e59a463c2761 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1402,6 +1402,9 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
 	query_menu->id = id;
 	query_menu->index = index;
 
+	if (index >= BITS_PER_TYPE(mapping->menu_mask))
+		return -EINVAL;
+
 	ret = mutex_lock_interruptible(&chain->ctrl_mutex);
 	if (ret < 0)
 		return -ERESTARTSYS;

---
base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
change-id: 20230717-uvc-oob-4b0148a00417

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>


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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-20 17:46 [PATCH v2] media: uvcvideo: Fix OOB read Ricardo Ribalda
@ 2023-07-20 21:47 ` Sergey Senozhatsky
  2023-07-25 21:34 ` Laurent Pinchart
  1 sibling, 0 replies; 11+ messages in thread
From: Sergey Senozhatsky @ 2023-07-20 21:47 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
	linux-kernel, Sergey Senozhatsky, stable, Zubin Mithra

On (23/07/20 17:46), Ricardo Ribalda wrote:
> 
> If the index provided by the user is bigger than the mask size, we might do an
> out of bound read.
> 
> CC: stable@kernel.org
> Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> Reported-by: Zubin Mithra <zsm@chromium.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org>

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-20 17:46 [PATCH v2] media: uvcvideo: Fix OOB read Ricardo Ribalda
  2023-07-20 21:47 ` Sergey Senozhatsky
@ 2023-07-25 21:34 ` Laurent Pinchart
  2023-07-26  6:24   ` Ricardo Ribalda
  1 sibling, 1 reply; 11+ messages in thread
From: Laurent Pinchart @ 2023-07-25 21:34 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel,
	Sergey Senozhatsky, stable, Zubin Mithra

Hi Ricardo,

Thank you for the patch.

On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
> If the index provided by the user is bigger than the mask size, we might do an
> out of bound read.
> 
> CC: stable@kernel.org
> Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> Reported-by: Zubin Mithra <zsm@chromium.org>

checkpatch now requests a Reported-by tag to be immediately followed by
a Closes tag that contains the URL to the report. Could you please
provide that ?

> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Avoid reading index >= 31
> ---
> Changes in v2:
> - Use BITS_PER_TYPE instead of 32 (thanks Sergey).
> - Add Reported-by tag.
> - Link to v1: https://lore.kernel.org/r/20230717-uvc-oob-v1-1-f5b9b4aba3b4@chromium.org
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 5e9d3da862dd..e59a463c2761 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1402,6 +1402,9 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
>  	query_menu->id = id;
>  	query_menu->index = index;
>  
> +	if (index >= BITS_PER_TYPE(mapping->menu_mask))
> +		return -EINVAL;
> +

I'd move this a few lines up, before setting query_menu.

With those minor changes,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

There's no need for a v3, I can handle the changes locally, but I need
the URL for the Closes tag.

>  	ret = mutex_lock_interruptible(&chain->ctrl_mutex);
>  	if (ret < 0)
>  		return -ERESTARTSYS;
> 
> ---
> base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
> change-id: 20230717-uvc-oob-4b0148a00417

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-25 21:34 ` Laurent Pinchart
@ 2023-07-26  6:24   ` Ricardo Ribalda
  2023-07-26  8:07     ` Laurent Pinchart
  0 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda @ 2023-07-26  6:24 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel,
	Sergey Senozhatsky, stable, Zubin Mithra

Hi Laurent

Thanks for the review!

On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
> > If the index provided by the user is bigger than the mask size, we might do an
> > out of bound read.
> >
> > CC: stable@kernel.org
> > Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> > Reported-by: Zubin Mithra <zsm@chromium.org>
>
> checkpatch now requests a Reported-by tag to be immediately followed by
> a Closes tag that contains the URL to the report. Could you please
> provide that ?
>
I saw that, but the URL is kind of private:

Closes: http://issuetracker.google.com/issues/289975230

> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > Avoid reading index >= 31
> > ---
> > Changes in v2:
> > - Use BITS_PER_TYPE instead of 32 (thanks Sergey).
> > - Add Reported-by tag.
> > - Link to v1: https://lore.kernel.org/r/20230717-uvc-oob-v1-1-f5b9b4aba3b4@chromium.org
> > ---
> >  drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 5e9d3da862dd..e59a463c2761 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -1402,6 +1402,9 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> >       query_menu->id = id;
> >       query_menu->index = index;
> >
> > +     if (index >= BITS_PER_TYPE(mapping->menu_mask))
> > +             return -EINVAL;
> > +
>
> I'd move this a few lines up, before setting query_menu.
>

SGTM, I just wanted to clear all the fields to mimic the other error
paths of the function.

> With those minor changes,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> There's no need for a v3, I can handle the changes locally, but I need
> the URL for the Closes tag.
>
> >       ret = mutex_lock_interruptible(&chain->ctrl_mutex);
> >       if (ret < 0)
> >               return -ERESTARTSYS;
> >
> > ---
> > base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
> > change-id: 20230717-uvc-oob-4b0148a00417
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-26  6:24   ` Ricardo Ribalda
@ 2023-07-26  8:07     ` Laurent Pinchart
  2023-07-26  8:33       ` Thorsten Leemhuis
  2023-07-26  8:37       ` Ricardo Ribalda
  0 siblings, 2 replies; 11+ messages in thread
From: Laurent Pinchart @ 2023-07-26  8:07 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel,
	Sergey Senozhatsky, stable, Zubin Mithra, Kai Wasserbäch,
	Thorsten Leemhuis

Hi Ricardo,

(CC'ing Kai and Thorsten who have added the check to checkpatch)

On Wed, Jul 26, 2023 at 08:24:50AM +0200, Ricardo Ribalda wrote:
> On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart wrote:
> > On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
> > > If the index provided by the user is bigger than the mask size, we might do an
> > > out of bound read.
> > >
> > > CC: stable@kernel.org
> > > Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> > > Reported-by: Zubin Mithra <zsm@chromium.org>
> >
> > checkpatch now requests a Reported-by tag to be immediately followed by
> > a Closes tag that contains the URL to the report. Could you please
> > provide that ?
>
> I saw that, but the URL is kind of private:
> 
> Closes: http://issuetracker.google.com/issues/289975230

Ah :-S I wonder if we should drop the Reported-by tag then ?

> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > > Avoid reading index >= 31
> > > ---
> > > Changes in v2:
> > > - Use BITS_PER_TYPE instead of 32 (thanks Sergey).
> > > - Add Reported-by tag.
> > > - Link to v1: https://lore.kernel.org/r/20230717-uvc-oob-v1-1-f5b9b4aba3b4@chromium.org
> > > ---
> > >  drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > index 5e9d3da862dd..e59a463c2761 100644
> > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > @@ -1402,6 +1402,9 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > >       query_menu->id = id;
> > >       query_menu->index = index;
> > >
> > > +     if (index >= BITS_PER_TYPE(mapping->menu_mask))
> > > +             return -EINVAL;
> > > +
> >
> > I'd move this a few lines up, before setting query_menu.
> 
> SGTM, I just wanted to clear all the fields to mimic the other error
> paths of the function.

I'm fine with that too if you prefer.

> > With those minor changes,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > There's no need for a v3, I can handle the changes locally, but I need
> > the URL for the Closes tag.
> >
> > >       ret = mutex_lock_interruptible(&chain->ctrl_mutex);
> > >       if (ret < 0)
> > >               return -ERESTARTSYS;
> > >
> > > ---
> > > base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
> > > change-id: 20230717-uvc-oob-4b0148a00417

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-26  8:07     ` Laurent Pinchart
@ 2023-07-26  8:33       ` Thorsten Leemhuis
  2023-07-26  8:38         ` Ricardo Ribalda
  2023-07-26  8:37       ` Ricardo Ribalda
  1 sibling, 1 reply; 11+ messages in thread
From: Thorsten Leemhuis @ 2023-07-26  8:33 UTC (permalink / raw)
  To: Laurent Pinchart, Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel,
	Sergey Senozhatsky, stable, Zubin Mithra, Kai Wasserbäch

On 26.07.23 10:07, Laurent Pinchart wrote:
> (CC'ing Kai and Thorsten who have added the check to checkpatch)
> 
> On Wed, Jul 26, 2023 at 08:24:50AM +0200, Ricardo Ribalda wrote:
>> On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart wrote:
>>> On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
>>>> If the index provided by the user is bigger than the mask size, we might do an
>>>> out of bound read.
>>>>
>>>> CC: stable@kernel.org
>>>> Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
>>>> Reported-by: Zubin Mithra <zsm@chromium.org>
>>>
>>> checkpatch now requests a Reported-by tag to be immediately followed by
>>> a Closes

Not that it matters, the changes I performed only required a Link: tag,
which is how things should have been done for many years already. It
later became Closes: due to patches from Matthieu. But whatever. :-D

>>> tag that contains the URL to the report. Could you please
>>> provide that ?
>> I saw that, but the URL is kind of private:
>> Closes: http://issuetracker.google.com/issues/289975230
> Ah :-S I wonder if we should drop the Reported-by tag then ?

That's what I do, unless the reporter granted his permission. To quote
Documentation/process/5.Posting.rst : ```Be careful in the addition of
tags to your patches, as only Cc: is appropriate for addition without
the explicit permission of the person named; using Reported-by: is fine
most of the time as well, but ask for permission if the bug was reported
in private.```

I heard of on instance where a GDPR complaint was filed due to a
Reported-by: tag. So maybe that part should be even revisited reg. the
Cc: aspect. :-/

Ciao, Thorsten

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-26  8:07     ` Laurent Pinchart
  2023-07-26  8:33       ` Thorsten Leemhuis
@ 2023-07-26  8:37       ` Ricardo Ribalda
  1 sibling, 0 replies; 11+ messages in thread
From: Ricardo Ribalda @ 2023-07-26  8:37 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, linux-media, linux-kernel,
	Sergey Senozhatsky, stable, Zubin Mithra, Kai Wasserbäch,
	Thorsten Leemhuis

Hi Laurent

On Wed, 26 Jul 2023 at 10:07, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> (CC'ing Kai and Thorsten who have added the check to checkpatch)
>
> On Wed, Jul 26, 2023 at 08:24:50AM +0200, Ricardo Ribalda wrote:
> > On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart wrote:
> > > On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
> > > > If the index provided by the user is bigger than the mask size, we might do an
> > > > out of bound read.
> > > >
> > > > CC: stable@kernel.org
> > > > Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> > > > Reported-by: Zubin Mithra <zsm@chromium.org>
> > >
> > > checkpatch now requests a Reported-by tag to be immediately followed by
> > > a Closes tag that contains the URL to the report. Could you please
> > > provide that ?
> >
> > I saw that, but the URL is kind of private:
> >
> > Closes: http://issuetracker.google.com/issues/289975230
>
> Ah :-S I wonder if we should drop the Reported-by tag then ?
>
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > > Avoid reading index >= 31
> > > > ---
> > > > Changes in v2:
> > > > - Use BITS_PER_TYPE instead of 32 (thanks Sergey).
> > > > - Add Reported-by tag.
> > > > - Link to v1: https://lore.kernel.org/r/20230717-uvc-oob-v1-1-f5b9b4aba3b4@chromium.org
> > > > ---
> > > >  drivers/media/usb/uvc/uvc_ctrl.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > index 5e9d3da862dd..e59a463c2761 100644
> > > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > > @@ -1402,6 +1402,9 @@ int uvc_query_v4l2_menu(struct uvc_video_chain *chain,
> > > >       query_menu->id = id;
> > > >       query_menu->index = index;
> > > >
> > > > +     if (index >= BITS_PER_TYPE(mapping->menu_mask))
> > > > +             return -EINVAL;
> > > > +
> > >
> > > I'd move this a few lines up, before setting query_menu.
> >
> > SGTM, I just wanted to clear all the fields to mimic the other error
> > paths of the function.
>
> I'm fine with that too if you prefer.

Your call. I prefer my version, but I am of course biased :P

>
> > > With those minor changes,
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > There's no need for a v3, I can handle the changes locally, but I need
> > > the URL for the Closes tag.
> > >
> > > >       ret = mutex_lock_interruptible(&chain->ctrl_mutex);
> > > >       if (ret < 0)
> > > >               return -ERESTARTSYS;
> > > >
> > > > ---
> > > > base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
> > > > change-id: 20230717-uvc-oob-4b0148a00417
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-26  8:33       ` Thorsten Leemhuis
@ 2023-07-26  8:38         ` Ricardo Ribalda
  2023-07-26  8:47           ` Thorsten Leemhuis
  0 siblings, 1 reply; 11+ messages in thread
From: Ricardo Ribalda @ 2023-07-26  8:38 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
	linux-kernel, Sergey Senozhatsky, stable, Zubin Mithra,
	Kai Wasserbäch

Hi Thorsten

On Wed, 26 Jul 2023 at 10:33, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>
> On 26.07.23 10:07, Laurent Pinchart wrote:
> > (CC'ing Kai and Thorsten who have added the check to checkpatch)
> >
> > On Wed, Jul 26, 2023 at 08:24:50AM +0200, Ricardo Ribalda wrote:
> >> On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart wrote:
> >>> On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
> >>>> If the index provided by the user is bigger than the mask size, we might do an
> >>>> out of bound read.
> >>>>
> >>>> CC: stable@kernel.org
> >>>> Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> >>>> Reported-by: Zubin Mithra <zsm@chromium.org>
> >>>
> >>> checkpatch now requests a Reported-by tag to be immediately followed by
> >>> a Closes
>
> Not that it matters, the changes I performed only required a Link: tag,
> which is how things should have been done for many years already. It
> later became Closes: due to patches from Matthieu. But whatever. :-D
>

I prefer to leave the Reported-by and remove the Closes, that way we
credit the reporter (assuming they approved to be referred).

But if that is not possible, just remove the reported-by. A private
link is pretty much noise on the tree.

Thanks!

> >>> tag that contains the URL to the report. Could you please
> >>> provide that ?
> >> I saw that, but the URL is kind of private:
> >> Closes: http://issuetracker.google.com/issues/289975230
> > Ah :-S I wonder if we should drop the Reported-by tag then ?
>
> That's what I do, unless the reporter granted his permission. To quote
> Documentation/process/5.Posting.rst : ```Be careful in the addition of
> tags to your patches, as only Cc: is appropriate for addition without
> the explicit permission of the person named; using Reported-by: is fine
> most of the time as well, but ask for permission if the bug was reported
> in private.```
>
> I heard of on instance where a GDPR complaint was filed due to a
> Reported-by: tag. So maybe that part should be even revisited reg. the
> Cc: aspect. :-/
>
> Ciao, Thorsten



-- 
Ricardo Ribalda

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-26  8:38         ` Ricardo Ribalda
@ 2023-07-26  8:47           ` Thorsten Leemhuis
  2023-07-26 13:43             ` Zubin Mithra
  2023-07-26 15:19             ` Laurent Pinchart
  0 siblings, 2 replies; 11+ messages in thread
From: Thorsten Leemhuis @ 2023-07-26  8:47 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Laurent Pinchart, Mauro Carvalho Chehab, linux-media,
	linux-kernel, Sergey Senozhatsky, stable, Zubin Mithra,
	Kai Wasserbäch

On 26.07.23 10:38, Ricardo Ribalda wrote:
> On Wed, 26 Jul 2023 at 10:33, Thorsten Leemhuis <linux@leemhuis.info> wrote:
>> On 26.07.23 10:07, Laurent Pinchart wrote:
>>> (CC'ing Kai and Thorsten who have added the check to checkpatch)
>>>
>>> On Wed, Jul 26, 2023 at 08:24:50AM +0200, Ricardo Ribalda wrote:
>>>> On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart wrote:
>>>>> On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
>>>>>> If the index provided by the user is bigger than the mask size, we might do an
>>>>>> out of bound read.
>>>>>>
>>>>>> CC: stable@kernel.org
>>>>>> Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
>>>>>> Reported-by: Zubin Mithra <zsm@chromium.org>
>>>>>
>>>>> checkpatch now requests a Reported-by tag to be immediately followed by
>>>>> a Closes
>>
>> Not that it matters, the changes I performed only required a Link: tag,
>> which is how things should have been done for many years already. It
>> later became Closes: due to patches from Matthieu. But whatever. :-D
> 
> I prefer to leave the Reported-by and remove the Closes, that way we
> credit the reporter (assuming they approved to be referred).
> 
> But if that is not possible, just remove the reported-by. A private
> link is pretty much noise on the tree.

Yeah, of course that's the right strategy (Linus made it pretty clear
that he doesn't want any private links) in case the reporter okay with
the Reported-by. Sorry, forgot to cover that case in my reply.

Ciao, Thorsten

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-26  8:47           ` Thorsten Leemhuis
@ 2023-07-26 13:43             ` Zubin Mithra
  2023-07-26 15:19             ` Laurent Pinchart
  1 sibling, 0 replies; 11+ messages in thread
From: Zubin Mithra @ 2023-07-26 13:43 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
	linux-media, linux-kernel, Sergey Senozhatsky, stable,
	Kai Wasserbäch

On Wed, Jul 26, 2023 at 10:47:46AM +0200, Thorsten Leemhuis wrote:
> On 26.07.23 10:38, Ricardo Ribalda wrote:
> > On Wed, 26 Jul 2023 at 10:33, Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >> On 26.07.23 10:07, Laurent Pinchart wrote:
> >>> (CC'ing Kai and Thorsten who have added the check to checkpatch)
> >>>
> >>> On Wed, Jul 26, 2023 at 08:24:50AM +0200, Ricardo Ribalda wrote:
> >>>> On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart wrote:
> >>>>> On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
> >>>>>> If the index provided by the user is bigger than the mask size, we might do an
> >>>>>> out of bound read.
> >>>>>>
> >>>>>> CC: stable@kernel.org
> >>>>>> Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> >>>>>> Reported-by: Zubin Mithra <zsm@chromium.org>
> >>>>>
> >>>>> checkpatch now requests a Reported-by tag to be immediately followed by
> >>>>> a Closes
> >>
> >> Not that it matters, the changes I performed only required a Link: tag,
> >> which is how things should have been done for many years already. It
> >> later became Closes: due to patches from Matthieu. But whatever. :-D
> > 
> > I prefer to leave the Reported-by and remove the Closes, that way we
> > credit the reporter (assuming they approved to be referred).
> > 
> > But if that is not possible, just remove the reported-by. A private
> > link is pretty much noise on the tree.
> 
> Yeah, of course that's the right strategy (Linus made it pretty clear
> that he doesn't want any private links) in case the reporter okay with
> the Reported-by. Sorry, forgot to cover that case in my reply.
> 

I don't have a preference either way. Please feel free to remove the
reported-by tag.

Thanks,
- Zubin

> Ciao, Thorsten

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

* Re: [PATCH v2] media: uvcvideo: Fix OOB read
  2023-07-26  8:47           ` Thorsten Leemhuis
  2023-07-26 13:43             ` Zubin Mithra
@ 2023-07-26 15:19             ` Laurent Pinchart
  1 sibling, 0 replies; 11+ messages in thread
From: Laurent Pinchart @ 2023-07-26 15:19 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Ricardo Ribalda, Mauro Carvalho Chehab, linux-media,
	linux-kernel, Sergey Senozhatsky, stable, Zubin Mithra,
	Kai Wasserbäch

On Wed, Jul 26, 2023 at 10:47:46AM +0200, Thorsten Leemhuis wrote:
> On 26.07.23 10:38, Ricardo Ribalda wrote:
> > On Wed, 26 Jul 2023 at 10:33, Thorsten Leemhuis <linux@leemhuis.info> wrote:
> >> On 26.07.23 10:07, Laurent Pinchart wrote:
> >>> (CC'ing Kai and Thorsten who have added the check to checkpatch)
> >>>
> >>> On Wed, Jul 26, 2023 at 08:24:50AM +0200, Ricardo Ribalda wrote:
> >>>> On Tue, 25 Jul 2023 at 23:34, Laurent Pinchart wrote:
> >>>>> On Thu, Jul 20, 2023 at 05:46:54PM +0000, Ricardo Ribalda wrote:
> >>>>>> If the index provided by the user is bigger than the mask size, we might do an
> >>>>>> out of bound read.
> >>>>>>
> >>>>>> CC: stable@kernel.org
> >>>>>> Fixes: 40140eda661e ("media: uvcvideo: Implement mask for V4L2_CTRL_TYPE_MENU")
> >>>>>> Reported-by: Zubin Mithra <zsm@chromium.org>
> >>>>>
> >>>>> checkpatch now requests a Reported-by tag to be immediately followed by
> >>>>> a Closes
> >>
> >> Not that it matters, the changes I performed only required a Link: tag,
> >> which is how things should have been done for many years already. It
> >> later became Closes: due to patches from Matthieu. But whatever. :-D
> > 
> > I prefer to leave the Reported-by and remove the Closes, that way we
> > credit the reporter (assuming they approved to be referred).
> > 
> > But if that is not possible, just remove the reported-by. A private
> > link is pretty much noise on the tree.
> 
> Yeah, of course that's the right strategy (Linus made it pretty clear
> that he doesn't want any private links) in case the reporter okay with
> the Reported-by. Sorry, forgot to cover that case in my reply.

I'll keep the Reported-by and omit the Link/Closes tags.

-- 
Regards,

Laurent Pinchart

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

end of thread, other threads:[~2023-07-26 15:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-20 17:46 [PATCH v2] media: uvcvideo: Fix OOB read Ricardo Ribalda
2023-07-20 21:47 ` Sergey Senozhatsky
2023-07-25 21:34 ` Laurent Pinchart
2023-07-26  6:24   ` Ricardo Ribalda
2023-07-26  8:07     ` Laurent Pinchart
2023-07-26  8:33       ` Thorsten Leemhuis
2023-07-26  8:38         ` Ricardo Ribalda
2023-07-26  8:47           ` Thorsten Leemhuis
2023-07-26 13:43             ` Zubin Mithra
2023-07-26 15:19             ` Laurent Pinchart
2023-07-26  8:37       ` Ricardo Ribalda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).