hso: Fix free of mutexes still in use.
authorDenis Joseph Barrow <D.Barow@option.com>
Tue, 25 Nov 2008 08:33:13 +0000 (00:33 -0800)
committerDavid S. Miller <davem@davemloft.net>
Tue, 25 Nov 2008 08:33:13 +0000 (00:33 -0800)
A new structure hso_mutex_table had to be declared statically
& used as as hso_device mutex_lock(&serial->parent->mutex) etc
is freed in hso_serial_open & hso_serial_close by kref_put while
the mutex is still in use.

This is a substantial change but should make the driver much stabler.

Signed-off-by: Denis Joseph Barrow <D.Barow@option.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/usb/hso.c

index 2c17243..cc37a2e 100644 (file)
@@ -230,6 +230,11 @@ struct hso_serial {
        struct work_struct    retry_unthrottle_workqueue;
 };
 
+struct hso_mutex_t {
+       struct mutex mutex;
+       u8 allocated;
+};
+
 struct hso_device {
        union {
                struct hso_serial *dev_serial;
@@ -248,7 +253,7 @@ struct hso_device {
 
        struct device *dev;
        struct kref ref;
-       struct mutex mutex;
+       struct hso_mutex_t *mutex;
 };
 
 /* Type of interface */
@@ -364,6 +369,13 @@ static struct hso_device *network_table[HSO_MAX_NET_DEVICES];
 static spinlock_t serial_table_lock;
 static struct ktermios *hso_serial_termios[HSO_SERIAL_TTY_MINORS];
 static struct ktermios *hso_serial_termios_locked[HSO_SERIAL_TTY_MINORS];
+/* hso_mutex_table has to be declared statically as hso_device
+ * is freed in hso_serial_open & hso_serial_close while
+ * the mutex is still in use.
+ */
+#define HSO_NUM_MUTEXES (HSO_SERIAL_TTY_MINORS+HSO_MAX_NET_DEVICES)
+static struct hso_mutex_t hso_mutex_table[HSO_NUM_MUTEXES];
+static spinlock_t hso_mutex_lock;
 
 static const s32 default_port_spec[] = {
        HSO_INTF_MUX | HSO_PORT_NETWORK,
@@ -604,6 +616,34 @@ static void set_serial_by_index(unsigned index, struct hso_serial *serial)
        spin_unlock_irqrestore(&serial_table_lock, flags);
 }
 
+
+static struct hso_mutex_t *hso_get_free_mutex(void)
+{
+       int index;
+       struct hso_mutex_t *curr_hso_mutex;
+
+       spin_lock(&hso_mutex_lock);
+       for (index = 0; index < HSO_NUM_MUTEXES; index++) {
+               curr_hso_mutex = &hso_mutex_table[index];
+                       if (!curr_hso_mutex->allocated) {
+                               curr_hso_mutex->allocated = 1;
+                               spin_unlock(&hso_mutex_lock);
+                               return curr_hso_mutex;
+                       }
+       }
+       printk(KERN_ERR "BUG %s: no free hso_mutexs devices in table\n"
+              , __func__);
+       spin_unlock(&hso_mutex_lock);
+       return NULL;
+}
+
+static void hso_free_mutex(struct hso_mutex_t *mutex)
+{
+       spin_lock(&hso_mutex_lock);
+       mutex->allocated = 0;
+       spin_unlock(&hso_mutex_lock);
+}
+
 /* log a meaningful explanation of an USB status */
 static void log_usb_status(int status, const char *function)
 {
@@ -1225,7 +1265,9 @@ void hso_unthrottle_workfunc(struct work_struct *work)
 static int hso_serial_open(struct tty_struct *tty, struct file *filp)
 {
        struct hso_serial *serial = get_serial_by_index(tty->index);
-       int result;
+       int result1 = 0, result2 = 0;
+       struct mutex *hso_mutex = NULL;
+       int   refcnt = 1;
 
        /* sanity check */
        if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) {
@@ -1234,14 +1276,15 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
                return -ENODEV;
        }
 
-       mutex_lock(&serial->parent->mutex);
+       hso_mutex = &serial->parent->mutex->mutex;
+       mutex_lock(hso_mutex);
        /* check for port already opened, if not set the termios */
        /* The serial->open count needs to be here as hso_serial_close
         *  will be called even if hso_serial_open returns -ENODEV.
         */
        serial->open_count++;
-       result = usb_autopm_get_interface(serial->parent->interface);
-       if (result < 0)
+       result1 = usb_autopm_get_interface(serial->parent->interface);
+       if (result1 < 0)
                goto err_out;
 
        D1("Opening %d", serial->minor);
@@ -1261,11 +1304,10 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
                             (unsigned long)serial);
                INIT_WORK(&serial->retry_unthrottle_workqueue,
                          hso_unthrottle_workfunc);
-               result = hso_start_serial_device(serial->parent, GFP_KERNEL);
-               if (result) {
+               result2 = hso_start_serial_device(serial->parent, GFP_KERNEL);
+               if (result2) {
                        hso_stop_serial_device(serial->parent);
                        serial->open_count--;
-                       kref_put(&serial->parent->ref, hso_serial_ref_free);
                }
        } else {
                D1("Port was already open");
@@ -1274,11 +1316,16 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp)
        usb_autopm_put_interface(serial->parent->interface);
 
        /* done */
-       if (result)
+       if (result1)
                hso_serial_tiocmset(tty, NULL, TIOCM_RTS | TIOCM_DTR, 0);
 err_out:
-       mutex_unlock(&serial->parent->mutex);
-       return result;
+       if (result2)
+               refcnt = kref_put(&serial->parent->ref, hso_serial_ref_free);
+       mutex_unlock(hso_mutex);
+       if (refcnt == 0)
+               hso_free_mutex(container_of(hso_mutex,
+                                           struct hso_mutex_t, mutex));
+       return result1 == 0 ? result2 : result1;
 }
 
 /* close the requested serial port */
@@ -1286,6 +1333,8 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
 {
        struct hso_serial *serial = tty->driver_data;
        u8 usb_gone;
+       struct mutex *hso_mutex;
+       int refcnt;
 
        D1("Closing serial port");
        if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) {
@@ -1293,8 +1342,9 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
                return;
        }
 
-       mutex_lock(&serial->parent->mutex);
        usb_gone = serial->parent->usb_gone;
+       hso_mutex = &serial->parent->mutex->mutex;
+       mutex_lock(hso_mutex);
 
        if (!usb_gone)
                usb_autopm_get_interface(serial->parent->interface);
@@ -1302,7 +1352,6 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
        /* reset the rts and dtr */
        /* do the actual close */
        serial->open_count--;
-       kref_put(&serial->parent->ref, hso_serial_ref_free);
        if (serial->open_count <= 0) {
                serial->open_count = 0;
                if (serial->tty) {
@@ -1317,8 +1366,11 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp)
 
        if (!usb_gone)
                usb_autopm_put_interface(serial->parent->interface);
-
-       mutex_unlock(&serial->parent->mutex);
+       refcnt = kref_put(&serial->parent->ref, hso_serial_ref_free);
+       mutex_unlock(hso_mutex);
+       if (refcnt == 0)
+               hso_free_mutex(container_of(hso_mutex,
+                                           struct hso_mutex_t, mutex));
 }
 
 /* close the requested serial port */
@@ -2084,8 +2136,12 @@ static struct hso_device *hso_create_device(struct usb_interface *intf,
        hso_dev->usb = interface_to_usbdev(intf);
        hso_dev->interface = intf;
        kref_init(&hso_dev->ref);
-       mutex_init(&hso_dev->mutex);
-
+       hso_dev->mutex = hso_get_free_mutex();
+       if (!hso_dev->mutex) {
+               kfree(hso_dev);
+               return NULL;
+       }
+       mutex_init(&hso_dev->mutex->mutex);
        INIT_WORK(&hso_dev->async_get_intf, async_get_intf);
        INIT_WORK(&hso_dev->async_put_intf, async_put_intf);
 
@@ -2131,7 +2187,7 @@ static void hso_free_net_device(struct hso_device *hso_dev)
                unregister_netdev(hso_net->net);
                free_netdev(hso_net->net);
        }
-
+       hso_free_mutex(hso_dev->mutex);
        hso_free_device(hso_dev);
 }
 
