Bjørn Mork <bjorn@xxxxxxx> writes: > Oliver Neukum <oneukum@xxxxxxx> writes: > >> From a1d8ec5966fb20dcc30f40ee3a253f3228d7e716 Mon Sep 17 00:00:00 2001 >> From: Oliver Neukum <oliver@xxxxxxxxxx> >> Date: Fri, 27 Apr 2012 14:36:37 +0200 >> Subject: [PATCH 2/2] cdc-wdm: fix memory leak >> >> cleanup() is not called if the last close() comes after >> disconnect(). That leads to a memory leak. Rectified >> by checking for an earlier disconnect() in release() >> >> Signed-off-by: Oliver Neukum <oneukum@xxxxxxx> >> --- >> drivers/usb/class/cdc-wdm.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c >> index 0209b46..6bf1a40 100644 >> --- a/drivers/usb/class/cdc-wdm.c >> +++ b/drivers/usb/class/cdc-wdm.c >> @@ -625,6 +625,8 @@ static int wdm_release(struct inode *inode, struct file *file) >> kill_urbs(desc); >> if (!test_bit(WDM_DISCONNECTING, &desc->flags)) >> desc->manage_power(desc->intf, 0); >> + else >> + cleanup(desc); >> } >> mutex_unlock(&wdm_mutex); >> return 0; > > Looks good to me. BTW, would it make sense to add a couple of debug messages to make it easier to follow the program flow here? Something like this maybe?: diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index da13bbc..b6010ae 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -623,10 +623,12 @@ static int wdm_release(struct inode *inode, struct file *file) if (!desc->count) { dev_dbg(&desc->intf->dev, "wdm_release: cleanup"); kill_urbs(desc); - if (!test_bit(WDM_DISCONNECTING, &desc->flags)) + if (!test_bit(WDM_DISCONNECTING, &desc->flags)) { desc->manage_power(desc->intf, 0); - else + } else { + dev_dbg(&desc->intf->dev, "%s: device gone - cleaning up\n", __func__); cleanup(desc); + } } mutex_unlock(&wdm_mutex); return 0; @@ -900,6 +902,8 @@ static void wdm_disconnect(struct usb_interface *intf) mutex_unlock(&desc->rlock); if (!desc->count) cleanup(desc); + else + dev_dbg(&intf->dev, "%s: %d open files - postponing cleanup\n", __func__, desc->count); mutex_unlock(&wdm_mutex); } Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html