linux-kernel - [PATCH v3 2/2] media: uvcvideo: Do not alloc dev->status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20221214-uvc-status-alloc-v3-2-9a67616cc549@chromium.org>
Date:   Thu, 15 Dec 2022 11:57:19 +0100
From:   Ricardo Ribalda <ribalda@...omium.org>
To:     Max Staudt <mstaudt@...omium.org>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Ming Lei <tom.leiming@...il.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Yunke Cao <yunkec@...omium.org>
Cc:     Ricardo Ribalda <ribalda@...omium.org>,
        Christoph Hellwig <hch@....de>, linux-usb@...r.kernel.org,
        Alan Stern <stern@...land.harvard.edu>,
        linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: [PATCH v3 2/2] media: uvcvideo: Do not alloc dev->status

UVC_MAX_STATUS_SIZE is 16 bytes, it makes more sense to have it inlined
than dynamically allocate it.

To avoid issues with non-coherent DMAs, give the memory the same
allocation as kmalloc.

This patch kind of reverts:
a31a4055473b ("V4L/DVB:usbvideo:don't use part of buffer for USB transfer #4")

Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
---
 drivers/media/usb/uvc/uvc_status.c | 19 ++++++-------------
 drivers/media/usb/uvc/uvcvideo.h   |  6 +++++-
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_status.c b/drivers/media/usb/uvc/uvc_status.c
index dbaa9b07d77f..adf63e7616c9 100644
--- a/drivers/media/usb/uvc/uvc_status.c
+++ b/drivers/media/usb/uvc/uvc_status.c
@@ -200,22 +200,22 @@ static void uvc_status_complete(struct urb *urb)
 
 	len = urb->actual_length;
 	if (len > 0) {
-		switch (dev->status->bStatusType & 0x0f) {
+		switch (dev->status.bStatusType & 0x0f) {
 		case UVC_STATUS_TYPE_CONTROL: {
-			if (uvc_event_control(urb, dev->status, len))
+			if (uvc_event_control(urb, &dev->status, len))
 				/* The URB will be resubmitted in work context. */
 				return;
 			break;
 		}
 
 		case UVC_STATUS_TYPE_STREAMING: {
-			uvc_event_streaming(dev, dev->status, len);
+			uvc_event_streaming(dev, &dev->status, len);
 			break;
 		}
 
 		default:
 			uvc_dbg(dev, STATUS, "Unknown status event type %u\n",
-				dev->status->bStatusType);
+				dev->status.bStatusType);
 			break;
 		}
 	}
@@ -239,15 +239,9 @@ int uvc_status_init(struct uvc_device *dev)
 
 	uvc_input_init(dev);
 
-	dev->status = kzalloc(sizeof(*dev->status), GFP_KERNEL);
-	if (dev->status == NULL)
-		return -ENOMEM;
-
 	dev->int_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!dev->int_urb) {
-		kfree(dev->status);
+	if (!dev->int_urb)
 		return -ENOMEM;
-	}
 
 	pipe = usb_rcvintpipe(dev->udev, ep->desc.bEndpointAddress);
 
@@ -261,7 +255,7 @@ int uvc_status_init(struct uvc_device *dev)
 		interval = fls(interval) - 1;
 
 	usb_fill_int_urb(dev->int_urb, dev->udev, pipe,
-		dev->status, sizeof(dev->status), uvc_status_complete,
+		&dev->status, sizeof(dev->status), uvc_status_complete,
 		dev, interval);
 
 	return 0;
@@ -276,7 +270,6 @@ void uvc_status_unregister(struct uvc_device *dev)
 void uvc_status_cleanup(struct uvc_device *dev)
 {
 	usb_free_urb(dev->int_urb);
-	kfree(dev->status);
 }
 
 int uvc_status_start(struct uvc_device *dev, gfp_t flags)
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 84326991ec36..b0abc70ca58e 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -577,7 +577,11 @@ struct uvc_device {
 	/* Status Interrupt Endpoint */
 	struct usb_host_endpoint *int_ep;
 	struct urb *int_urb;
-	struct uvc_status *status;
+	/*
+	 * Ensure that status is aligned, making it safe to use with
+	 * non-coherent DMA.
+	 */
+	struct uvc_status status __aligned(ARCH_KMALLOC_MINALIGN);
 
 	struct input_dev *input;
 	char input_phys[64];

-- 
2.39.0.314.g84b9a713c41-goog-b4-0.11.0-dev-696ae