Re: [PATCH v2] usb: gadget: uvc: align XU descriptor pointers to fix kmemleak reports
Greg Kroah-Hartman <[email protected]> Tue, 4 Aug 2026 08:28:55 +0200
| Newsgroups | org.kernel.vger.linux-usb,org.kernel.vger.linux-kernel |
|---|---|
| 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