[PATCH v4] media: usb: siano: Fix allocation of urbs

From: Ricardo Ribalda
Date: Mon Apr 15 2024 - 08:48:24 EST


USB urbs must be allocated with usb_alloc_urb. Quoting the manual

Only use this function (usb_init_urb) if you _really_ understand what you
are doing.

Fix the following smatch error:

drivers/media/usb/siano/smsusb.c:53:38: warning: array of flexible structures

Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
---
Changes in v4:
- Remove unneeded label free_urbs
- Link to v3: https://lore.kernel.org/r/20240415-smatch-v3-1-ed3f23f9d5ee@xxxxxxxxxxxx

Changes in v3: Thanks Hans
- Remove unneeded NULL assignment
- Use the proper variable :)
- Link to v2: https://lore.kernel.org/r/20240415-smatch-v2-1-65215936d398@xxxxxxxxxxxx

Changes in v2: Thanks Hans
- Only leave 1/6, the other ones are in another PR
- Fix the return tag and NULLify the urbs on return
- Link to v1: https://lore.kernel.org/r/20240410-smatch-v1-0-785d009a852b@xxxxxxxxxxxx
---
drivers/media/usb/siano/smsusb.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
index 723510520d09..2c8179a84991 100644
--- a/drivers/media/usb/siano/smsusb.c
+++ b/drivers/media/usb/siano/smsusb.c
@@ -40,7 +40,7 @@ struct smsusb_urb_t {
struct smscore_buffer_t *cb;
struct smsusb_device_t *dev;

- struct urb urb;
+ struct urb *urb;

/* For the bottom half */
struct work_struct wq;
@@ -160,7 +160,7 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
}

usb_fill_bulk_urb(
- &surb->urb,
+ surb->urb,
dev->udev,
usb_rcvbulkpipe(dev->udev, dev->in_ep),
surb->cb->p,
@@ -168,9 +168,9 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
smsusb_onresponse,
surb
);
- surb->urb.transfer_flags |= URB_FREE_BUFFER;
+ surb->urb->transfer_flags |= URB_FREE_BUFFER;

- return usb_submit_urb(&surb->urb, GFP_ATOMIC);
+ return usb_submit_urb(surb->urb, GFP_ATOMIC);
}

static void smsusb_stop_streaming(struct smsusb_device_t *dev)
@@ -178,7 +178,7 @@ static void smsusb_stop_streaming(struct smsusb_device_t *dev)
int i;

for (i = 0; i < MAX_URBS; i++) {
- usb_kill_urb(&dev->surbs[i].urb);
+ usb_kill_urb(dev->surbs[i].urb);
if (dev->surbs[i].wq.func)
cancel_work_sync(&dev->surbs[i].wq);

@@ -338,6 +338,8 @@ static void smsusb_term_device(struct usb_interface *intf)
struct smsusb_device_t *dev = usb_get_intfdata(intf);

if (dev) {
+ int i;
+
dev->state = SMSUSB_DISCONNECTED;

smsusb_stop_streaming(dev);
@@ -346,6 +348,9 @@ static void smsusb_term_device(struct usb_interface *intf)
if (dev->coredev)
smscore_unregister_device(dev->coredev);

+ for (i = 0; i < MAX_URBS; i++)
+ usb_free_urb(dev->surbs[i].urb);
+
pr_debug("device 0x%p destroyed\n", dev);
kfree(dev);
}
@@ -463,7 +468,9 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
/* initialize urbs */
for (i = 0; i < MAX_URBS; i++) {
dev->surbs[i].dev = dev;
- usb_init_urb(&dev->surbs[i].urb);
+ dev->surbs[i].urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!dev->surbs[i].urb)
+ goto err_unregister_device;
}

pr_debug("smsusb_start_streaming(...).\n");
@@ -486,6 +493,7 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
return rc;

err_unregister_device:
+ /* smsusb_term_device() frees any allocated urb. */
smsusb_term_device(intf);
#ifdef CONFIG_MEDIA_CONTROLLER_DVB
media_device_unregister(mdev);

---
base-commit: 34d7bf1c8e59f5fbf438ee32c96389ebe41ca2e8
change-id: 20240410-smatch-8f235d50753d

Best regards,
--
Ricardo Ribalda <ribalda@xxxxxxxxxxxx>