2022-06-06 05:04:57

by Xiaohui Zhang

[permalink] [raw]
Subject: [PATCH 1/1] nfc: nfcmrvl: Fix memory leak in nfcmrvl_play_deferred

From: hifoolno <[email protected]>

Currently usb_submit_urb is called directly to submit deferred tx
urbs after unanchor them.

So the usb_giveback_urb_bh would failed to unref it in usb_unanchor_urb
and cause memory leak.

Put those urbs in tx_anchor to avoid the leak, and also fix the error
handling.

Signed-off-by: hifoolno <[email protected]>
---
drivers/nfc/nfcmrvl/usb.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/nfcmrvl/usb.c b/drivers/nfc/nfcmrvl/usb.c
index a99aedff795d..815255b8d72e 100644
--- a/drivers/nfc/nfcmrvl/usb.c
+++ b/drivers/nfc/nfcmrvl/usb.c
@@ -388,13 +388,27 @@ static void nfcmrvl_play_deferred(struct nfcmrvl_usb_drv_data *drv_data)
int err;

while ((urb = usb_get_from_anchor(&drv_data->deferred))) {
+ usb_anchor_urb(urb, &drv_data->tx_anchor);
+
err = usb_submit_urb(urb, GFP_ATOMIC);
- if (err)
+ if (err) {
+ if (err != -EPERM && err != -ENODEV)
+ BT_ERR("%s urb %p submission failed (%d)",
+ drv_data->hdev->name, urb, -err);
+ kfree(urb->setup_packet);
+ usb_unanchor_urb(urb);
+ usb_free_urb(urb);
break;
+ }

drv_data->tx_in_flight++;
+ usb_free_urb(urb);
+ }
+ /* Cleanup the rest deferred urbs. */
+ while ((urb = usb_get_from_anchor(&drv_data->deferred))) {
+ kfree(urb->setup_packet);
+ usb_free_urb(urb);
}
- usb_scuttle_anchored_urbs(&drv_data->deferred);
}

static int nfcmrvl_resume(struct usb_interface *intf)
--
2.17.1


2022-06-06 05:41:52

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH 1/1] nfc: nfcmrvl: Fix memory leak in nfcmrvl_play_deferred

On Sat, 4 Jun 2022 00:31:27 +0800 Xiaohui Zhang wrote:
> From: hifoolno <[email protected]>
>
> Currently usb_submit_urb is called directly to submit deferred tx
> urbs after unanchor them.
>
> So the usb_giveback_urb_bh would failed to unref it in usb_unanchor_urb
> and cause memory leak.
>
> Put those urbs in tx_anchor to avoid the leak, and also fix the error
> handling.
>
> Signed-off-by: hifoolno <[email protected]>

hifoolno is too close to "Hi Fool" for comfort. Is that really your
name?

Also the patch doesn't build.