diff options
author | Denis Joseph Barrow <D.Barow@option.com> | 2008-11-25 03:33:13 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2008-11-25 03:33:13 -0500 |
commit | 52429eb216385fdc6969c0112ba8b46cffefaaef (patch) | |
tree | 49ac34f8d779a706a58414c5f64e317bc1b323c2 /drivers/net/usb/hso.c | |
parent | 89930b7b5e3e9bfe9c6ec5e19920451c8f5d9088 (diff) |
hso: Fix free of mutexes still in use.
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>
Diffstat (limited to 'drivers/net/usb/hso.c')
-rw-r--r-- | drivers/net/usb/hso.c | 110 |
1 files changed, 87 insertions, 23 deletions
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c index 2c172435495f..cc37a2e67177 100644 --- a/drivers/net/usb/hso.c +++ b/drivers/net/usb/hso.c | |||
@@ -230,6 +230,11 @@ struct hso_serial { | |||
230 | struct work_struct retry_unthrottle_workqueue; | 230 | struct work_struct retry_unthrottle_workqueue; |
231 | }; | 231 | }; |
232 | 232 | ||
233 | struct hso_mutex_t { | ||
234 | struct mutex mutex; | ||
235 | u8 allocated; | ||
236 | }; | ||
237 | |||
233 | struct hso_device { | 238 | struct hso_device { |
234 | union { | 239 | union { |
235 | struct hso_serial *dev_serial; | 240 | struct hso_serial *dev_serial; |
@@ -248,7 +253,7 @@ struct hso_device { | |||
248 | 253 | ||
249 | struct device *dev; | 254 | struct device *dev; |
250 | struct kref ref; | 255 | struct kref ref; |
251 | struct mutex mutex; | 256 | struct hso_mutex_t *mutex; |
252 | }; | 257 | }; |
253 | 258 | ||
254 | /* Type of interface */ | 259 | /* Type of interface */ |
@@ -364,6 +369,13 @@ static struct hso_device *network_table[HSO_MAX_NET_DEVICES]; | |||
364 | static spinlock_t serial_table_lock; | 369 | static spinlock_t serial_table_lock; |
365 | static struct ktermios *hso_serial_termios[HSO_SERIAL_TTY_MINORS]; | 370 | static struct ktermios *hso_serial_termios[HSO_SERIAL_TTY_MINORS]; |
366 | static struct ktermios *hso_serial_termios_locked[HSO_SERIAL_TTY_MINORS]; | 371 | static struct ktermios *hso_serial_termios_locked[HSO_SERIAL_TTY_MINORS]; |
372 | /* hso_mutex_table has to be declared statically as hso_device | ||
373 | * is freed in hso_serial_open & hso_serial_close while | ||
374 | * the mutex is still in use. | ||
375 | */ | ||
376 | #define HSO_NUM_MUTEXES (HSO_SERIAL_TTY_MINORS+HSO_MAX_NET_DEVICES) | ||
377 | static struct hso_mutex_t hso_mutex_table[HSO_NUM_MUTEXES]; | ||
378 | static spinlock_t hso_mutex_lock; | ||
367 | 379 | ||
368 | static const s32 default_port_spec[] = { | 380 | static const s32 default_port_spec[] = { |
369 | HSO_INTF_MUX | HSO_PORT_NETWORK, | 381 | HSO_INTF_MUX | HSO_PORT_NETWORK, |
@@ -604,6 +616,34 @@ static void set_serial_by_index(unsigned index, struct hso_serial *serial) | |||
604 | spin_unlock_irqrestore(&serial_table_lock, flags); | 616 | spin_unlock_irqrestore(&serial_table_lock, flags); |
605 | } | 617 | } |
606 | 618 | ||
619 | |||
620 | static struct hso_mutex_t *hso_get_free_mutex(void) | ||
621 | { | ||
622 | int index; | ||
623 | struct hso_mutex_t *curr_hso_mutex; | ||
624 | |||
625 | spin_lock(&hso_mutex_lock); | ||
626 | for (index = 0; index < HSO_NUM_MUTEXES; index++) { | ||
627 | curr_hso_mutex = &hso_mutex_table[index]; | ||
628 | if (!curr_hso_mutex->allocated) { | ||
629 | curr_hso_mutex->allocated = 1; | ||
630 | spin_unlock(&hso_mutex_lock); | ||
631 | return curr_hso_mutex; | ||
632 | } | ||
633 | } | ||
634 | printk(KERN_ERR "BUG %s: no free hso_mutexs devices in table\n" | ||
635 | , __func__); | ||
636 | spin_unlock(&hso_mutex_lock); | ||
637 | return NULL; | ||
638 | } | ||
639 | |||
640 | static void hso_free_mutex(struct hso_mutex_t *mutex) | ||
641 | { | ||
642 | spin_lock(&hso_mutex_lock); | ||
643 | mutex->allocated = 0; | ||
644 | spin_unlock(&hso_mutex_lock); | ||
645 | } | ||
646 | |||
607 | /* log a meaningful explanation of an USB status */ | 647 | /* log a meaningful explanation of an USB status */ |
608 | static void log_usb_status(int status, const char *function) | 648 | static void log_usb_status(int status, const char *function) |
609 | { | 649 | { |
@@ -1225,7 +1265,9 @@ void hso_unthrottle_workfunc(struct work_struct *work) | |||
1225 | static int hso_serial_open(struct tty_struct *tty, struct file *filp) | 1265 | static int hso_serial_open(struct tty_struct *tty, struct file *filp) |
1226 | { | 1266 | { |
1227 | struct hso_serial *serial = get_serial_by_index(tty->index); | 1267 | struct hso_serial *serial = get_serial_by_index(tty->index); |
1228 | int result; | 1268 | int result1 = 0, result2 = 0; |
1269 | struct mutex *hso_mutex = NULL; | ||
1270 | int refcnt = 1; | ||
1229 | 1271 | ||
1230 | /* sanity check */ | 1272 | /* sanity check */ |
1231 | if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) { | 1273 | if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) { |
@@ -1234,14 +1276,15 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp) | |||
1234 | return -ENODEV; | 1276 | return -ENODEV; |
1235 | } | 1277 | } |
1236 | 1278 | ||
1237 | mutex_lock(&serial->parent->mutex); | 1279 | hso_mutex = &serial->parent->mutex->mutex; |
1280 | mutex_lock(hso_mutex); | ||
1238 | /* check for port already opened, if not set the termios */ | 1281 | /* check for port already opened, if not set the termios */ |
1239 | /* The serial->open count needs to be here as hso_serial_close | 1282 | /* The serial->open count needs to be here as hso_serial_close |
1240 | * will be called even if hso_serial_open returns -ENODEV. | 1283 | * will be called even if hso_serial_open returns -ENODEV. |
1241 | */ | 1284 | */ |
1242 | serial->open_count++; | 1285 | serial->open_count++; |
1243 | result = usb_autopm_get_interface(serial->parent->interface); | 1286 | result1 = usb_autopm_get_interface(serial->parent->interface); |
1244 | if (result < 0) | 1287 | if (result1 < 0) |
1245 | goto err_out; | 1288 | goto err_out; |
1246 | 1289 | ||
1247 | D1("Opening %d", serial->minor); | 1290 | D1("Opening %d", serial->minor); |
@@ -1261,11 +1304,10 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp) | |||
1261 | (unsigned long)serial); | 1304 | (unsigned long)serial); |
1262 | INIT_WORK(&serial->retry_unthrottle_workqueue, | 1305 | INIT_WORK(&serial->retry_unthrottle_workqueue, |
1263 | hso_unthrottle_workfunc); | 1306 | hso_unthrottle_workfunc); |
1264 | result = hso_start_serial_device(serial->parent, GFP_KERNEL); | 1307 | result2 = hso_start_serial_device(serial->parent, GFP_KERNEL); |
1265 | if (result) { | 1308 | if (result2) { |
1266 | hso_stop_serial_device(serial->parent); | 1309 | hso_stop_serial_device(serial->parent); |
1267 | serial->open_count--; | 1310 | serial->open_count--; |
1268 | kref_put(&serial->parent->ref, hso_serial_ref_free); | ||
1269 | } | 1311 | } |
1270 | } else { | 1312 | } else { |
1271 | D1("Port was already open"); | 1313 | D1("Port was already open"); |
@@ -1274,11 +1316,16 @@ static int hso_serial_open(struct tty_struct *tty, struct file *filp) | |||
1274 | usb_autopm_put_interface(serial->parent->interface); | 1316 | usb_autopm_put_interface(serial->parent->interface); |
1275 | 1317 | ||
1276 | /* done */ | 1318 | /* done */ |
1277 | if (result) | 1319 | if (result1) |
1278 | hso_serial_tiocmset(tty, NULL, TIOCM_RTS | TIOCM_DTR, 0); | 1320 | hso_serial_tiocmset(tty, NULL, TIOCM_RTS | TIOCM_DTR, 0); |
1279 | err_out: | 1321 | err_out: |
1280 | mutex_unlock(&serial->parent->mutex); | 1322 | if (result2) |
1281 | return result; | 1323 | refcnt = kref_put(&serial->parent->ref, hso_serial_ref_free); |
1324 | mutex_unlock(hso_mutex); | ||
1325 | if (refcnt == 0) | ||
1326 | hso_free_mutex(container_of(hso_mutex, | ||
1327 | struct hso_mutex_t, mutex)); | ||
1328 | return result1 == 0 ? result2 : result1; | ||
1282 | } | 1329 | } |
1283 | 1330 | ||
1284 | /* close the requested serial port */ | 1331 | /* close the requested serial port */ |
@@ -1286,6 +1333,8 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp) | |||
1286 | { | 1333 | { |
1287 | struct hso_serial *serial = tty->driver_data; | 1334 | struct hso_serial *serial = tty->driver_data; |
1288 | u8 usb_gone; | 1335 | u8 usb_gone; |
1336 | struct mutex *hso_mutex; | ||
1337 | int refcnt; | ||
1289 | 1338 | ||
1290 | D1("Closing serial port"); | 1339 | D1("Closing serial port"); |
1291 | if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) { | 1340 | if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) { |
@@ -1293,8 +1342,9 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp) | |||
1293 | return; | 1342 | return; |
1294 | } | 1343 | } |
1295 | 1344 | ||
1296 | mutex_lock(&serial->parent->mutex); | ||
1297 | usb_gone = serial->parent->usb_gone; | 1345 | usb_gone = serial->parent->usb_gone; |
1346 | hso_mutex = &serial->parent->mutex->mutex; | ||
1347 | mutex_lock(hso_mutex); | ||
1298 | 1348 | ||
1299 | if (!usb_gone) | 1349 | if (!usb_gone) |
1300 | usb_autopm_get_interface(serial->parent->interface); | 1350 | usb_autopm_get_interface(serial->parent->interface); |
@@ -1302,7 +1352,6 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp) | |||
1302 | /* reset the rts and dtr */ | 1352 | /* reset the rts and dtr */ |
1303 | /* do the actual close */ | 1353 | /* do the actual close */ |
1304 | serial->open_count--; | 1354 | serial->open_count--; |
1305 | kref_put(&serial->parent->ref, hso_serial_ref_free); | ||
1306 | if (serial->open_count <= 0) { | 1355 | if (serial->open_count <= 0) { |
1307 | serial->open_count = 0; | 1356 | serial->open_count = 0; |
1308 | if (serial->tty) { | 1357 | if (serial->tty) { |
@@ -1317,8 +1366,11 @@ static void hso_serial_close(struct tty_struct *tty, struct file *filp) | |||
1317 | 1366 | ||
1318 | if (!usb_gone) | 1367 | if (!usb_gone) |
1319 | usb_autopm_put_interface(serial->parent->interface); | 1368 | usb_autopm_put_interface(serial->parent->interface); |
1320 | 1369 | refcnt = kref_put(&serial->parent->ref, hso_serial_ref_free); | |
1321 | mutex_unlock(&serial->parent->mutex); | 1370 | mutex_unlock(hso_mutex); |
1371 | if (refcnt == 0) | ||
1372 | hso_free_mutex(container_of(hso_mutex, | ||
1373 | struct hso_mutex_t, mutex)); | ||
1322 | } | 1374 | } |
1323 | 1375 | ||
1324 | /* close the requested serial port */ | 1376 | /* close the requested serial port */ |
@@ -2084,8 +2136,12 @@ static struct hso_device *hso_create_device(struct usb_interface *intf, | |||
2084 | hso_dev->usb = interface_to_usbdev(intf); | 2136 | hso_dev->usb = interface_to_usbdev(intf); |
2085 | hso_dev->interface = intf; | 2137 | hso_dev->interface = intf; |
2086 | kref_init(&hso_dev->ref); | 2138 | kref_init(&hso_dev->ref); |
2087 | mutex_init(&hso_dev->mutex); | 2139 | hso_dev->mutex = hso_get_free_mutex(); |
2088 | 2140 | if (!hso_dev->mutex) { | |
2141 | kfree(hso_dev); | ||
2142 | return NULL; | ||
2143 | } | ||
2144 | mutex_init(&hso_dev->mutex->mutex); | ||
2089 | INIT_WORK(&hso_dev->async_get_intf, async_get_intf); | 2145 | INIT_WORK(&hso_dev->async_get_intf, async_get_intf); |
2090 | INIT_WORK(&hso_dev->async_put_intf, async_put_intf); | 2146 | INIT_WORK(&hso_dev->async_put_intf, async_put_intf); |
2091 | 2147 | ||
@@ -2131,7 +2187,7 @@ static void hso_free_net_device(struct hso_device *hso_dev) | |||
2131 | unregister_netdev(hso_net->net); | 2187 | unregister_netdev(hso_net->net); |
2132 | free_netdev(hso_net->net); | 2188 | free_netdev(hso_net->net); |
2133 | } | 2189 | } |
2134 | 2190 | hso_free_mutex(hso_dev->mutex); | |
2135 | hso_free_device(hso_dev); | 2191 | hso_free_device(hso_dev); |
2136 | } | 2192 | } |
2137 | 2193 | ||
@@ -2180,14 +2236,14 @@ static int hso_radio_toggle(void *data, enum rfkill_state state) | |||
2180 | int enabled = (state == RFKILL_STATE_ON); | 2236 | int enabled = (state == RFKILL_STATE_ON); |
2181 | int rv; | 2237 | int rv; |
2182 | 2238 | ||
2183 | mutex_lock(&hso_dev->mutex); | 2239 | mutex_lock(&hso_dev->mutex->mutex); |
2184 | if (hso_dev->usb_gone) | 2240 | if (hso_dev->usb_gone) |
2185 | rv = 0; | 2241 | rv = 0; |
2186 | else | 2242 | else |
2187 | rv = usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0), | 2243 | rv = usb_control_msg(hso_dev->usb, usb_rcvctrlpipe(hso_dev->usb, 0), |
2188 | enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0, | 2244 | enabled ? 0x82 : 0x81, 0x40, 0, 0, NULL, 0, |
2189 | USB_CTRL_SET_TIMEOUT); | 2245 | USB_CTRL_SET_TIMEOUT); |
2190 | mutex_unlock(&hso_dev->mutex); | 2246 | mutex_unlock(&hso_dev->mutex->mutex); |
2191 | return rv; | 2247 | return rv; |
2192 | } | 2248 | } |
2193 | 2249 | ||
@@ -2795,6 +2851,8 @@ static void hso_free_interface(struct usb_interface *interface) | |||
2795 | { | 2851 | { |
2796 | struct hso_serial *hso_dev; | 2852 | struct hso_serial *hso_dev; |
2797 | int i; | 2853 | int i; |
2854 | struct mutex *hso_mutex = NULL; | ||
2855 | int refcnt = 1; | ||
2798 | 2856 | ||
2799 | for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) { | 2857 | for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) { |
2800 | if (serial_table[i] | 2858 | if (serial_table[i] |
@@ -2802,10 +2860,12 @@ static void hso_free_interface(struct usb_interface *interface) | |||
2802 | hso_dev = dev2ser(serial_table[i]); | 2860 | hso_dev = dev2ser(serial_table[i]); |
2803 | if (hso_dev->tty) | 2861 | if (hso_dev->tty) |
2804 | tty_hangup(hso_dev->tty); | 2862 | tty_hangup(hso_dev->tty); |
2805 | mutex_lock(&hso_dev->parent->mutex); | 2863 | hso_mutex = &hso_dev->parent->mutex->mutex; |
2864 | mutex_lock(hso_mutex); | ||
2806 | hso_dev->parent->usb_gone = 1; | 2865 | hso_dev->parent->usb_gone = 1; |
2807 | mutex_unlock(&hso_dev->parent->mutex); | 2866 | refcnt = kref_put(&serial_table[i]->ref, |
2808 | kref_put(&serial_table[i]->ref, hso_serial_ref_free); | 2867 | hso_serial_ref_free); |
2868 | mutex_unlock(hso_mutex); | ||
2809 | } | 2869 | } |
2810 | } | 2870 | } |
2811 | 2871 | ||
@@ -2824,6 +2884,9 @@ static void hso_free_interface(struct usb_interface *interface) | |||
2824 | hso_free_net_device(network_table[i]); | 2884 | hso_free_net_device(network_table[i]); |
2825 | } | 2885 | } |
2826 | } | 2886 | } |
2887 | if (refcnt == 0) | ||
2888 | hso_free_mutex(container_of(hso_mutex, | ||
2889 | struct hso_mutex_t, mutex)); | ||
2827 | } | 2890 | } |
2828 | 2891 | ||
2829 | /* Helper functions */ | 2892 | /* Helper functions */ |
@@ -2922,6 +2985,7 @@ static int __init hso_init(void) | |||
2922 | 2985 | ||
2923 | /* Initialise the serial table semaphore and table */ | 2986 | /* Initialise the serial table semaphore and table */ |
2924 | spin_lock_init(&serial_table_lock); | 2987 | spin_lock_init(&serial_table_lock); |
2988 | spin_lock_init(&hso_mutex_lock); | ||
2925 | for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) | 2989 | for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) |
2926 | serial_table[i] = NULL; | 2990 | serial_table[i] = NULL; |
2927 | 2991 | ||