respin of hso patches for linux-2.6.28-rc6 hso_close_fix.patch [patch 2/6 ]
Subsystems (4)
Moved serial_open_count in hso_serial_open to prevent crashes owing to the serial structure being made NULL when hso_serial_close is called even though hso_serial_open returned -ENODEV, Alan Cox pointed out this happens, also put in sanity check in hso_serial_close
to check for a valid serial structure which should prevent the most reproducable crash in the driver when the hso device is disconnected while in use.
---
drivers/net/usb/hso.c +9 -2
@@ -1235,6 +1235,11 @@ }mutex_lock (&serial->parent->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 ) goto err_out;
@@ -1246,8 +1251,6 @@ tty->driver_data = serial; serial->tty = tty; -/* check for port already opened, if not set the termios */ - serial->open_count++;if (serial->open_count ==1 ) { tty->low_latency =1 ; serial->rx_state =RX_IDLE ;
@@ -1285,6 +1288,10 @@u8 usb_gone;D1 ("Closing serial port" ); +if (serial ==NULL || serial->magic !=HSO_SERIAL_MAGIC ) { +D1 ("invalid serial structure bailing out.\n" ); +return ; + }mutex_lock (&serial->parent->mutex); usb_gone = serial->parent->usb_gone;
Signature · 2 lines
best regards, D.J. Barrow
Moved serial_open_count in hso_serial_open to prevent crashes owing to the serial structure being made NULL when hso_serial_close is called even though hso_serial_open returned -ENODEV, Alan Cox pointed out this happens, also put in sanity check in hso_serial_close
to check for a valid serial structure which should prevent the most reproducable crash in the driver when the hso device is disconnected while in use.
---
drivers/net/usb/hso.c +9 -2
@@ -1235,6 +1235,11 @@ }mutex_lock (&serial->parent->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 ) goto err_out;
@@ -1246,8 +1251,6 @@ tty->driver_data = serial; serial->tty = tty; -/* check for port already opened, if not set the termios */ - serial->open_count++;if (serial->open_count ==1 ) { tty->low_latency =1 ; serial->rx_state =RX_IDLE ;
@@ -1285,6 +1288,10 @@u8 usb_gone;D1 ("Closing serial port" ); +if (serial ==NULL || serial->magic !=HSO_SERIAL_MAGIC ) { +D1 ("invalid serial structure bailing out.\n" ); +return ; + }mutex_lock (&serial->parent->mutex); usb_gone = serial->parent->usb_gone;
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi Denis,
Quoted 38 lines
Moved
serial_open_countinhso_serial_opento prevent crashes owing to the serial structure being made NULL whenhso_serial_closeis called even thoughhso_serial_openreturned-ENODEV, Alan Cox pointed out this happens, also put in sanity check inhso_serial_close
to check for a valid serial structure which should prevent the most reproducable crash in the driver when the hso device is disconnected while in use.Signed-off-by Denis Joseph Barrow <[email protected]>---
Index:linux-2.6.28-rc6.patches/drivers/net/usb/hso.c
===================================================================
---linux-2.6.28-rc6.patches.orig/drivers/net/usb/hso.c2008-11-24
14:01:45.000000000 +0100
+++linux-2.6.28-rc6.patches/drivers/net/usb/hso.c2008-11-24
14:10:15.000000000 +0100drivers/net/usb/hso.c +5
-2@@ -1235,6 +1235,11 @@ }mutex_lock (&serial->parent->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 ) goto err_out;@@ -1246,8 +1251,6 @@ tty->driver_data = serial; serial->tty = tty; -/* check for port already opened, if not set the termios */ - serial->open_count++;if (serial->open_count ==1 ) { tty->low_latency =1 ; serial->rx_state =RX_IDLE ;
drivers/net/usb/hso.c line 1294
Quoted 7 lines
drivers/net/usb/hso.c +4
-0@@ -1285,6 +1288,10 @@u8 usb_gone;D1 ("Closing serial port" ); +if (serial ==NULL || serial->magic !=HSO_SERIAL_MAGIC ) { +D1 ("invalid serial structure bailing out.\n" ); +return ; + }
using !serial instead of serial == NULL would be better.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 7 lines
drivers/net/usb/hso.c line 1294
Quoted 5 lines
D1("Closing serial port");drivers/net/usb/hso.c +4
-0+if (serial ==NULL || serial->magic !=HSO_SERIAL_MAGIC ) { +D1 ("invalid serial structure bailing out.\n" ); +return ; + }using !serial instead of serial == NULL would be better.
A lot of serial code uses == NULL and its just as clear IMHO.
A bigger concern would be the lack of refcounting. Really for 2.6.28 now the refcount patches are in the direction should be refcounting the tty objects at least.
I've got a pair of proposed patches I'll post for comment
Alan
From: Denis Joseph Barrow <[email protected]>
Date: Mon, 24 Nov 2008 15:42:34 +0100
Quoted 9 lines
Moved
serial_open_countinhso_serial_opento prevent crashes owing to the serial structure being made NULL whenhso_serial_closeis called even thoughhso_serial_openreturned-ENODEV, Alan Cox pointed out this happens, also put in sanity check inhso_serial_close
to check for a valid serial structure which should prevent the most reproducable crash in the driver when the hso device is disconnected while in use.Signed-off-by Denis Joseph Barrow <[email protected]>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html