* [PATCH 1/4] igorplugusb: respect DMA coherency
@ 2022-05-12 12:38 Oliver Neukum
2022-05-12 12:38 ` [PATCH 2/4] igorplugusb: prevent use after free in probe error Oliver Neukum
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Oliver Neukum @ 2022-05-12 12:38 UTC (permalink / raw)
To: linux-media, mchehab, sean; +Cc: Oliver Neukum
The coherency rules mean that you cannot embed
a buffer inside a descriptor. kmalloc() separately.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/media/rc/igorplugusb.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index b40dbf500186..b46362da8623 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -38,7 +38,7 @@ struct igorplugusb {
struct timer_list timer;
- uint8_t buf_in[MAX_PACKET];
+ u8 *buf_in;
char phys[64];
};
@@ -177,6 +177,9 @@ static int igorplugusb_probe(struct usb_interface *intf,
if (!ir->urb)
goto fail;
+ ir->buf_in = kmalloc(MAX_PACKET, GFP_KERNEL);
+ if (!ir->buf_in)
+ goto fail;
usb_fill_control_urb(ir->urb, udev,
usb_rcvctrlpipe(udev, 0), (uint8_t *)&ir->request,
ir->buf_in, sizeof(ir->buf_in), igorplugusb_callback, ir);
@@ -223,6 +226,7 @@ static int igorplugusb_probe(struct usb_interface *intf,
rc_free_device(ir->rc);
usb_free_urb(ir->urb);
del_timer(&ir->timer);
+ kfree(ir->buf_in);
return ret;
}
@@ -236,6 +240,7 @@ static void igorplugusb_disconnect(struct usb_interface *intf)
usb_set_intfdata(intf, NULL);
usb_kill_urb(ir->urb);
usb_free_urb(ir->urb);
+ kfree(ir->buf_in);
}
static const struct usb_device_id igorplugusb_table[] = {
--
2.35.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] igorplugusb: prevent use after free in probe error
2022-05-12 12:38 [PATCH 1/4] igorplugusb: respect DMA coherency Oliver Neukum
@ 2022-05-12 12:38 ` Oliver Neukum
2022-05-12 12:38 ` [PATCH 3/4] igorplugusb: break cyclical race on disconnect Oliver Neukum
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Oliver Neukum @ 2022-05-12 12:38 UTC (permalink / raw)
To: linux-media, mchehab, sean; +Cc: Oliver Neukum
The timer uses the URB. Free it only after the timer
has been stopped.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/media/rc/igorplugusb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index b46362da8623..1afba95409ff 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -223,9 +223,9 @@ static int igorplugusb_probe(struct usb_interface *intf,
return 0;
fail:
- rc_free_device(ir->rc);
- usb_free_urb(ir->urb);
del_timer(&ir->timer);
+ usb_free_urb(ir->urb);
+ rc_free_device(ir->rc);
kfree(ir->buf_in);
return ret;
--
2.35.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] igorplugusb: break cyclical race on disconnect
2022-05-12 12:38 [PATCH 1/4] igorplugusb: respect DMA coherency Oliver Neukum
2022-05-12 12:38 ` [PATCH 2/4] igorplugusb: prevent use after free in probe error Oliver Neukum
@ 2022-05-12 12:38 ` Oliver Neukum
2022-05-12 12:38 ` [PATCH 4/4] igorplugusb: remove superfluous usb_unlink_urb() Oliver Neukum
2022-05-13 16:20 ` [PATCH 1/4] igorplugusb: respect DMA coherency Sean Young
3 siblings, 0 replies; 5+ messages in thread
From: Oliver Neukum @ 2022-05-12 12:38 UTC (permalink / raw)
To: linux-media, mchehab, sean; +Cc: Oliver Neukum
The driver uses a timer, that may submit the URB and
the URB may start the timer. No simple order of killing
can break te cycle. Poison the URB before killing
the timer.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/media/rc/igorplugusb.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index 1afba95409ff..b2245849f7aa 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -126,7 +126,7 @@ static void igorplugusb_cmd(struct igorplugusb *ir, int cmd)
ir->request.bRequest = cmd;
ir->urb->transfer_flags = 0;
ret = usb_submit_urb(ir->urb, GFP_ATOMIC);
- if (ret)
+ if (ret && ret != -EPERM)
dev_err(ir->dev, "submit urb failed: %d", ret);
}
@@ -223,7 +223,9 @@ static int igorplugusb_probe(struct usb_interface *intf,
return 0;
fail:
+ usb_poison_urb(ir->urb);
del_timer(&ir->timer);
+ usb_unpoison_urb(ir->urb);
usb_free_urb(ir->urb);
rc_free_device(ir->rc);
kfree(ir->buf_in);
@@ -236,9 +238,10 @@ static void igorplugusb_disconnect(struct usb_interface *intf)
struct igorplugusb *ir = usb_get_intfdata(intf);
rc_unregister_device(ir->rc);
+ usb_poison_urb(ir->urb);
del_timer_sync(&ir->timer);
usb_set_intfdata(intf, NULL);
- usb_kill_urb(ir->urb);
+ usb_unpoison_urb(ir->urb);
usb_free_urb(ir->urb);
kfree(ir->buf_in);
}
--
2.35.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] igorplugusb: remove superfluous usb_unlink_urb()
2022-05-12 12:38 [PATCH 1/4] igorplugusb: respect DMA coherency Oliver Neukum
2022-05-12 12:38 ` [PATCH 2/4] igorplugusb: prevent use after free in probe error Oliver Neukum
2022-05-12 12:38 ` [PATCH 3/4] igorplugusb: break cyclical race on disconnect Oliver Neukum
@ 2022-05-12 12:38 ` Oliver Neukum
2022-05-13 16:20 ` [PATCH 1/4] igorplugusb: respect DMA coherency Sean Young
3 siblings, 0 replies; 5+ messages in thread
From: Oliver Neukum @ 2022-05-12 12:38 UTC (permalink / raw)
To: linux-media, mchehab, sean; +Cc: Oliver Neukum
Calling that on yourself while the completion handler
is running is a NOP. Remove it.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/media/rc/igorplugusb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index b2245849f7aa..12ee5dd0a61a 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -110,7 +110,6 @@ static void igorplugusb_callback(struct urb *urb)
case -ECONNRESET:
case -ENOENT:
case -ESHUTDOWN:
- usb_unlink_urb(urb);
return;
default:
dev_warn(ir->dev, "Error: urb status = %d\n", urb->status);
--
2.35.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/4] igorplugusb: respect DMA coherency
2022-05-12 12:38 [PATCH 1/4] igorplugusb: respect DMA coherency Oliver Neukum
` (2 preceding siblings ...)
2022-05-12 12:38 ` [PATCH 4/4] igorplugusb: remove superfluous usb_unlink_urb() Oliver Neukum
@ 2022-05-13 16:20 ` Sean Young
3 siblings, 0 replies; 5+ messages in thread
From: Sean Young @ 2022-05-13 16:20 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-media, mchehab
On Thu, May 12, 2022 at 02:38:46PM +0200, Oliver Neukum wrote:
> The coherency rules mean that you cannot embed
> a buffer inside a descriptor. kmalloc() separately.
I've merged this series to my tree. Thanks!
Sean
>
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
> drivers/media/rc/igorplugusb.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
> index b40dbf500186..b46362da8623 100644
> --- a/drivers/media/rc/igorplugusb.c
> +++ b/drivers/media/rc/igorplugusb.c
> @@ -38,7 +38,7 @@ struct igorplugusb {
>
> struct timer_list timer;
>
> - uint8_t buf_in[MAX_PACKET];
> + u8 *buf_in;
>
> char phys[64];
> };
> @@ -177,6 +177,9 @@ static int igorplugusb_probe(struct usb_interface *intf,
> if (!ir->urb)
> goto fail;
>
> + ir->buf_in = kmalloc(MAX_PACKET, GFP_KERNEL);
> + if (!ir->buf_in)
> + goto fail;
> usb_fill_control_urb(ir->urb, udev,
> usb_rcvctrlpipe(udev, 0), (uint8_t *)&ir->request,
> ir->buf_in, sizeof(ir->buf_in), igorplugusb_callback, ir);
> @@ -223,6 +226,7 @@ static int igorplugusb_probe(struct usb_interface *intf,
> rc_free_device(ir->rc);
> usb_free_urb(ir->urb);
> del_timer(&ir->timer);
> + kfree(ir->buf_in);
>
> return ret;
> }
> @@ -236,6 +240,7 @@ static void igorplugusb_disconnect(struct usb_interface *intf)
> usb_set_intfdata(intf, NULL);
> usb_kill_urb(ir->urb);
> usb_free_urb(ir->urb);
> + kfree(ir->buf_in);
> }
>
> static const struct usb_device_id igorplugusb_table[] = {
> --
> 2.35.3
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-13 16:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12 12:38 [PATCH 1/4] igorplugusb: respect DMA coherency Oliver Neukum
2022-05-12 12:38 ` [PATCH 2/4] igorplugusb: prevent use after free in probe error Oliver Neukum
2022-05-12 12:38 ` [PATCH 3/4] igorplugusb: break cyclical race on disconnect Oliver Neukum
2022-05-12 12:38 ` [PATCH 4/4] igorplugusb: remove superfluous usb_unlink_urb() Oliver Neukum
2022-05-13 16:20 ` [PATCH 1/4] igorplugusb: respect DMA coherency Sean Young
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).