[PATCH][resend] MMC, vub300: Resolve mem leak in vub300_probe() and simplify the code a bit
In drivers/mmc/host/vub300.c::vub300_probe() we need both
'command_out_urb' and 'command_res_urb'. Currently we fail to free the
former if allocating the latter fails. Fix that and simplify the code
a bit at the same time by just doing both allocations and if either
fails then free both - usb_free_urb() deals gracefully with NULL
pointers, so this is safe.
We also initialize 'retval' to '-ENOMEM' when we declare the variable,
so there's no reason to re-set it to '-ENOMEM' before jumping to
'error0:' when one of the initial usb_alloc_urb() calls fail().
Also rename the 'error[0145]:' labels to be just 'error0' and
'error1'.
Signed-off-by: Jesper Juhl
---
drivers/mmc/host/vub300.c | 29 +++--
1 files changed, 11 insertions(+), 18 deletions(-)
This is a resend. I originally submitted this back in January:
http://lkml.indiana.edu/hypermail/linux/kernel/1201.3/00305.html
but got no response.
Note:
I have no real way to actually test this patch, so it is compile tested
only. Please review carefully before applying.
Also please keep me on Cc: when replying.
diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index cb9f361..3f59fe1 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2116,23 +2116,18 @@ static int vub300_probe(struct usb_interface *interface,
udev->descriptor.idVendor, udev->descriptor.idProduct,
manufacturer, product, serial_number);
command_out_urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!command_out_urb) {
- retval = -ENOMEM;
- dev_err(>dev, "not enough memory for command_out_urb\n");
- goto error0;
- }
command_res_urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!command_res_urb) {
+ if (!command_res_urb || !command_out_urb) {
retval = -ENOMEM;
- dev_err(>dev, "not enough memory for command_res_urb\n");
- goto error1;
+ dev_err(>dev, "not enough memory for command urbs\n");
+ goto error0;
}
/* this also allocates memory for our VUB300 mmc host device */
mmc = mmc_alloc_host(sizeof(struct vub300_mmc_host), >dev);
if (!mmc) {
retval = -ENOMEM;
dev_err(>dev, "not enough memory for the mmc_host\n");
- goto error4;
+ goto error0;
}
/* MMC core transfer sizes tunable parameters */
mmc->caps = 0;
@@ -2285,7 +2280,7 @@ static int vub300_probe(struct usb_interface *interface,
dev_err(>udev->dev,
"Could not find two sets of bulk-in/out endpoint pairs\n");
retval = -EINVAL;
- goto error5;
+ goto error1;
}
retval =
usb_control_msg(vub300->udev, usb_rcvctrlpipe(vub300->udev, 0),
@@ -2294,14 +2289,14 @@ static int vub300_probe(struct usb_interface *interface,
0x, 0x, >hc_info,
sizeof(vub300->hc_info), HZ);
if (retval < 0)
- goto error5;
+ goto error1;
retval =
usb_control_msg(vub300->udev, usb_rcvctrlpipe(vub300->udev, 0),
SET_ROM_WAIT_STATES,
USB_DIR_OUT | USB_TYPE_VENDOR |
USB_RECIP_DEVICE,
firmware_rom_wait_states, 0x, NULL, 0, HZ);
if (retval < 0)
- goto error5;
+ goto error1;
dev_info(>udev->dev,
"operating_mode = %s %s %d MHz %s %d byte USB packets\n",
(mmc->caps & MMC_CAP_SDIO_IRQ) ? "IRQs" : "POLL",
@@ -2316,14 +2311,14 @@ static int vub300_probe(struct usb_interface *interface,
0x, 0x, >system_port_status,
sizeof(vub300->system_port_status), HZ);
if (retval < 0) {
- goto error4;
+ goto error0;
} else if (sizeof(vub300->system_port_status) == retval) {
vub300->card_present =
(0x0001 & vub300->system_port_status.port_flags) ? 1 :
0;
vub300->read_only =
(0x0010 & vub300->system_port_status.port_flags) ? 1 :
0;
} else {
- goto error4;
+ goto error0;
}
usb_set_intfdata(interface, vub300);
INIT_DELAYED_WORK(>pollwork, vub300_pollwork_thread);
@@ -2351,17 +2346,15 @@ static int vub300_probe(struct usb_interface *interface,
interface_to_InterfaceNumber(interface));
mmc_add_host(mmc);
return 0;
-error5:
+error1:
mmc_free_host(mmc);
/*
* and hence also frees vub300
* which is contained at the end of struct mmc
*/
-error4:
+error0:
usb_free_urb(command_res_urb);
-error1:
[PATCH][resend] MMC, vub300: Resolve mem leak in vub300_probe() and simplify the code a bit
In drivers/mmc/host/vub300.c::vub300_probe() we need both
'command_out_urb' and 'command_res_urb'. Currently we fail to free the
former if allocating the latter fails. Fix that and simplify the code
a bit at the same time by just doing both allocations and if either
fails then free both - usb_free_urb() deals gracefully with NULL
pointers, so this is safe.
We also initialize 'retval' to '-ENOMEM' when we declare the variable,
so there's no reason to re-set it to '-ENOMEM' before jumping to
'error0:' when one of the initial usb_alloc_urb() calls fail().
Also rename the 'error[0145]:' labels to be just 'error0' and
'error1'.
Signed-off-by: Jesper Juhl j...@chaosbits.net
---
drivers/mmc/host/vub300.c | 29 +++--
1 files changed, 11 insertions(+), 18 deletions(-)
This is a resend. I originally submitted this back in January:
http://lkml.indiana.edu/hypermail/linux/kernel/1201.3/00305.html
but got no response.
Note:
I have no real way to actually test this patch, so it is compile tested
only. Please review carefully before applying.
Also please keep me on Cc: when replying.
diff --git a/drivers/mmc/host/vub300.c b/drivers/mmc/host/vub300.c
index cb9f361..3f59fe1 100644
--- a/drivers/mmc/host/vub300.c
+++ b/drivers/mmc/host/vub300.c
@@ -2116,23 +2116,18 @@ static int vub300_probe(struct usb_interface *interface,
udev-descriptor.idVendor, udev-descriptor.idProduct,
manufacturer, product, serial_number);
command_out_urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!command_out_urb) {
- retval = -ENOMEM;
- dev_err(udev-dev, not enough memory for command_out_urb\n);
- goto error0;
- }
command_res_urb = usb_alloc_urb(0, GFP_KERNEL);
- if (!command_res_urb) {
+ if (!command_res_urb || !command_out_urb) {
retval = -ENOMEM;
- dev_err(udev-dev, not enough memory for command_res_urb\n);
- goto error1;
+ dev_err(udev-dev, not enough memory for command urbs\n);
+ goto error0;
}
/* this also allocates memory for our VUB300 mmc host device */
mmc = mmc_alloc_host(sizeof(struct vub300_mmc_host), udev-dev);
if (!mmc) {
retval = -ENOMEM;
dev_err(udev-dev, not enough memory for the mmc_host\n);
- goto error4;
+ goto error0;
}
/* MMC core transfer sizes tunable parameters */
mmc-caps = 0;
@@ -2285,7 +2280,7 @@ static int vub300_probe(struct usb_interface *interface,
dev_err(vub300-udev-dev,
Could not find two sets of bulk-in/out endpoint pairs\n);
retval = -EINVAL;
- goto error5;
+ goto error1;
}
retval =
usb_control_msg(vub300-udev, usb_rcvctrlpipe(vub300-udev, 0),
@@ -2294,14 +2289,14 @@ static int vub300_probe(struct usb_interface *interface,
0x, 0x, vub300-hc_info,
sizeof(vub300-hc_info), HZ);
if (retval 0)
- goto error5;
+ goto error1;
retval =
usb_control_msg(vub300-udev, usb_rcvctrlpipe(vub300-udev, 0),
SET_ROM_WAIT_STATES,
USB_DIR_OUT | USB_TYPE_VENDOR |
USB_RECIP_DEVICE,
firmware_rom_wait_states, 0x, NULL, 0, HZ);
if (retval 0)
- goto error5;
+ goto error1;
dev_info(vub300-udev-dev,
operating_mode = %s %s %d MHz %s %d byte USB packets\n,
(mmc-caps MMC_CAP_SDIO_IRQ) ? IRQs : POLL,
@@ -2316,14 +2311,14 @@ static int vub300_probe(struct usb_interface *interface,
0x, 0x, vub300-system_port_status,
sizeof(vub300-system_port_status), HZ);
if (retval 0) {
- goto error4;
+ goto error0;
} else if (sizeof(vub300-system_port_status) == retval) {
vub300-card_present =
(0x0001 vub300-system_port_status.port_flags) ? 1 :
0;
vub300-read_only =
(0x0010 vub300-system_port_status.port_flags) ? 1 :
0;
} else {
- goto error4;
+ goto error0;
}
usb_set_intfdata(interface, vub300);
INIT_DELAYED_WORK(vub300-pollwork, vub300_pollwork_thread);
@@ -2351,17 +2346,15 @@ static int vub300_probe(struct usb_interface *interface,
interface_to_InterfaceNumber(interface));
mmc_add_host(mmc);
return 0;
-error5:
+error1:
mmc_free_host(mmc);
/*
* and hence also frees vub300
* which is contained at the end of struct mmc
*/
-error4:
+error0:
