Re: [PATCH v2] usb: gadget: uvc: align XU descriptor pointers to fix kmemleak reports

Greg Kroah-Hartman <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.usb.general
Message-ID <2026080453-crushed-dolly-04ed@gregkh>
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: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=54927260acba030187a6
> Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
> Suggested-by: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Rituparna Warwatkar <[email protected]>

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.