@@ -2180,14 +2236,14 @@ static int hso_radio_toggle(void *data, enum rfkill_state state)
        int enabled = (state == RFKILL_STATE_ON);
        int rv;
 
-       mutex_lock(&hso_dev->mutex);
+       mutex_lock(&hso_dev->mutex->mutex);
        if (hso_dev->usb_gone)
                rv = 0;
        else
                rv = usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0),
                                       enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0,
                                       USB_CTRL_SET_TIMEOUT);
-       mutex_unlock(&hso_dev->mutex);
+       mutex_unlock(&hso_dev->mutex->mutex);
        return rv;
 }
 
@@ -2795,6 +2851,8 @@ static void hso_free_interface(struct usb_interface *interface)
 {
        struct hso_serial *hso_dev;
        int i;
+       struct mutex *hso_mutex = NULL;
+       int    refcnt = 1;
 
        for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
                if (serial_table[i]
@@ -2802,10 +2860,12 @@ static void hso_free_interface(struct usb_interface *interface)
                        hso_dev = dev2ser(serial_table[i]);
                        if (hso_dev->tty)
                                tty_hangup(hso_dev->tty);
-                       mutex_lock(&hso_dev->parent->mutex);
+                       hso_mutex = &hso_dev->parent->mutex->mutex;
+                       mutex_lock(hso_mutex);
                        hso_dev->parent->usb_gone = 1;
-                       mutex_unlock(&hso_dev->parent->mutex);
-                       kref_put(&serial_table[i]->ref, hso_serial_ref_free);
+                       refcnt = kref_put(&serial_table[i]->ref,
+                                       hso_serial_ref_free);
+                       mutex_unlock(hso_mutex);
                }
        }
 
@@ -2824,6 +2884,9 @@ static void hso_free_interface(struct usb_interface *interface)
                        hso_free_net_device(network_table[i]);
                }
        }
+       if (refcnt == 0)
+               hso_free_mutex(container_of(hso_mutex,
+                                           struct hso_mutex_t, mutex));
 }
 
 /* Helper functions */
@@ -2922,6 +2985,7 @@ static int __init hso_init(void)
 
        /* Initialise the serial table semaphore and table */
        spin_lock_init(&serial_table_lock);
+       spin_lock_init(&hso_mutex_lock);
        for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++)
                serial_table[i] = NULL;