lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 6.0 030/862] usb: gadget: uvc: Fix argument to sizeof() in uvc_register_video()
    Date
    From: Nathan Chancellor <nathan@kernel.org>

    commit a15e17acce5aaae54243f55a7349c2225450b9bc upstream.

    When building s390 allmodconfig after commit 9b91a6523078 ("usb: gadget:
    uvc: increase worker prio to WQ_HIGHPRI"), the following error occurs:

    In file included from ../include/linux/string.h:253,
    from ../include/linux/bitmap.h:11,
    from ../include/linux/cpumask.h:12,
    from ../include/linux/smp.h:13,
    from ../include/linux/lockdep.h:14,
    from ../include/linux/rcupdate.h:29,
    from ../include/linux/rculist.h:11,
    from ../include/linux/pid.h:5,
    from ../include/linux/sched.h:14,
    from ../include/linux/ratelimit.h:6,
    from ../include/linux/dev_printk.h:16,
    from ../include/linux/device.h:15,
    from ../drivers/usb/gadget/function/f_uvc.c:9:
    In function ‘fortify_memset_chk’,
    inlined from ‘uvc_register_video’ at ../drivers/usb/gadget/function/f_uvc.c:424:2:
    ../include/linux/fortify-string.h:301:25: error: call to ‘__write_overflow_field’ declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
    301 | __write_overflow_field(p_size_field, size);
    | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    This points to the memset() in uvc_register_video(). It is clear that
    the argument to sizeof() is incorrect, as uvc->vdev (a 'struct
    video_device') is being zeroed out but the size of uvc->video (a 'struct
    uvc_video') is being used as the third arugment to memset().

    pahole shows that prior to commit 9b91a6523078 ("usb: gadget: uvc:
    increase worker prio to WQ_HIGHPRI"), 'struct video_device' and
    'struct ucv_video' had the same size, meaning that the argument to
    sizeof() is incorrect semantically but there is no visible issue:

    $ pahole -s build/drivers/usb/gadget/function/f_uvc.o | grep -E "(uvc_video|video_device)\s+"
    video_device 1400 4
    uvc_video 1400 3

    After that change, uvc_video becomes slightly larger, meaning that the
    memset() will overwrite by 8 bytes:

    $ pahole -s build/drivers/usb/gadget/function/f_uvc.o | grep -E "(uvc_video|video_device)\s+"
    video_device 1400 4
    uvc_video 1408 3

    Fix the arugment to sizeof() so that there is no overwrite.

    Cc: stable@vger.kernel.org
    Fixes: e4ce9ed835bc ("usb: gadget: uvc: ensure the vdev is unset")
    Signed-off-by: Nathan Chancellor <nathan@kernel.org>
    Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
    Reviewed-by: Kees Cook <keescook@chromium.org>
    Link: https://lore.kernel.org/r/20220928201921.3152163-1-nathan@kernel.org
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    drivers/usb/gadget/function/f_uvc.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    --- a/drivers/usb/gadget/function/f_uvc.c
    +++ b/drivers/usb/gadget/function/f_uvc.c
    @@ -421,7 +421,7 @@ uvc_register_video(struct uvc_device *uv
    int ret;

    /* TODO reference counting. */
    - memset(&uvc->vdev, 0, sizeof(uvc->video));
    + memset(&uvc->vdev, 0, sizeof(uvc->vdev));
    uvc->vdev.v4l2_dev = &uvc->v4l2_dev;
    uvc->vdev.v4l2_dev->dev = &cdev->gadget->dev;
    uvc->vdev.fops = &uvc_v4l2_fops;

    \
     
     \ /
      Last update: 2022-10-19 10:44    [W:4.123 / U:0.060 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site