| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <200905061917.09454.oliver@neukum.org>
Date: Wed, 6 May 2009 19:17:09 +0200
From: Oliver Neukum <oliver@...kum.org>
To: Alan Cox <alan@...rguk.ukuu.org.uk>
Cc: Alan Stern <stern@...land.harvard.edu>,
Jason Wessel <jason.wessel@...driver.com>, greg@...ah.com,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/5] usb_debug: EXPERIMENTAL - poll hcd device to force writes
Am Mittwoch, 6. Mai 2009 17:41:41 schrieb Alan Cox:
> Stuff data into the chip and if need be maintain an internal buffer. One
> of the problems with the serial stuff is that the buffering algorithms
> you need to packetize serial streams without poor behaviour or overruns
> are non trivial and the USB serial layer doesn't provide anything
> remotely resembling sanity.
Something a bit a like this:
commit a02639fe7d3f9788263305cff0669eac91f54002
Author: Oliver Neukum <oneukum@...ux-d698.(none)>
Date: Wed May 6 19:14:30 2009 +0200
terrible implementation of usb serial write buffering
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 3a09777..83acde4 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -801,8 +801,13 @@ kevent (struct work_struct *work)
if (test_bit(EVENT_DEV_WAKING, &dev->flags)) {
status = usb_autopm_get_interface(dev->intf);
clear_bit(EVENT_DEV_WAKING, &dev->flags);
- if (!status)
+ if (!status) {
usb_autopm_put_interface(dev->intf);
+ } else {
+ dev_kfree_skb_any((struct sk_buff*)dev->deferred->context);
+ usb_free_urb(dev->deferred);
+ netif_start_queue(dev->net);
+ }
}
/* usb_clear_halt() needs a thread context */
if (test_bit (EVENT_TX_HALT, &dev->flags)) {
@@ -1351,12 +1356,13 @@ int usbnet_resume (struct usb_interface *intf)
clear_bit(EVENT_DEV_ASLEEP, &dev->flags);
spin_unlock_irq(&dev->txq.lock);
if (res) {
+ skb = (struct sk_buff *)res->context;
retval = usb_submit_urb(res, GFP_NOIO);
if (retval < 0) {
+ dev_kfree_skb_any(skb);
usb_free_urb(res);
netif_start_queue(dev->net);
} else {
- skb = (struct sk_buff *)res->context;
dev->net->trans_start = jiffies;
__skb_queue_tail (&dev->txq, skb);
if (!(dev->txq.qlen >= TX_QLEN(dev)))
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 4cec990..31dcbf7 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -191,12 +191,14 @@ void usb_serial_generic_close(struct tty_struct *tty,
generic_cleanup(port);
}
+#define UPPER_WRITE_LIMIT 4
int usb_serial_generic_write(struct tty_struct *tty,
struct usb_serial_port *port, const unsigned char *buf, int count)
{
struct usb_serial *serial = port->serial;
+ struct urb *write_urb;
int result;
- unsigned char *data;
+ u8 *data;
dbg("%s - port %d", __func__, port->number);
@@ -209,41 +211,66 @@ int usb_serial_generic_write(struct tty_struct *tty,
if (serial->num_bulk_out) {
unsigned long flags;
spin_lock_irqsave(&port->lock, flags);
- if (port->write_urb_busy) {
+ if (port->writes_in_flight >= UPPER_WRITE_LIMIT) {
+ if (!port->reserve_buffer) {
+ spin_unlock_irqrestore(&port->lock, flags);
+ return 0;
+ }
+ count = (count > port->bulk_out_size - port->reserve_filled) ?
+ port->bulk_out_size - port->reserve_filled
+ : count;
+ memcpy(port->reserve_buffer + port->reserve_filled,
+ buf, count);
+ port->reserve_filled += count;
spin_unlock_irqrestore(&port->lock, flags);
- dbg("%s - already writing", __func__);
- return 0;
+ dbg("%s - hitting reserves %d bytes left",
+ __func__, port->bulk_out_size - port->reserve_filled);
+ return count;
+ } else {
+ count = (count > port->bulk_out_size) ?
+ port->bulk_out_size : count;
+ write_urb = usb_alloc_urb(0, GFP_ATOMIC);
+ if (!write_urb) {
+ spin_unlock_irqrestore(&port->lock, flags);
+ return 0;
+ }
+ data = kmalloc(count, GFP_ATOMIC);
+ if (!data) {
+ usb_free_urb(write_urb);
+ spin_unlock_irqrestore(&port->lock, flags);
+ return 0;
+ }
}
- port->write_urb_busy = 1;
+ port->writes_in_flight++;
spin_unlock_irqrestore(&port->lock, flags);
- count = (count > port->bulk_out_size) ?
- port->bulk_out_size : count;
-
- memcpy(port->write_urb->transfer_buffer, buf, count);
- data = port->write_urb->transfer_buffer;
+ memcpy(data, buf, count);
+ write_urb->transfer_buffer = data;
usb_serial_debug_data(debug, &port->dev, __func__, count, data);
/* set up our urb */
- usb_fill_bulk_urb(port->write_urb, serial->dev,
+ usb_fill_bulk_urb(write_urb, serial->dev,
usb_sndbulkpipe(serial->dev,
port->bulk_out_endpointAddress),
- port->write_urb->transfer_buffer, count,
+ data, count,
((serial->type->write_bulk_callback) ?
serial->type->write_bulk_callback :
usb_serial_generic_write_bulk_callback),
port);
/* send the data out the bulk port */
- port->write_urb_busy = 1;
- result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
+ result = usb_submit_urb(write_urb, GFP_ATOMIC);
if (result) {
dev_err(&port->dev,
"%s - failed submitting write urb, error %d\n",
__func__, result);
/* don't have to grab the lock here, as we will
retry if != 0 */
- port->write_urb_busy = 0;
+ kfree(data);
+ usb_free_urb(write_urb);
+ spin_lock_irqsave(&port->lock, flags);
+ port->writes_in_flight--;
+ spin_unlock_irqrestore(&port->lock, flags);
} else
result = count;
@@ -264,7 +291,7 @@ int usb_serial_generic_write_room(struct tty_struct *tty)
/* FIXME: Locking */
if (serial->num_bulk_out) {
- if (!(port->write_urb_busy))
+ if (port->writes_in_flight < UPPER_WRITE_LIMIT)
room = port->bulk_out_size;
}
@@ -282,8 +309,10 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct
*tty)
/* FIXME: Locking */
if (serial->num_bulk_out) {
- if (port->write_urb_busy)
- chars = port->write_urb->transfer_buffer_length;
+ if (port->writes_in_flight < UPPER_WRITE_LIMIT)
+ chars = port->bulk_out_size * UPPER_WRITE_LIMIT / 2;
+ else
+ chars = port->bulk_out_size * UPPER_WRITE_LIMIT;
}
dbg("%s - returns %d", __func__, chars);
@@ -369,7 +398,12 @@ void usb_serial_generic_write_bulk_callback(struct urb
*urb)
dbg("%s - port %d", __func__, port->number);
- port->write_urb_busy = 0;
+ kfree(urb->transfer_buffer);
+ spin_lock(&port->lock);
+ if (!port->reserve_filled || status)
+ port->writes_in_flight--;
+ spin_unlock(&port->lock);
+
if (status) {
dbg("%s - nonzero write bulk status received: %d",
__func__, status);
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 0a566ee..33d6ea4 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -519,10 +519,49 @@ static void usb_serial_port_work(struct work_struct
*work)
{
struct usb_serial_port *port =
container_of(work, struct usb_serial_port, work);
+ struct usb_serial *serial = port->serial;
+ struct urb *urb;
struct tty_struct *tty;
+ u8 *new_reserve;
+ int err;
+ unsigned long flags;
dbg("%s - port %d", __func__, port->number);
+ if (port->reserve_filled) {
+ /*
+ * this cannot be done any later and here we can sleep
+ * to allocate memory
+ */
+ new_reserve = kmalloc(port->bulk_out_size, GFP_KERNEL);
+ urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!new_reserve || !urb) {
+ /* now we drop data */
+ kfree(new_reserve);
+failure:
+ usb_free_urb(urb);
+ spin_lock_irqsave(&port->lock, flags);
+ port->writes_in_flight--;
+ port->reserve_filled = 0;
+ spin_unlock_irqrestore(&port->lock, flags);
+ goto bail_buffer_exhaustion;
+ }
+ urb->transfer_buffer = port->reserve_buffer;
+ usb_fill_bulk_urb(urb, serial->dev,
+ usb_rcvbulkpipe(serial->dev,
+ port->bulk_in_endpointAddress),
+ port->reserve_buffer,
+ port->reserve_filled,
+ usb_serial_generic_write_bulk_callback,
+ port);
+ port->reserve_buffer = new_reserve;
+ err = usb_submit_urb(urb, GFP_KERNEL);
+ if (err < 0)
+ goto failure;
+ port->reserve_filled = 0;
+ }
+
+bail_buffer_exhaustion:
tty = tty_port_tty_get(&port->port);
if (!tty)
return;
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 625e9e4..4e0afed 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -88,7 +88,9 @@ struct usb_serial_port {
unsigned char *bulk_out_buffer;
int bulk_out_size;
struct urb *write_urb;
- int write_urb_busy;
+ int writes_in_flight;
+ int reserve_filled;
+ u8 *reserve_buffer;
__u8 bulk_out_endpointAddress;
wait_queue_head_t write_wait;
Just for discussion.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists