* [PATCH 1/2] media: uvcvideo: Avoid invalid memory access
@ 2021-11-30 15:50 Ricardo Ribalda
2021-11-30 15:50 ` [PATCH 2/2] media: uvcvideo: Avoid returning invalid controls Ricardo Ribalda
2021-12-01 2:36 ` [PATCH 1/2] media: uvcvideo: Avoid invalid memory access Laurent Pinchart
0 siblings, 2 replies; 5+ messages in thread
From: Ricardo Ribalda @ 2021-11-30 15:50 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Hans Verkuil,
linux-media, linux-kernel
Cc: Ricardo Ribalda
If mappings points to an invalid memory, we will be invalid accessing
it.
Solve it by initializing the value of the variable mapping and by
changing the order in the conditional statement (to avoid accessing
mapping->id if not needed).
Fix:
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN NOPTI
Fixes: 6350d6a4ed487 ("media: uvcvideo: Set error_idx during ctrl_commit errors")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_ctrl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 30bfe9069a1fb..f7b7add3cfa59 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -852,8 +852,8 @@ static void __uvc_find_control(struct uvc_entity *entity, u32 v4l2_id,
return;
}
- if ((*mapping == NULL || (*mapping)->id > map->id) &&
- (map->id > v4l2_id) && next) {
+ if (next && (map->id > v4l2_id) &&
+ (*mapping == NULL || (*mapping)->id > map->id)) {
*control = ctrl;
*mapping = map;
}
@@ -1638,7 +1638,7 @@ static int uvc_ctrl_find_ctrl_idx(struct uvc_entity *entity,
struct v4l2_ext_controls *ctrls,
struct uvc_control *uvc_control)
{
- struct uvc_control_mapping *mapping;
+ struct uvc_control_mapping *mapping = NULL;
struct uvc_control *ctrl_found;
unsigned int i;
--
2.34.0.384.gca35af8252-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] media: uvcvideo: Avoid returning invalid controls
2021-11-30 15:50 [PATCH 1/2] media: uvcvideo: Avoid invalid memory access Ricardo Ribalda
@ 2021-11-30 15:50 ` Ricardo Ribalda
2021-12-01 2:39 ` Laurent Pinchart
2021-12-01 2:36 ` [PATCH 1/2] media: uvcvideo: Avoid invalid memory access Laurent Pinchart
1 sibling, 1 reply; 5+ messages in thread
From: Ricardo Ribalda @ 2021-11-30 15:50 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Hans Verkuil,
linux-media, linux-kernel
Cc: Ricardo Ribalda
If the memory where ctrl_found is places has the value of uvc_ctrl and
__uvc_find_control does not find the control we will return and invalid
index.
Fixes: 6350d6a4ed487 ("media: uvcvideo: Set error_idx during ctrl_commit errors")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index f7b7add3cfa59..f1f6bb14fb0a6 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1639,7 +1639,7 @@ static int uvc_ctrl_find_ctrl_idx(struct uvc_entity *entity,
struct uvc_control *uvc_control)
{
struct uvc_control_mapping *mapping = NULL;
- struct uvc_control *ctrl_found;
+ struct uvc_control *ctrl_found = NULL;
unsigned int i;
if (!entity)
--
2.34.0.384.gca35af8252-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] media: uvcvideo: Avoid returning invalid controls
2021-11-30 15:50 ` [PATCH 2/2] media: uvcvideo: Avoid returning invalid controls Ricardo Ribalda
@ 2021-12-01 2:39 ` Laurent Pinchart
0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2021-12-01 2:39 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel
Hi Ricardo,
Thank you for the patch.
On Tue, Nov 30, 2021 at 03:50:26PM +0000, Ricardo Ribalda wrote:
> If the memory where ctrl_found is places has the value of uvc_ctrl and
s/places/placed/
s/uvc_ctrl/uvc_control/
> __uvc_find_control does not find the control we will return and invalid
s/and invalid/an invalid/
> index.
The change of this happening is small, but it exists.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Fixes: 6350d6a4ed487 ("media: uvcvideo: Set error_idx during ctrl_commit errors")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index f7b7add3cfa59..f1f6bb14fb0a6 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1639,7 +1639,7 @@ static int uvc_ctrl_find_ctrl_idx(struct uvc_entity *entity,
> struct uvc_control *uvc_control)
> {
> struct uvc_control_mapping *mapping = NULL;
> - struct uvc_control *ctrl_found;
> + struct uvc_control *ctrl_found = NULL;
> unsigned int i;
>
> if (!entity)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] media: uvcvideo: Avoid invalid memory access
2021-11-30 15:50 [PATCH 1/2] media: uvcvideo: Avoid invalid memory access Ricardo Ribalda
2021-11-30 15:50 ` [PATCH 2/2] media: uvcvideo: Avoid returning invalid controls Ricardo Ribalda
@ 2021-12-01 2:36 ` Laurent Pinchart
2021-12-01 5:41 ` Ricardo Ribalda
1 sibling, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2021-12-01 2:36 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel
Hi Ricardo,
Thank you for the patch.
On Tue, Nov 30, 2021 at 03:50:25PM +0000, Ricardo Ribalda wrote:
> If mappings points to an invalid memory, we will be invalid accessing
> it.
> Solve it by initializing the value of the variable mapping and by
> changing the order in the conditional statement (to avoid accessing
> mapping->id if not needed).
>
> Fix:
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] PREEMPT SMP KASAN NOPTI
>
> Fixes: 6350d6a4ed487 ("media: uvcvideo: Set error_idx during ctrl_commit errors")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 30bfe9069a1fb..f7b7add3cfa59 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -852,8 +852,8 @@ static void __uvc_find_control(struct uvc_entity *entity, u32 v4l2_id,
> return;
> }
>
> - if ((*mapping == NULL || (*mapping)->id > map->id) &&
> - (map->id > v4l2_id) && next) {
> + if (next && (map->id > v4l2_id) &&
> + (*mapping == NULL || (*mapping)->id > map->id)) {
> *control = ctrl;
> *mapping = map;
> }
> @@ -1638,7 +1638,7 @@ static int uvc_ctrl_find_ctrl_idx(struct uvc_entity *entity,
> struct v4l2_ext_controls *ctrls,
> struct uvc_control *uvc_control)
> {
> - struct uvc_control_mapping *mapping;
> + struct uvc_control_mapping *mapping = NULL;
It seems to me that either change will fix the bug, we don't need both,
is that right ? If so I'd drop the change to __uvc_find_control(), as it
seems quite fragile to allow mapping to be uninitialized.
> struct uvc_control *ctrl_found;
> unsigned int i;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2] media: uvcvideo: Avoid invalid memory access
2021-12-01 2:36 ` [PATCH 1/2] media: uvcvideo: Avoid invalid memory access Laurent Pinchart
@ 2021-12-01 5:41 ` Ricardo Ribalda
0 siblings, 0 replies; 5+ messages in thread
From: Ricardo Ribalda @ 2021-12-01 5:41 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, Hans Verkuil, linux-media, linux-kernel
Hi Laurent
Thanks for the prompt reply :)
On Wed, 1 Dec 2021 at 03:37, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Tue, Nov 30, 2021 at 03:50:25PM +0000, Ricardo Ribalda wrote:
> > If mappings points to an invalid memory, we will be invalid accessing
> > it.
> > Solve it by initializing the value of the variable mapping and by
> > changing the order in the conditional statement (to avoid accessing
> > mapping->id if not needed).
> >
> > Fix:
> > kasan: GPF could be caused by NULL-ptr deref or user memory access
> > general protection fault: 0000 [#1] PREEMPT SMP KASAN NOPTI
> >
> > Fixes: 6350d6a4ed487 ("media: uvcvideo: Set error_idx during ctrl_commit errors")
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 30bfe9069a1fb..f7b7add3cfa59 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -852,8 +852,8 @@ static void __uvc_find_control(struct uvc_entity *entity, u32 v4l2_id,
> > return;
> > }
> >
> > - if ((*mapping == NULL || (*mapping)->id > map->id) &&
> > - (map->id > v4l2_id) && next) {
> > + if (next && (map->id > v4l2_id) &&
> > + (*mapping == NULL || (*mapping)->id > map->id)) {
> > *control = ctrl;
> > *mapping = map;
> > }
> > @@ -1638,7 +1638,7 @@ static int uvc_ctrl_find_ctrl_idx(struct uvc_entity *entity,
> > struct v4l2_ext_controls *ctrls,
> > struct uvc_control *uvc_control)
> > {
> > - struct uvc_control_mapping *mapping;
> > + struct uvc_control_mapping *mapping = NULL;
>
> It seems to me that either change will fix the bug, we don't need both,
> is that right ? If so I'd drop the change to __uvc_find_control(), as it
> seems quite fragile to allow mapping to be uninitialized.
Just wanted to be extra paranoid. I have just sent a v2 of the patch.
Thanks!
>
> > struct uvc_control *ctrl_found;
> > unsigned int i;
> >
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-01 5:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30 15:50 [PATCH 1/2] media: uvcvideo: Avoid invalid memory access Ricardo Ribalda
2021-11-30 15:50 ` [PATCH 2/2] media: uvcvideo: Avoid returning invalid controls Ricardo Ribalda
2021-12-01 2:39 ` Laurent Pinchart
2021-12-01 2:36 ` [PATCH 1/2] media: uvcvideo: Avoid invalid memory access Laurent Pinchart
2021-12-01 5:41 ` 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).