Re: [PATCH] Memory leak in visor.c and ftdi_sio.c
From: nardelli (jnardelli_at_infosciences.com)
Date: 06/07/04
- Previous message: Russell King: "Re: [RFC] ASLA design, depth of code review and lack thereof"
- In reply to: Greg KH: "Re: [PATCH] Memory leak in visor.c and ftdi_sio.c"
- Next in thread: Richard B. Johnson: "Re: [PATCH] Memory leak in visor.c and ftdi_sio.c"
- Reply: Richard B. Johnson: "Re: [PATCH] Memory leak in visor.c and ftdi_sio.c"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Mon, 07 Jun 2004 10:19:30 -0400 To: Greg KH <greg@kroah.com>
Greg KH wrote:
> On Fri, Jun 04, 2004 at 05:34:41PM +0100, Ian Abbott wrote:
>
>>On 04/06/2004 15:59, nardelli wrote:
>>
>>A related problem with the current implementation is that is easy to
>>run out of memory by running something similar to this:
>>
>> # cat /dev/zero > /dev/ttyUSB0
>>
>>That affects both the ftdi_sio and visor drivers.
>
>
> Care to try out the following (build test only) patch to the visor
> driver to see if it prevents this from happening? I don't have a
> working visor right now to test it out myself :(
>
> Oops, ignore the fact that we never free the structure on disconnect, I
> see that now...
>
> thanks,
>
> greg k-h
>
>
> ===== drivers/usb/serial/visor.c 1.114 vs edited =====
> --- 1.114/drivers/usb/serial/visor.c Fri Jun 4 07:13:10 2004
> +++ edited/drivers/usb/serial/visor.c Fri Jun 4 17:12:53 2004
...
Just curious - is there something special about 42? Grepping wasn't
very useful, as numbers like this are scattered all over the place.
> +/* number of outstanding urbs to prevent userspace DoS from happening */
> +#define URB_UPPER_LIMIT 42
...
>
> static int visor_write (struct usb_serial_port *port, int from_user, const unsigned char *buf, int count)
> {
> + struct visor_private *priv = usb_get_serial_port_data(port);
> struct usb_serial *serial = port->serial;
> struct urb *urb;
> unsigned char *buffer;
> + unsigned long flags;
> int status;
>
> dbg("%s - port %d", __FUNCTION__, port->number);
>
> + spin_lock_irqsave(&priv->lock, flags);
> + if (priv->outstanding_urbs > URB_UPPER_LIMIT) {
> + spin_unlock_irqrestore(&priv->lock, flags);
> + dev_dbg(&port->dev, "write limit hit\n");
> + return 0;
> + }
> + ++priv->outstanding_urbs;
> + spin_unlock_irqrestore(&priv->lock, flags);
> +
> buffer = kmalloc (count, GFP_ATOMIC);
> if (!buffer) {
> dev_err(&port->dev, "out of memory\n");
> @@ -520,7 +545,10 @@
> count = status;
> kfree (buffer);
> } else {
> - bytes_out += count;
> + spin_lock_irqsave(&priv->lock, flags);
> + ++priv->outstanding_urbs;
> + priv->bytes_out += count;
> + spin_unlock_irqrestore(&priv->lock, flags);
> }
>
> /* we are done with this urb, so let the host driver
Removing the first of two priv->outstanding_urbs increments in
visor_write (I assume that was your intention) produced very nice
results ;-)
1) When being flooded, after the initial bunch of URBs were sent,
only 1 per second was sent, and it appeared that all of them were
being freed.
2) Even after the flood, the driver survived, and backups were
possible after the device was reconnected.
I'm fairly ignorant of most of the lower level usb infrastructure
(hcd, hub, ehci, etc), so I'm not sure what the root cause (ignoring
the patch above) of the completion handler not being called might be.
I would suspect overflow of some callback list, but that's just a
guess. Do you have any ideas on what this might be, and could this
be a problem in other devices?
-- Joe Nardelli jnardelli@infosciences.com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
- Previous message: Russell King: "Re: [RFC] ASLA design, depth of code review and lack thereof"
- In reply to: Greg KH: "Re: [PATCH] Memory leak in visor.c and ftdi_sio.c"
- Next in thread: Richard B. Johnson: "Re: [PATCH] Memory leak in visor.c and ftdi_sio.c"
- Reply: Richard B. Johnson: "Re: [PATCH] Memory leak in visor.c and ftdi_sio.c"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
