Linux USB
 help / color / mirror / Atom feed
* [PATCH v2] usb: gadget: uvc: align XU descriptor pointers to fix kmemleak reports
@ 2026-08-04  5:29 Rituparna Warwatkar
  2026-08-04  6:28 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 2+ messages in thread
From: Rituparna Warwatkar @ 2026-08-04  5:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-usb, linux-kernel, Rituparna Warwatkar,
	syzbot+54927260acba030187a6

kmemleak reports the baSourceID and bmControls arrays allocated by the
UVC extension-unit configfs attributes as leaked, e.g.:

  BUG: memory leak
  unreferenced object 0xffff888114fee2c0 (size 8):
    __kmalloc_noprof
    uvcg_extension_ba_source_id_store
    configfs_write_iter
    vfs_write
    ksys_write

The arrays are not actually leaked: they are reachable through
xu->desc.baSourceID / xu->desc.bmControls and are freed when the
extension unit is removed. The problem is that struct
uvcg_extension_unit_descriptor is marked __packed, so these two heap
pointers are stored at unaligned offsets. kmemleak only scans memory on
pointer-aligned boundaries, so it never sees the pointers and reports
the arrays as unreferenced.

Unlike the UAPI struct uvc_extension_unit_descriptor, this is an
in-memory staging structure: baSourceID and bmControls are pointers,
not inline arrays, and the wire descriptor is assembled field by field
in UVC_COPY_XU_DESCRIPTOR(). So __packed is not needed for layout
correctness and only serves to misalign the pointers.

Drop __packed and move the two remaining scalar members (bControlSize
and iExtension) ahead of the pointers so that baSourceID lands on a
natural 8-byte boundary. This keeps the pointers aligned and visible to
kmemleak while leaving the structure hole-free and the same size as
before (40 bytes on 64-bit). The bLength..bNrInPins prefix is unchanged,
so the "memcpy(dst, desc, 22)" in UVC_COPY_XU_DESCRIPTOR() and the wire
descriptor layout are unaffected.

Reported-by: syzbot+54927260acba030187a6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=54927260acba030187a6
Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Rituparna Warwatkar <rwarwatkar@gmail.com>
---
Changes in v2:
 - Rather than only dropping __packed (which left padding holes and
   grew the struct), reorder the members so bControlSize and
   iExtension precede the two pointers, grouping baSourceID and
   bmControls at a natural 8-byte boundary. The struct stays 40 bytes
   with no padding, and the bLength..bNrInPins prefix (and thus
   UVC_COPY_XU_DESCRIPTOR() and the wire layout) is unchanged.

 drivers/usb/gadget/function/uvc_configfs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h
index 9391614135e..049fb9e14e4 100644
--- a/drivers/usb/gadget/function/uvc_configfs.h
+++ b/drivers/usb/gadget/function/uvc_configfs.h
@@ -172,11 +172,11 @@ struct uvcg_extension_unit_descriptor {
        u8 guidExtensionCode[16];
        u8 bNumControls;
        u8 bNrInPins;
-       u8 *baSourceID;
        u8 bControlSize;
-       u8 *bmControls;
        u8 iExtension;
-} __packed;
+       u8 *baSourceID;
+       u8 *bmControls;
+};

 struct uvcg_extension {
        struct config_item item;
--
2.47.3


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

* Re: [PATCH v2] usb: gadget: uvc: align XU descriptor pointers to fix kmemleak reports
  2026-08-04  5:29 [PATCH v2] usb: gadget: uvc: align XU descriptor pointers to fix kmemleak reports Rituparna Warwatkar
@ 2026-08-04  6:28 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-04  6:28 UTC (permalink / raw)
  To: Rituparna Warwatkar; +Cc: linux-usb, linux-kernel, syzbot+54927260acba030187a6

On Tue, Aug 04, 2026 at 07:29:47AM +0200, Rituparna Warwatkar wrote:
> kmemleak reports the baSourceID and bmControls arrays allocated by the
> UVC extension-unit configfs attributes as leaked, e.g.:
> 
>   BUG: memory leak
>   unreferenced object 0xffff888114fee2c0 (size 8):
>     __kmalloc_noprof
>     uvcg_extension_ba_source_id_store
>     configfs_write_iter
>     vfs_write
>     ksys_write
> 
> The arrays are not actually leaked: they are reachable through
> xu->desc.baSourceID / xu->desc.bmControls and are freed when the
> extension unit is removed. The problem is that struct
> uvcg_extension_unit_descriptor is marked __packed, so these two heap
> pointers are stored at unaligned offsets. kmemleak only scans memory on
> pointer-aligned boundaries, so it never sees the pointers and reports
> the arrays as unreferenced.

So the subject should say "... fix invalid kmemleak reports" as nothing
is really leaking here, this is purely a work-around for a broken tool.

> Unlike the UAPI struct uvc_extension_unit_descriptor, this is an
> in-memory staging structure: baSourceID and bmControls are pointers,
> not inline arrays, and the wire descriptor is assembled field by field
> in UVC_COPY_XU_DESCRIPTOR(). So __packed is not needed for layout
> correctness and only serves to misalign the pointers.
> 
> Drop __packed and move the two remaining scalar members (bControlSize
> and iExtension) ahead of the pointers so that baSourceID lands on a
> natural 8-byte boundary. This keeps the pointers aligned and visible to
> kmemleak while leaving the structure hole-free and the same size as
> before (40 bytes on 64-bit). The bLength..bNrInPins prefix is unchanged,
> so the "memcpy(dst, desc, 22)" in UVC_COPY_XU_DESCRIPTOR() and the wire
> descriptor layout are unaffected.

Wait, what wire descriptor layout?  You said this was NOT on the wire at
all?

> 
> Reported-by: syzbot+54927260acba030187a6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=54927260acba030187a6
> Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Rituparna Warwatkar <rwarwatkar@gmail.com>

Did a LLM write the above text?


> ---
> Changes in v2:
>  - Rather than only dropping __packed (which left padding holes and
>    grew the struct), reorder the members so bControlSize and
>    iExtension precede the two pointers, grouping baSourceID and
>    bmControls at a natural 8-byte boundary. The struct stays 40 bytes
>    with no padding, and the bLength..bNrInPins prefix (and thus
>    UVC_COPY_XU_DESCRIPTOR() and the wire layout) is unchanged.
> 
>  drivers/usb/gadget/function/uvc_configfs.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h
> index 9391614135e..049fb9e14e4 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.h
> +++ b/drivers/usb/gadget/function/uvc_configfs.h
> @@ -172,11 +172,11 @@ struct uvcg_extension_unit_descriptor {
>         u8 guidExtensionCode[16];
>         u8 bNumControls;
>         u8 bNrInPins;
> -       u8 *baSourceID;
>         u8 bControlSize;
> -       u8 *bmControls;
>         u8 iExtension;
> -} __packed;
> +       u8 *baSourceID;
> +       u8 *bmControls;
> +};

You can keep the __packed line right?

thanks,

greg k-h

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

end of thread, other threads:[~2026-08-04  6:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04  5:29 [PATCH v2] usb: gadget: uvc: align XU descriptor pointers to fix kmemleak reports Rituparna Warwatkar
2026-08-04  6:28 ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox