* [PATCH v3] usb: misc: legousbtower: Fix buffers on stack
[not found] <20170425222842.60efc758@gmail.com>
@ 2017-04-25 19:49 ` Maksim Salau
2017-04-26 9:29 ` Greg Kroah-Hartman
2017-05-04 12:36 ` Heikki Krogerus
0 siblings, 2 replies; 5+ messages in thread
From: Maksim Salau @ 2017-04-25 19:49 UTC (permalink / raw)
To: Juergen Stuber, Greg Kroah-Hartman, legousb-devel, linux-usb,
linux-kernel, Alfredo Rafael Vicente Boix
Cc: Maksim Salau
Allocate buffers on HEAP instead of STACK for local structures
that are to be received using usb_control_msg().
Signed-off-by: Maksim Salau <maksim.salau@gmail.com>
---
Changes in v3:
* rebased against usb-next;
* removed Tested-by: Alfredo Rafael Vicente Boix <alviboi@gmail.com>;
* removed Cc: stable@vger.kernel.org
since this patch doesn't apply against v4.10.12
Changes in v2:
* made checkpatch happy with the format string passed to dev_info
in tower_probe() (merged two parts into a single string literal);
* changed commit message to better reflect location of the module;
was: USB: legousbtower: Fix buffers on stack;
* added Tested-by: Alfredo Rafael Vicente Boix <alviboi@gmail.com>
and Cc: stable@vger.kernel.org
drivers/usb/misc/legousbtower.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 201c9c3..aa3c280 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -317,9 +317,16 @@ static int tower_open (struct inode *inode, struct file *file)
int subminor;
int retval = 0;
struct usb_interface *interface;
- struct tower_reset_reply reset_reply;
+ struct tower_reset_reply *reset_reply;
int result;
+ reset_reply = kmalloc(sizeof(*reset_reply), GFP_KERNEL);
+
+ if (!reset_reply) {
+ retval = -ENOMEM;
+ goto exit;
+ }
+
nonseekable_open(inode, file);
subminor = iminor(inode);
@@ -364,8 +371,8 @@ static int tower_open (struct inode *inode, struct file *file)
USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
0,
0,
- &reset_reply,
- sizeof(reset_reply),
+ reset_reply,
+ sizeof(*reset_reply),
1000);
if (result < 0) {
dev_err(&dev->udev->dev,
@@ -406,6 +413,7 @@ static int tower_open (struct inode *inode, struct file *file)
mutex_unlock(&dev->lock);
exit:
+ kfree(reset_reply);
return retval;
}
@@ -806,7 +814,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
struct device *idev = &interface->dev;
struct usb_device *udev = interface_to_usbdev(interface);
struct lego_usb_tower *dev = NULL;
- struct tower_get_version_reply get_version_reply;
+ struct tower_get_version_reply *get_version_reply = NULL;
int retval = -ENOMEM;
int result;
@@ -871,6 +879,13 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
dev->interrupt_in_interval = interrupt_in_interval ? interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
dev->interrupt_out_interval = interrupt_out_interval ? interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
+ get_version_reply = kmalloc(sizeof(*get_version_reply), GFP_KERNEL);
+
+ if (!get_version_reply) {
+ retval = -ENOMEM;
+ goto error;
+ }
+
/* get the firmware version and log it */
result = usb_control_msg (udev,
usb_rcvctrlpipe(udev, 0),
@@ -878,18 +893,19 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
0,
0,
- &get_version_reply,
- sizeof(get_version_reply),
+ get_version_reply,
+ sizeof(*get_version_reply),
1000);
if (result < 0) {
dev_err(idev, "LEGO USB Tower get version control request failed\n");
retval = result;
goto error;
}
- dev_info(&interface->dev, "LEGO USB Tower firmware version is %d.%d "
- "build %d\n", get_version_reply.major,
- get_version_reply.minor,
- le16_to_cpu(get_version_reply.build_no));
+ dev_info(&interface->dev,
+ "LEGO USB Tower firmware version is %d.%d build %d\n",
+ get_version_reply->major,
+ get_version_reply->minor,
+ le16_to_cpu(get_version_reply->build_no));
/* we can register the device now, as it is ready */
usb_set_intfdata (interface, dev);
@@ -913,6 +929,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
return retval;
error:
+ kfree(get_version_reply);
tower_delete(dev);
return retval;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: misc: legousbtower: Fix buffers on stack
2017-04-25 19:49 ` [PATCH v3] usb: misc: legousbtower: Fix buffers on stack Maksim Salau
@ 2017-04-26 9:29 ` Greg Kroah-Hartman
2017-04-27 6:30 ` Maksim Salau
2017-05-04 12:36 ` Heikki Krogerus
1 sibling, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2017-04-26 9:29 UTC (permalink / raw)
To: Maksim Salau
Cc: Juergen Stuber, legousb-devel, linux-usb, linux-kernel,
Alfredo Rafael Vicente Boix
On Tue, Apr 25, 2017 at 10:49:21PM +0300, Maksim Salau wrote:
> Allocate buffers on HEAP instead of STACK for local structures
> that are to be received using usb_control_msg().
>
> Signed-off-by: Maksim Salau <maksim.salau@gmail.com>
> Tested-by: Alfredo Rafael Vicente Boix <alviboi@gmail.com>;
> Cc: stable <stable@vger.kernel.org>
>
> ---
> Changes in v3:
> * rebased against usb-next;
> * removed Tested-by: Alfredo Rafael Vicente Boix <alviboi@gmail.com>;
I added this back, as it matters, and your change from the previous
version was trivial.
> * removed Cc: stable@vger.kernel.org
> since this patch doesn't apply against v4.10.12
I added this back as well :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: misc: legousbtower: Fix buffers on stack
2017-04-26 9:29 ` Greg Kroah-Hartman
@ 2017-04-27 6:30 ` Maksim Salau
0 siblings, 0 replies; 5+ messages in thread
From: Maksim Salau @ 2017-04-27 6:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Juergen Stuber, legousb-devel, linux-usb, linux-kernel,
Alfredo Rafael Vicente Boix
> > * removed Tested-by: Alfredo Rafael Vicente Boix <alviboi@gmail.com>;
>
> I added this back, as it matters, and your change from the previous
> version was trivial.
>
> > * removed Cc: stable@vger.kernel.org
> > since this patch doesn't apply against v4.10.12
>
> I added this back as well :)
Thanks, Greg!
I was not sure about how strict are the rules about these tags.
Maksim.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: misc: legousbtower: Fix buffers on stack
2017-04-25 19:49 ` [PATCH v3] usb: misc: legousbtower: Fix buffers on stack Maksim Salau
2017-04-26 9:29 ` Greg Kroah-Hartman
@ 2017-05-04 12:36 ` Heikki Krogerus
2017-05-04 19:11 ` Maksim Salau
1 sibling, 1 reply; 5+ messages in thread
From: Heikki Krogerus @ 2017-05-04 12:36 UTC (permalink / raw)
To: Maksim Salau
Cc: Juergen Stuber, Greg Kroah-Hartman, legousb-devel, linux-usb,
linux-kernel, Alfredo Rafael Vicente Boix
Hi Maksim,
Sorry for commenting this so late but..
On Tue, Apr 25, 2017 at 10:49:21PM +0300, Maksim Salau wrote:
> @@ -806,7 +814,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> struct device *idev = &interface->dev;
> struct usb_device *udev = interface_to_usbdev(interface);
> struct lego_usb_tower *dev = NULL;
> - struct tower_get_version_reply get_version_reply;
> + struct tower_get_version_reply *get_version_reply = NULL;
> int retval = -ENOMEM;
> int result;
>
> @@ -871,6 +879,13 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> dev->interrupt_in_interval = interrupt_in_interval ? interrupt_in_interval : dev->interrupt_in_endpoint->bInterval;
> dev->interrupt_out_interval = interrupt_out_interval ? interrupt_out_interval : dev->interrupt_out_endpoint->bInterval;
>
> + get_version_reply = kmalloc(sizeof(*get_version_reply), GFP_KERNEL);
> +
> + if (!get_version_reply) {
> + retval = -ENOMEM;
> + goto error;
> + }
> +
> /* get the firmware version and log it */
> result = usb_control_msg (udev,
> usb_rcvctrlpipe(udev, 0),
> @@ -878,18 +893,19 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
> USB_TYPE_VENDOR | USB_DIR_IN | USB_RECIP_DEVICE,
> 0,
> 0,
> - &get_version_reply,
> - sizeof(get_version_reply),
> + get_version_reply,
> + sizeof(*get_version_reply),
> 1000);
> if (result < 0) {
> dev_err(idev, "LEGO USB Tower get version control request failed\n");
> retval = result;
> goto error;
> }
> - dev_info(&interface->dev, "LEGO USB Tower firmware version is %d.%d "
> - "build %d\n", get_version_reply.major,
> - get_version_reply.minor,
> - le16_to_cpu(get_version_reply.build_no));
> + dev_info(&interface->dev,
> + "LEGO USB Tower firmware version is %d.%d build %d\n",
> + get_version_reply->major,
> + get_version_reply->minor,
> + le16_to_cpu(get_version_reply->build_no));
>
> /* we can register the device now, as it is ready */
> usb_set_intfdata (interface, dev);
> @@ -913,6 +929,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
Don't you need to free get_version_reply here?
> return retval;
>
> error:
> + kfree(get_version_reply);
> tower_delete(dev);
> return retval;
> }
Thanks,
--
heikki
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] usb: misc: legousbtower: Fix buffers on stack
2017-05-04 12:36 ` Heikki Krogerus
@ 2017-05-04 19:11 ` Maksim Salau
0 siblings, 0 replies; 5+ messages in thread
From: Maksim Salau @ 2017-05-04 19:11 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Juergen Stuber, Greg Kroah-Hartman, legousb-devel, linux-usb,
linux-kernel, Alfredo Rafael Vicente Boix
> > @@ -913,6 +929,7 @@ static int tower_probe (struct usb_interface *interface, const struct usb_device
>
> Don't you need to free get_version_reply here?
>
> > return retval;
> >
> > error:
> > + kfree(get_version_reply);
> > tower_delete(dev);
> > return retval;
> > }
Thank you very much, Heikki!
I was so focused on failure cases, that missed memory leak in the case
when all calls succeeded.
I'll prepare a patch shortly to fix this.
Regards,
Maksim.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-05-04 19:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20170425222842.60efc758@gmail.com>
2017-04-25 19:49 ` [PATCH v3] usb: misc: legousbtower: Fix buffers on stack Maksim Salau
2017-04-26 9:29 ` Greg Kroah-Hartman
2017-04-27 6:30 ` Maksim Salau
2017-05-04 12:36 ` Heikki Krogerus
2017-05-04 19:11 ` Maksim Salau
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).