* [PATCH] usb: gadget: uvc: Initialize color matching descriptors for frame-based
@ 2025-06-25 10:16 Akash Kumar
2025-06-28 14:59 ` Greg Kroah-Hartman
0 siblings, 1 reply; 3+ messages in thread
From: Akash Kumar @ 2025-06-25 10:16 UTC (permalink / raw)
To: Thinh Nguyen, Greg Kroah-Hartman, Felipe Balbi, Jack Pham, kernel,
Wesley Cheng, Laurent Pinchart, Daniel Scally
Cc: Vijayavardhan Vennapusa, Krishna Kurapati, linux-usb,
linux-kernel, Akash Kumar
Fix NULL pointer crash in uvcg_framebased_make due to uninitialize
color matching descriptor for frame-based format.
[ 2.771141][ T486] pc : __uvcg_fill_strm+0x198/0x2cc
[ 2.771145][ T486] lr : __uvcg_iter_strm_cls+0xc8/0x17c
[ 2.771146][ T486] sp : ffffffc08140bbb0
[ 2.771146][ T486] x29: ffffffc08140bbb0 x28: ffffff803bc81380 x27: ffffff8023bbd250
[ 2.771147][ T486] x26: ffffff8023bbd250 x25: ffffff803c361348 x24: ffffff803d8e6768
[ 2.771148][ T486] x23: 0000000000000004 x22: 0000000000000003 x21: ffffffc08140bc48
[ 2.771149][ T486] x20: 0000000000000000 x19: ffffffc08140bc48 x18: ffffffe9f8cf4a00
[ 2.771150][ T486] x17: 000000001bf64ec3 x16: 000000001bf64ec3 x15: ffffff8023bbd250
[ 2.771151][ T486] x14: 000000000000000f x13: 004c4b40000f4240 x12: 000a2c2a00051615
[ 2.771152][ T486] x11: 000000000000004f x10: ffffffe9f76b40ec x9 : ffffffe9f7e389d0
[ 2.771153][ T486] x8 : ffffff803d0d31ce x7 : 000f4240000a2c2a x6 : 0005161500028b0a
[ 2.771154][ T486] x5 : ffffff803d0d31ce x4 : 0000000000000003 x3 : 0000000000000000
[ 2.771155][ T486] x2 : ffffffc08140bc50 x1 : ffffffc08140bc48 x0 : 0000000000000000
[ 2.771156][ T486] Call trace:
[ 2.771157][ T486] __uvcg_fill_strm+0x198/0x2cc
[ 2.771157][ T486] __uvcg_iter_strm_cls+0xc8/0x17c
[ 2.771158][ T486] uvcg_streaming_class_allow_link+0x240/0x290
[ 2.771159][ T486] configfs_symlink+0x1f8/0x630
[ 2.771161][ T486] vfs_symlink+0x114/0x1a0
[ 2.771163][ T486] do_symlinkat+0x94/0x28c
[ 2.771164][ T486] __arm64_sys_symlinkat+0x54/0x70
[ 2.771164][ T486] invoke_syscall+0x58/0x114
[ 2.771166][ T486] el0_svc_common+0x80/0xe0
[ 2.771168][ T486] do_el0_svc+0x1c/0x28
[ 2.771169][ T486] el0_svc+0x3c/0x70
[ 2.771172][ T486] el0t_64_sync_handler+0x68/0xbc
[ 2.771173][ T486] el0t_64_sync+0x1a8/0x1ac
Initialize color matching descriptor for frame-based format to prevent
NULL pointer crash.
This fix prevents a NULL pointer crash in uvcg_framebased_make due to
an uninitialized color matching descriptor.
Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
---
drivers/usb/gadget/function/uvc_configfs.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index f131943254a4..a4a2d3dcb0d6 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2916,8 +2916,15 @@ static struct config_group *uvcg_framebased_make(struct config_group *group,
'H', '2', '6', '4', 0x00, 0x00, 0x10, 0x00,
0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71
};
+ struct uvcg_color_matching *color_match;
+ struct config_item *streaming;
struct uvcg_framebased *h;
+ streaming = group->cg_item.ci_parent;
+ color_match = uvcg_format_get_default_color_match(streaming);
+ if (!color_match)
+ return ERR_PTR(-EINVAL);
+
h = kzalloc(sizeof(*h), GFP_KERNEL);
if (!h)
return ERR_PTR(-ENOMEM);
@@ -2936,6 +2943,9 @@ static struct config_group *uvcg_framebased_make(struct config_group *group,
INIT_LIST_HEAD(&h->fmt.frames);
h->fmt.type = UVCG_FRAMEBASED;
+
+ h->fmt.color_matching = color_match;
+ color_match->refcnt++;
config_group_init_type_name(&h->fmt.group, name,
&uvcg_framebased_type);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: uvc: Initialize color matching descriptors for frame-based
2025-06-25 10:16 [PATCH] usb: gadget: uvc: Initialize color matching descriptors for frame-based Akash Kumar
@ 2025-06-28 14:59 ` Greg Kroah-Hartman
2025-06-30 6:36 ` AKASH KUMAR
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2025-06-28 14:59 UTC (permalink / raw)
To: Akash Kumar
Cc: Thinh Nguyen, Felipe Balbi, Jack Pham, kernel, Wesley Cheng,
Laurent Pinchart, Daniel Scally, Vijayavardhan Vennapusa,
Krishna Kurapati, linux-usb, linux-kernel
On Wed, Jun 25, 2025 at 03:46:39PM +0530, Akash Kumar wrote:
> Fix NULL pointer crash in uvcg_framebased_make due to uninitialize
> color matching descriptor for frame-based format.
>
> [ 2.771141][ T486] pc : __uvcg_fill_strm+0x198/0x2cc
> [ 2.771145][ T486] lr : __uvcg_iter_strm_cls+0xc8/0x17c
> [ 2.771146][ T486] sp : ffffffc08140bbb0
> [ 2.771146][ T486] x29: ffffffc08140bbb0 x28: ffffff803bc81380 x27: ffffff8023bbd250
> [ 2.771147][ T486] x26: ffffff8023bbd250 x25: ffffff803c361348 x24: ffffff803d8e6768
> [ 2.771148][ T486] x23: 0000000000000004 x22: 0000000000000003 x21: ffffffc08140bc48
> [ 2.771149][ T486] x20: 0000000000000000 x19: ffffffc08140bc48 x18: ffffffe9f8cf4a00
> [ 2.771150][ T486] x17: 000000001bf64ec3 x16: 000000001bf64ec3 x15: ffffff8023bbd250
> [ 2.771151][ T486] x14: 000000000000000f x13: 004c4b40000f4240 x12: 000a2c2a00051615
> [ 2.771152][ T486] x11: 000000000000004f x10: ffffffe9f76b40ec x9 : ffffffe9f7e389d0
> [ 2.771153][ T486] x8 : ffffff803d0d31ce x7 : 000f4240000a2c2a x6 : 0005161500028b0a
> [ 2.771154][ T486] x5 : ffffff803d0d31ce x4 : 0000000000000003 x3 : 0000000000000000
> [ 2.771155][ T486] x2 : ffffffc08140bc50 x1 : ffffffc08140bc48 x0 : 0000000000000000
> [ 2.771156][ T486] Call trace:
> [ 2.771157][ T486] __uvcg_fill_strm+0x198/0x2cc
> [ 2.771157][ T486] __uvcg_iter_strm_cls+0xc8/0x17c
> [ 2.771158][ T486] uvcg_streaming_class_allow_link+0x240/0x290
> [ 2.771159][ T486] configfs_symlink+0x1f8/0x630
> [ 2.771161][ T486] vfs_symlink+0x114/0x1a0
> [ 2.771163][ T486] do_symlinkat+0x94/0x28c
> [ 2.771164][ T486] __arm64_sys_symlinkat+0x54/0x70
> [ 2.771164][ T486] invoke_syscall+0x58/0x114
> [ 2.771166][ T486] el0_svc_common+0x80/0xe0
> [ 2.771168][ T486] do_el0_svc+0x1c/0x28
> [ 2.771169][ T486] el0_svc+0x3c/0x70
> [ 2.771172][ T486] el0t_64_sync_handler+0x68/0xbc
> [ 2.771173][ T486] el0t_64_sync+0x1a8/0x1ac
What is "[ T486]"?
And where did the beginning of the crash report go?
> Initialize color matching descriptor for frame-based format to prevent
> NULL pointer crash.
> This fix prevents a NULL pointer crash in uvcg_framebased_make due to
> an uninitialized color matching descriptor.
What causes an unitialized color matching descriptor to happen? Do we
have that in the kernel today? Or is this userspace controlled?
Hardware controlled?
>
> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
What git id does this fix?
> ---
> drivers/usb/gadget/function/uvc_configfs.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index f131943254a4..a4a2d3dcb0d6 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -2916,8 +2916,15 @@ static struct config_group *uvcg_framebased_make(struct config_group *group,
> 'H', '2', '6', '4', 0x00, 0x00, 0x10, 0x00,
> 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71
> };
> + struct uvcg_color_matching *color_match;
> + struct config_item *streaming;
> struct uvcg_framebased *h;
>
> + streaming = group->cg_item.ci_parent;
> + color_match = uvcg_format_get_default_color_match(streaming);
> + if (!color_match)
> + return ERR_PTR(-EINVAL);
> +
> h = kzalloc(sizeof(*h), GFP_KERNEL);
> if (!h)
> return ERR_PTR(-ENOMEM);
> @@ -2936,6 +2943,9 @@ static struct config_group *uvcg_framebased_make(struct config_group *group,
>
> INIT_LIST_HEAD(&h->fmt.frames);
> h->fmt.type = UVCG_FRAMEBASED;
> +
> + h->fmt.color_matching = color_match;
> + color_match->refcnt++;
reference counts are almost never done "by hand" like this, are you sure
this is right? I don't see the lock being held that is used when
reading/writing this value elsewhere in the driver, why is this safe
here?
And shouldn't the changelog text be something like "mirror what we do in
the uncompressed mode?" Or the other modes? Why is this one the only
one that does not have this check in it today, was it just forgotten or
was it intentional?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] usb: gadget: uvc: Initialize color matching descriptors for frame-based
2025-06-28 14:59 ` Greg Kroah-Hartman
@ 2025-06-30 6:36 ` AKASH KUMAR
0 siblings, 0 replies; 3+ messages in thread
From: AKASH KUMAR @ 2025-06-30 6:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Thinh Nguyen, Felipe Balbi, Jack Pham, kernel, Wesley Cheng,
Laurent Pinchart, Daniel Scally, Vijayavardhan Vennapusa,
Krishna Kurapati, linux-usb, linux-kernel
Hi Greg,
Thanks for reviewing.
On 6/28/2025 8:29 PM, Greg Kroah-Hartman wrote:
> On Wed, Jun 25, 2025 at 03:46:39PM +0530, Akash Kumar wrote:
>> Fix NULL pointer crash in uvcg_framebased_make due to uninitialize
>> color matching descriptor for frame-based format.
>>
>> [ 2.771141][ T486] pc : __uvcg_fill_strm+0x198/0x2cc
>> [ 2.771145][ T486] lr : __uvcg_iter_strm_cls+0xc8/0x17c
>> [ 2.771146][ T486] sp : ffffffc08140bbb0
>> [ 2.771146][ T486] x29: ffffffc08140bbb0 x28: ffffff803bc81380 x27: ffffff8023bbd250
>> [ 2.771147][ T486] x26: ffffff8023bbd250 x25: ffffff803c361348 x24: ffffff803d8e6768
>> [ 2.771148][ T486] x23: 0000000000000004 x22: 0000000000000003 x21: ffffffc08140bc48
>> [ 2.771149][ T486] x20: 0000000000000000 x19: ffffffc08140bc48 x18: ffffffe9f8cf4a00
>> [ 2.771150][ T486] x17: 000000001bf64ec3 x16: 000000001bf64ec3 x15: ffffff8023bbd250
>> [ 2.771151][ T486] x14: 000000000000000f x13: 004c4b40000f4240 x12: 000a2c2a00051615
>> [ 2.771152][ T486] x11: 000000000000004f x10: ffffffe9f76b40ec x9 : ffffffe9f7e389d0
>> [ 2.771153][ T486] x8 : ffffff803d0d31ce x7 : 000f4240000a2c2a x6 : 0005161500028b0a
>> [ 2.771154][ T486] x5 : ffffff803d0d31ce x4 : 0000000000000003 x3 : 0000000000000000
>> [ 2.771155][ T486] x2 : ffffffc08140bc50 x1 : ffffffc08140bc48 x0 : 0000000000000000
>> [ 2.771156][ T486] Call trace:
>> [ 2.771157][ T486] __uvcg_fill_strm+0x198/0x2cc
>> [ 2.771157][ T486] __uvcg_iter_strm_cls+0xc8/0x17c
>> [ 2.771158][ T486] uvcg_streaming_class_allow_link+0x240/0x290
>> [ 2.771159][ T486] configfs_symlink+0x1f8/0x630
>> [ 2.771161][ T486] vfs_symlink+0x114/0x1a0
>> [ 2.771163][ T486] do_symlinkat+0x94/0x28c
>> [ 2.771164][ T486] __arm64_sys_symlinkat+0x54/0x70
>> [ 2.771164][ T486] invoke_syscall+0x58/0x114
>> [ 2.771166][ T486] el0_svc_common+0x80/0xe0
>> [ 2.771168][ T486] do_el0_svc+0x1c/0x28
>> [ 2.771169][ T486] el0_svc+0x3c/0x70
>> [ 2.771172][ T486] el0t_64_sync_handler+0x68/0xbc
>> [ 2.771173][ T486] el0t_64_sync+0x1a8/0x1ac
> What is "[ T486]"?
>
> And where did the beginning of the crash report go?
Will share full crash log, its a thread id and must not belong to a
change commit text, will remove and update message.
>
>> Initialize color matching descriptor for frame-based format to prevent
>> NULL pointer crash.
>> This fix prevents a NULL pointer crash in uvcg_framebased_make due to
>> an uninitialized color matching descriptor.
> What causes an unitialized color matching descriptor to happen? Do we
> have that in the kernel today? Or is this userspace controlled?
> Hardware controlled?
Issue is seen when userspace configuration (via configfs) does not
explicitly define the color matching descriptor.
If color_matching is not found, config_group_find_item() returns NULL.
The code then jumps to out_put_cm, where it calls
config_item_put(color_matching);.
If color_matching is NULL, this will dereference a null pointer, leading
to a crash.
refer: [RESEND v3 6/6] usb: gadget: uvc: Allow creating new color
matching descriptors - Daniel Scally
<https://lore.kernel.org/all/20230202114142.300858-7-dan.scally@ideasonboard.com/>
This is userspace controlled and associated with uvc formats.
I will send a fix to handle from kernel driver as well.
>> Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
> What git id does this fix?
Will update git id [PATCH v3] usb: gadget: uvc: configfs: Add
frame-based frame format support - Akash Kumar
<https://lore.kernel.org/all/20240927152138.31416-1-quic_akakum@quicinc.com/>
>
>> ---
>> drivers/usb/gadget/function/uvc_configfs.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
>> index f131943254a4..a4a2d3dcb0d6 100644
>> --- a/drivers/usb/gadget/function/uvc_configfs.c
>> +++ b/drivers/usb/gadget/function/uvc_configfs.c
>> @@ -2916,8 +2916,15 @@ static struct config_group *uvcg_framebased_make(struct config_group *group,
>> 'H', '2', '6', '4', 0x00, 0x00, 0x10, 0x00,
>> 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71
>> };
>> + struct uvcg_color_matching *color_match;
>> + struct config_item *streaming;
>> struct uvcg_framebased *h;
>>
>> + streaming = group->cg_item.ci_parent;
>> + color_match = uvcg_format_get_default_color_match(streaming);
>> + if (!color_match)
>> + return ERR_PTR(-EINVAL);
>> +
>> h = kzalloc(sizeof(*h), GFP_KERNEL);
>> if (!h)
>> return ERR_PTR(-ENOMEM);
>> @@ -2936,6 +2943,9 @@ static struct config_group *uvcg_framebased_make(struct config_group *group,
>>
>> INIT_LIST_HEAD(&h->fmt.frames);
>> h->fmt.type = UVCG_FRAMEBASED;
>> +
>> + h->fmt.color_matching = color_match;
>> + color_match->refcnt++;
> reference counts are almost never done "by hand" like this, are you sure
> this is right? I don't see the lock being held that is used when
> reading/writing this value elsewhere in the driver, why is this safe
> here?
>
> And shouldn't the changelog text be something like "mirror what we do in
> the uncompressed mode?" Or the other modes? Why is this one the only
> one that does not have this check in it today, was it just forgotten or
> was it intentional?
yes it is not safe, i simply mirrored the code used in other formats,
due you to suggest updating locks or using
refcount_inc.
Thanks,
Akash
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-06-30 6:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-25 10:16 [PATCH] usb: gadget: uvc: Initialize color matching descriptors for frame-based Akash Kumar
2025-06-28 14:59 ` Greg Kroah-Hartman
2025-06-30 6:36 ` AKASH KUMAR
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).