Use flex array destination for memcpy() in drivers/media/usb/pwc/pwc-uncompress.c

[GustavoARSilva]

raw_frame is a pointer to a flexible structure (a structure that contains a trailing flexible-array member). Use the flexible-array member as destination in memcpy(), explicitly.

drivers/media/usb/pwc/pwc.h:190:struct pwc_raw_frame {
drivers/media/usb/pwc/pwc.h-191-        __le16 type;            /* type of the webcam */
drivers/media/usb/pwc/pwc.h-192-        __le16 vbandlength;     /* Size of 4 lines compressed (used by the
drivers/media/usb/pwc/pwc.h-193-                                   decompressor) */
drivers/media/usb/pwc/pwc.h-194-        __u8   cmd[4];          /* the four byte of the command (in case of
drivers/media/usb/pwc/pwc.h-195-                                   nala, only the first 3 bytes is filled) */
drivers/media/usb/pwc/pwc.h-196-        __u8   rawframe[];      /* frame_size = H / 4 * vbandlength */
drivers/media/usb/pwc/pwc.h-197-} __packed;

diff -u -p ./drivers/media/usb/pwc/pwc-uncompress.c /tmp/nothing/drivers/media/usb/pwc/pwc-uncompress.c
--- ./drivers/media/usb/pwc/pwc-uncompress.c
+++ /tmp/nothing/drivers/media/usb/pwc/pwc-uncompress.c
@@ -39,7 +39,6 @@ int pwc_decompress(struct pwc_device *pd
                         * first 3 bytes is filled (Nala case). We can
                         * determine this using the type of the webcam */
                memcpy(raw_frame->cmd, pdev->cmd_buf, 4);
-               memcpy(raw_frame+1, yuv, pdev->frame_size);
                vb2_set_plane_payload(&fbuf->vb.vb2_buf, 0,
                        struct_size(raw_frame, rawframe, pdev->frame_size));
                return 0;

Caught with the following Cocci script:

@r1@
identifier foo, member, array;
type T1, T2;
@@

 struct foo {
        ...
        T1 member;
(
        T2 array[1];
|
        T2 array[0];
|
        T2 array[];
)
 };

@@
identifier r1.foo;
expression CONST;
struct foo *E1;
@@

*       memcpy(E1 + CONST,...);

[kees]

Yeah, it should be possible to just change "raw_frame+1" to "raw_frame->rawframe".

[GustavoARSilva]

Yeah, it should be possible to just change "raw_frame+1" to "raw_frame->rawframe".

Yep:

diff --git a/drivers/media/usb/pwc/pwc-uncompress.c b/drivers/media/usb/pwc/pwc-uncompress.c
index faf44cdeb2688..cf2591a9675cf 100644
--- a/[drivers/media/usb/pwc/pwc-uncompress.c](https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/tree/drivers/media/usb/pwc/pwc-uncompress.c?h=testing/6.0-rc2-kspp-misc&id=61b2005335d446d9169be9405696be6e7507ebd3)
+++ b/[drivers/media/usb/pwc/pwc-uncompress.c](https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/tree/drivers/media/usb/pwc/pwc-uncompress.c?h=testing/6.0-rc2-kspp-misc&id=e306cf23b09c2d126d09d06497c6a1c680799834)
@@ -39,7 +39,7 @@ int pwc_decompress(struct pwc_device *pdev, struct pwc_frame_buf *fbuf)
             * first 3 bytes is filled (Nala case). We can
             * determine this using the type of the webcam */
        memcpy(raw_frame->cmd, pdev->cmd_buf, 4);
-       memcpy(raw_frame+1, yuv, pdev->frame_size);
+       memcpy(raw_frame->rawframe, yuv, pdev->frame_size);
        vb2_set_plane_payload(&fbuf->vb.vb2_buf, 0,
            struct_size(raw_frame, rawframe, pdev->frame_size));
        return 0;

[GustavoARSilva]

I just sent out a patch for this: https://lore.kernel.org/linux-hardening/YxjpMMUb3WP5Rbu7@work/

[HBh25Y]

Hi guys, is there a similar problem in drivers/infiniband/ulp/srpt/ib_srpt.c? It looks like srpt_build_cmd_rsp also use flexible-array member as destination in memcpy(). https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/infiniband/ulp/srpt/ib_srpt.c#L1424

[GustavoARSilva]

Hi guys, is there a similar problem in drivers/infiniband/ulp/srpt/ib_srpt.c? It looks like srpt_build_cmd_rsp also use flexible-array member as destination in memcpy(). https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/infiniband/ulp/srpt/ib_srpt.c#L1424

Hi!

Yep; that seems to be the same problem. Wanna send a patch? :)

I can open an issue for that so you can include the link to it.