From: Benny Halevy Date: Tue, 15 May 2007 08:15:27 +0000 (+0300) Subject: synchronization in usb_serial_put X-Git-Tag: v2.6.23-rc1~1083^2~81 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=52f6b5e1f15fa8c06efa69a4b5faa69c04707c92;p=linux-2.6 synchronization in usb_serial_put I think there is a race between usb_serial_put() and usb_serial_get_by_index() (and get_free_serial()) with regards to handling the serial port refcount. usb_serial_get_by_index() gets a reference on the serial port under table_lock while return_serial releases all the returned ports from the table under the same lock. However, the table_lock is not taken around the call to kref_put, theoretically allowing to sneak in and grab a reference after kref_put has already determined that the reference count is zero (and before calling destroy_serial) causing use after free. Signed-off-by: Benny Halevy Cc: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index e3e3728e16..a3665659d1 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -122,11 +122,9 @@ static void return_serial(struct usb_serial *serial) if (serial == NULL) return; - spin_lock(&table_lock); for (i = 0; i < serial->num_ports; ++i) { serial_table[serial->minor + i] = NULL; } - spin_unlock(&table_lock); } static void destroy_serial(struct kref *kref) @@ -174,7 +172,9 @@ static void destroy_serial(struct kref *kref) void usb_serial_put(struct usb_serial *serial) { + spin_lock(&table_lock); kref_put(&serial->kref, destroy_serial); + spin_unlock(&table_lock); } /*****************************************************************************