From b7db5733a5ace9acc1f3104c9050c5aa1363f13b Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 18 Aug 2022 23:01:15 +0200 Subject: usb: move from strlcpy with unused retval to strscpy Follow the advice of the below link and prefer 'strscpy' in this subsystem. Conversion is 1:1 because the return value is not used. Generated by a coccinelle script. Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/ Reviewed-by: Richard Leitner Reviewed-by: Laurent Pinchart Acked-by: Shuah Khan Signed-off-by: Wolfram Sang Link: https://lore.kernel.org/r/20220818210116.7517-1-wsa+renesas@sang-engineering.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/function/f_uvc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/gadget/function/f_uvc.c') diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 71669e0e4d00..f4f6cf75930b 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -430,7 +430,7 @@ uvc_register_video(struct uvc_device *uvc) uvc->vdev.vfl_dir = VFL_DIR_TX; uvc->vdev.lock = &uvc->video.mutex; uvc->vdev.device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING; - strlcpy(uvc->vdev.name, cdev->gadget->name, sizeof(uvc->vdev.name)); + strscpy(uvc->vdev.name, cdev->gadget->name, sizeof(uvc->vdev.name)); video_set_drvdata(&uvc->vdev, uvc); -- cgit v1.2.3 From 9b91a65230784a9ef644b8bdbb82a79ba4ae9456 Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Wed, 7 Sep 2022 23:58:18 +0200 Subject: usb: gadget: uvc: increase worker prio to WQ_HIGHPRI This patch is changing the simple workqueue in the gadget driver to be allocated as async_wq with a higher priority. The pump worker, that is filling the usb requests, will have a higher priority and will not be scheduled away so often while the video stream is handled. This will lead to fewer streaming underruns. Signed-off-by: Michael Grzeschik Link: https://lore.kernel.org/r/20220907215818.2670097-1-m.grzeschik@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/function/f_uvc.c | 4 ++++ drivers/usb/gadget/function/uvc.h | 1 + drivers/usb/gadget/function/uvc_v4l2.c | 2 +- drivers/usb/gadget/function/uvc_video.c | 9 +++++++-- 4 files changed, 13 insertions(+), 3 deletions(-) (limited to 'drivers/usb/gadget/function/f_uvc.c') diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index f4f6cf75930b..09961f4ca981 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -897,10 +897,14 @@ static void uvc_function_unbind(struct usb_configuration *c, { struct usb_composite_dev *cdev = c->cdev; struct uvc_device *uvc = to_uvc(f); + struct uvc_video *video = &uvc->video; long wait_ret = 1; uvcg_info(f, "%s()\n", __func__); + if (video->async_wq) + destroy_workqueue(video->async_wq); + /* * If we know we're connected via v4l2, then there should be a cleanup * of the device from userspace either via UVC_EVENT_DISCONNECT or diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 58e383afdd44..1a31e6c6a5ff 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -88,6 +88,7 @@ struct uvc_video { struct usb_ep *ep; struct work_struct pump; + struct workqueue_struct *async_wq; /* Frame parameters */ u8 bpp; diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index 511f106f9843..d6dbf9b763b2 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -170,7 +170,7 @@ uvc_v4l2_qbuf(struct file *file, void *fh, struct v4l2_buffer *b) return ret; if (uvc->state == UVC_STATE_STREAMING) - schedule_work(&video->pump); + queue_work(video->async_wq, &video->pump); return ret; } diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index c00ce0e91f5d..bb037fcc90e6 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -277,7 +277,7 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) spin_unlock_irqrestore(&video->req_lock, flags); if (uvc->state == UVC_STATE_STREAMING) - schedule_work(&video->pump); + queue_work(video->async_wq, &video->pump); } static int @@ -485,7 +485,7 @@ int uvcg_video_enable(struct uvc_video *video, int enable) video->req_int_count = 0; - schedule_work(&video->pump); + queue_work(video->async_wq, &video->pump); return ret; } @@ -499,6 +499,11 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc) spin_lock_init(&video->req_lock); INIT_WORK(&video->pump, uvcg_video_pump); + /* Allocate a work queue for asynchronous video pump handler. */ + video->async_wq = alloc_workqueue("uvcgadget", WQ_UNBOUND | WQ_HIGHPRI, 0); + if (!video->async_wq) + return -EINVAL; + video->uvc = uvc; video->fcc = V4L2_PIX_FMT_YUYV; video->bpp = 16; -- cgit v1.2.3 From 588b9e85609bcb2f84a2be83591480aa943943b6 Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Sat, 10 Sep 2022 00:13:34 +0200 Subject: usb: gadget: uvc: add v4l2 enumeration api calls This patch adds support to the v4l2 VIDIOCs for enum_format, enum_framesizes and enum_frameintervals. This way, the userspace application can use these VIDIOCS to query the via configfs exported frame capabilities. With thes callbacks the userspace doesn't have to bring its own configfs parser. Signed-off-by: Michael Grzeschik Link: https://lore.kernel.org/r/20220909221335.15033-4-m.grzeschik@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/function/f_uvc.c | 30 ++++++ drivers/usb/gadget/function/uvc.h | 2 + drivers/usb/gadget/function/uvc_v4l2.c | 176 +++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+) (limited to 'drivers/usb/gadget/function/f_uvc.c') diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 09961f4ca981..e6948cf8def3 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -888,6 +888,7 @@ static void uvc_free(struct usb_function *f) struct uvc_device *uvc = to_uvc(f); struct f_uvc_opts *opts = container_of(f->fi, struct f_uvc_opts, func_inst); + config_item_put(&uvc->header->item); --opts->refcnt; kfree(uvc); } @@ -945,6 +946,7 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi) struct uvc_device *uvc; struct f_uvc_opts *opts; struct uvc_descriptor_header **strm_cls; + struct config_item *streaming, *header, *h; uvc = kzalloc(sizeof(*uvc), GFP_KERNEL); if (uvc == NULL) @@ -977,6 +979,29 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi) uvc->desc.fs_streaming = opts->fs_streaming; uvc->desc.hs_streaming = opts->hs_streaming; uvc->desc.ss_streaming = opts->ss_streaming; + + streaming = config_group_find_item(&opts->func_inst.group, "streaming"); + if (!streaming) + goto err_config; + + header = config_group_find_item(to_config_group(streaming), "header"); + config_item_put(streaming); + if (!header) + goto err_config; + + h = config_group_find_item(to_config_group(header), "h"); + config_item_put(header); + if (!h) + goto err_config; + + uvc->header = to_uvcg_streaming_header(h); + config_item_put(h); + if (!uvc->header->linked) { + mutex_unlock(&opts->lock); + kfree(uvc); + return ERR_PTR(-EBUSY); + } + ++opts->refcnt; mutex_unlock(&opts->lock); @@ -992,6 +1017,11 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi) uvc->func.bind_deactivated = true; return &uvc->func; + +err_config: + mutex_unlock(&opts->lock); + kfree(uvc); + return ERR_PTR(-ENOENT); } DECLARE_USB_FUNCTION_INIT(uvc, uvc_alloc_inst, uvc_alloc); diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 1a31e6c6a5ff..40226b1f7e14 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -134,6 +134,8 @@ struct uvc_device { bool func_connected; wait_queue_head_t func_connected_queue; + struct uvcg_streaming_header *header; + /* Descriptors */ struct { const struct uvc_descriptor_header * const *fs_control; diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c index d6dbf9b763b2..417655e4a83d 100644 --- a/drivers/usb/gadget/function/uvc_v4l2.c +++ b/drivers/usb/gadget/function/uvc_v4l2.c @@ -18,12 +18,92 @@ #include #include #include +#include #include "f_uvc.h" #include "uvc.h" #include "uvc_queue.h" #include "uvc_video.h" #include "uvc_v4l2.h" +#include "uvc_configfs.h" + +static struct uvc_format_desc *to_uvc_format(struct uvcg_format *uformat) +{ + char guid[16] = UVC_GUID_FORMAT_MJPEG; + struct uvc_format_desc *format; + struct uvcg_uncompressed *unc; + + if (uformat->type == UVCG_UNCOMPRESSED) { + unc = to_uvcg_uncompressed(&uformat->group.cg_item); + if (!unc) + return ERR_PTR(-EINVAL); + + memcpy(guid, unc->desc.guidFormat, sizeof(guid)); + } + + format = uvc_format_by_guid(guid); + if (!format) + return ERR_PTR(-EINVAL); + + return format; +} + +static struct uvcg_format *find_format_by_index(struct uvc_device *uvc, int index) +{ + struct uvcg_format_ptr *format; + struct uvcg_format *uformat = NULL; + int i = 1; + + list_for_each_entry(format, &uvc->header->formats, entry) { + if (index == i) { + uformat = format->fmt; + break; + } + i++; + } + + return uformat; +} + +static struct uvcg_frame *find_frame_by_index(struct uvc_device *uvc, + struct uvcg_format *uformat, + int index) +{ + struct uvcg_format_ptr *format; + struct uvcg_frame_ptr *frame; + struct uvcg_frame *uframe = NULL; + + list_for_each_entry(format, &uvc->header->formats, entry) { + if (format->fmt->type != uformat->type) + continue; + list_for_each_entry(frame, &format->fmt->frames, entry) { + if (index == frame->frm->frame.b_frame_index) { + uframe = frame->frm; + break; + } + } + } + + return uframe; +} + +static struct uvcg_format *find_format_by_pix(struct uvc_device *uvc, + u32 pixelformat) +{ + struct uvcg_format_ptr *format; + struct uvcg_format *uformat = NULL; + + list_for_each_entry(format, &uvc->header->formats, entry) { + struct uvc_format_desc *fmtdesc = to_uvc_format(format->fmt); + + if (fmtdesc->fcc == pixelformat) { + uformat = format->fmt; + break; + } + } + + return uformat; +} /* -------------------------------------------------------------------------- * Requests handling @@ -134,6 +214,99 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt) return 0; } +static int +uvc_v4l2_enum_frameintervals(struct file *file, void *fh, + struct v4l2_frmivalenum *fival) +{ + struct video_device *vdev = video_devdata(file); + struct uvc_device *uvc = video_get_drvdata(vdev); + struct uvcg_format *uformat = NULL; + struct uvcg_frame *uframe = NULL; + struct uvcg_frame_ptr *frame; + + uformat = find_format_by_pix(uvc, fival->pixel_format); + if (!uformat) + return -EINVAL; + + list_for_each_entry(frame, &uformat->frames, entry) { + if (frame->frm->frame.w_width == fival->width && + frame->frm->frame.w_height == fival->height) { + uframe = frame->frm; + break; + } + } + if (!uframe) + return -EINVAL; + + if (fival->index >= uframe->frame.b_frame_interval_type) + return -EINVAL; + + fival->discrete.numerator = + uframe->dw_frame_interval[fival->index]; + + /* TODO: handle V4L2_FRMIVAL_TYPE_STEPWISE */ + fival->type = V4L2_FRMIVAL_TYPE_DISCRETE; + fival->discrete.denominator = 10000000; + v4l2_simplify_fraction(&fival->discrete.numerator, + &fival->discrete.denominator, 8, 333); + + return 0; +} + +static int +uvc_v4l2_enum_framesizes(struct file *file, void *fh, + struct v4l2_frmsizeenum *fsize) +{ + struct video_device *vdev = video_devdata(file); + struct uvc_device *uvc = video_get_drvdata(vdev); + struct uvcg_format *uformat = NULL; + struct uvcg_frame *uframe = NULL; + + uformat = find_format_by_pix(uvc, fsize->pixel_format); + if (!uformat) + return -EINVAL; + + if (fsize->index >= uformat->num_frames) + return -EINVAL; + + uframe = find_frame_by_index(uvc, uformat, fsize->index + 1); + if (!uframe) + return -EINVAL; + + fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE; + fsize->discrete.width = uframe->frame.w_width; + fsize->discrete.height = uframe->frame.w_height; + + return 0; +} + +static int +uvc_v4l2_enum_format(struct file *file, void *fh, struct v4l2_fmtdesc *f) +{ + struct video_device *vdev = video_devdata(file); + struct uvc_device *uvc = video_get_drvdata(vdev); + struct uvc_format_desc *fmtdesc; + struct uvcg_format *uformat; + + if (f->index >= uvc->header->num_fmt) + return -EINVAL; + + uformat = find_format_by_index(uvc, f->index + 1); + if (!uformat) + return -EINVAL; + + if (uformat->type != UVCG_UNCOMPRESSED) + f->flags |= V4L2_FMT_FLAG_COMPRESSED; + + fmtdesc = to_uvc_format(uformat); + f->pixelformat = fmtdesc->fcc; + + strscpy(f->description, fmtdesc->name, sizeof(f->description)); + f->description[strlen(fmtdesc->name) - 1] = 0; + + return 0; +} + static int uvc_v4l2_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *b) { @@ -300,6 +473,9 @@ const struct v4l2_ioctl_ops uvc_v4l2_ioctl_ops = { .vidioc_querycap = uvc_v4l2_querycap, .vidioc_g_fmt_vid_out = uvc_v4l2_get_format, .vidioc_s_fmt_vid_out = uvc_v4l2_set_format, + .vidioc_enum_frameintervals = uvc_v4l2_enum_frameintervals, + .vidioc_enum_framesizes = uvc_v4l2_enum_framesizes, + .vidioc_enum_fmt_vid_out = uvc_v4l2_enum_format, .vidioc_reqbufs = uvc_v4l2_reqbufs, .vidioc_querybuf = uvc_v4l2_querybuf, .vidioc_qbuf = uvc_v4l2_qbuf, -- cgit v1.2.3 From a15e17acce5aaae54243f55a7349c2225450b9bc Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Wed, 28 Sep 2022 13:19:21 -0700 Subject: usb: gadget: uvc: Fix argument to sizeof() in uvc_register_video() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Laurent Pinchart Reviewed-by: Kees Cook Link: https://lore.kernel.org/r/20220928201921.3152163-1-nathan@kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/function/f_uvc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/gadget/function/f_uvc.c') diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index e6948cf8def3..836601227155 100644 --- 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 *uvc) 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; -- cgit v1.2.3 From 3180d827c807d8d6e5d6ba4f2e08eed9efa083af Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Fri, 30 Sep 2022 14:28:39 +0200 Subject: usb: gadget: uvc: don't put item still in use With the patch "588b9e85609b (usb: gadget: uvc: add v4l2 enumeration api calls)" the driver is keeping a list of configfs entries currently configured. The list is used in uvc_v4l2 on runtime. The driver now is giving back the list item just after it was referenced with config_item_put. It also calls config_item_put on uvc_free, which is the only and right place to give back the reference. This patch fixes the issue by removing the extra config_item_put in uvc_alloc. Fixes: 588b9e85609b (usb: gadget: uvc: add v4l2 enumeration api calls) Signed-off-by: Michael Grzeschik Link: https://lore.kernel.org/r/20220930122839.1747279-1-m.grzeschik@pengutronix.de Signed-off-by: Greg Kroah-Hartman --- drivers/usb/gadget/function/f_uvc.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/usb/gadget/function/f_uvc.c') diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 836601227155..6e196e06181e 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -995,7 +995,6 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi) goto err_config; uvc->header = to_uvcg_streaming_header(h); - config_item_put(h); if (!uvc->header->linked) { mutex_unlock(&opts->lock); kfree(uvc); -- cgit v1.2.3