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

Rituparna Warwatkar <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kernel.vger.linux-usb
Message-ID <[email protected]>
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: [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]>
---
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
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.