USB: serial: xr: fix NULL-deref on disconnect
Subsystems (2)
Johan Hovold
maintainerPATCH 1/1 USB: serial: xr: fix NULL-deref on disconnect
Claiming the sibling control interface is a bit more involved and specifically requires adding support to USB-serial core for managing either interface being unbound first, something which could otherwise lead to a NULL-pointer dereference.
Similarly, additional infrastructure is also needed to handle suspend properly.
Since the driver currently isn't actually using the control interface, we can defer this for now by simply not claiming the control interface.
--- drivers/usb/serial/xr_serial.c | 25 ------------------------- 1 file changed, 25 deletions(-)
drivers/usb/serial/xr_serial.c +0 -25
@@ -545,37 +545,13 @@ static void xr_close(struct usb_serial_port *port)static int xr_probe (struct usb_serial *serial,const struct usb_device_id *id) { -struct usb_driver *driver = serial->type->usb_driver; -struct usb_interface *control_interface; -int ret; -/* Don't bind to control interface */ if (serial->interface->cur_altsetting->desc.bInterfaceNumber ==0 )return -ENODEV ; -/* But claim the control interface during data interface probe */ - control_interface =usb_ifnum_to_if (serial->dev,0 ); -if (!control_interface) -return -ENODEV ; - - ret =usb_driver_claim_interface (driver, control_interface,NULL ); -if (ret) { -dev_err (&serial->interface->dev,"Failed to claim control interface\n" ); -return ret; - } -return 0 ; } -static void xr_disconnect (struct usb_serial *serial) -{ -struct usb_driver *driver = serial->type->usb_driver; -struct usb_interface *control_interface; - - control_interface =usb_ifnum_to_if (serial->dev,0 ); -usb_driver_release_interface (driver, control_interface); -} -static const struct usb_device_id id_table[] = { {USB_DEVICE (0x04e2 ,0x1410 ) },/* XR21V141X */ { }
@@ -590,7 +566,6 @@ static struct usb_serial_driver xr_device = { .id_table = id_table, .num_ports = 1, .probe = xr_probe, - .disconnect = xr_disconnect, .open = xr_open, .close = xr_close, .break_ctl = xr_break_ctl,
Signature · 1 line
2.26.2
Quoted 8 lines · Johan Hovold
Claiming the sibling control interface is a bit more involved and specifically requires adding support to USB-serial core for managing either interface being unbound first, something which could otherwise lead to a NULL-pointer dereference.
Similarly, additional infrastructure is also needed to handle suspend properly.
Quoted 7 lines
Since the driver currently isn't actually using the control interface, we can defer this for now by simply not claiming the control interface.
Fixes c2d405aa86b4 ("USB: serial: add MaxLinear/Exar USB to Serial driver")Reported-by Mauro Carvalho Chehab <[email protected]>Cc Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Em Fri, 26 Feb 2021 11:08:26 +0100
Johan Hovold <[email protected]> escreveu:
Quoted 8 lines
Claiming the sibling control interface is a bit more involved and specifically requires adding support to USB-serial core for managing either interface being unbound first, something which could otherwise lead to a NULL-pointer dereference.
Similarly, additional infrastructure is also needed to handle suspend properly.
Quoted 7 lines
Since the driver currently isn't actually using the control interface, we can defer this for now by simply not claiming the control interface.
Fixes c2d405aa86b4 ("USB: serial: add MaxLinear/Exar USB to Serial driver")Reported-by Mauro Carvalho Chehab <[email protected]>Cc Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>
That solved the issue with XR21V1410:
[ 8176.265862] usbcore: registered new interface driver xr_serial [ 8176.265885] usbserial: USB Serial support registered for xr_serial [ 8176.265921] xr_serial 2-1:1.1: xr_serial converter detected [ 8176.266041] usb 2-1: xr_serial converter now attached to ttyUSB0 [ 8176.268023] printk: console [ttyUSB0] enabled [ 8186.512841] usb 2-1: USB disconnect, device number 5 [ 8186.513131] printk: console [ttyUSB0] disabled [ 8186.513340] xr_serial ttyUSB0: xr_serial converter now disconnected from ttyUSB0 [ 8186.513376] xr_serial 2-1:1.1: device disconnected
Regards,
Mauro
Quoted 47 lines
--- drivers/usb/serial/xr_serial.c | 25 ------------------------- 1 file changed, 25 deletions(-)drivers/usb/serial/xr_serial.c +0
-24@@ -545,37 +545,13 @@ static void xr_close(struct usb_serial_port *port)static int xr_probe (struct usb_serial *serial,const struct usb_device_id *id) { -struct usb_driver *driver = serial->type->usb_driver; -struct usb_interface *control_interface; -int ret; -/* Don't bind to control interface */ if (serial->interface->cur_altsetting->desc.bInterfaceNumber ==0 )return -ENODEV ; -/* But claim the control interface during data interface probe */ - control_interface =usb_ifnum_to_if (serial->dev,0 ); -if (!control_interface) -return -ENODEV ; - - ret =usb_driver_claim_interface (driver, control_interface,NULL ); -if (ret) { -dev_err (&serial->interface->dev,"Failed to claim control interface\n" ); -return ret; - } -return 0 ; } -static void xr_disconnect (struct usb_serial *serial) -{ -struct usb_driver *driver = serial->type->usb_driver; -struct usb_interface *control_interface; - - control_interface =usb_ifnum_to_if (serial->dev,0 ); -usb_driver_release_interface (driver, control_interface); -} -static const struct usb_device_id id_table[] = { {USB_DEVICE (0x04e2 ,0x1410 ) },/* XR21V141X */ { }
drivers/usb/serial/xr_serial.c static struct usb_serial_driver xr_device = { · line 571
Quoted 7 lines
drivers/usb/serial/xr_serial.c +0
-1@@ -590,7 +566,6 @@ static struct usb_serial_driver xr_device = { .id_table = id_table, .num_ports = 1, .probe = xr_probe, - .disconnect = xr_disconnect, .open = xr_open, .close = xr_close, .break_ctl = xr_break_ctl,
Thanks,
Mauro
Johan Hovold
maintainerQuoted 31 lines · Mauro Carvalho Chehab
Em Fri, 26 Feb 2021 11:08:26 +0100
Johan Hovold <[email protected]> escreveu:Quoted 8 lines
Claiming the sibling control interface is a bit more involved and specifically requires adding support to USB-serial core for managing either interface being unbound first, something which could otherwise lead to a NULL-pointer dereference.
Similarly, additional infrastructure is also needed to handle suspend properly.
Quoted 7 lines
Since the driver currently isn't actually using the control interface, we can defer this for now by simply not claiming the control interface.
Fixes c2d405aa86b4 ("USB: serial: add MaxLinear/Exar USB to Serial driver")Reported-by Mauro Carvalho Chehab <[email protected]>Cc Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>That solved the issue with XR21V1410:
[ 8176.265862] usbcore: registered new interface driver xr_serial [ 8176.265885] usbserial: USB Serial support registered for xr_serial [ 8176.265921] xr_serial 2-1:1.1: xr_serial converter detected [ 8176.266041] usb 2-1: xr_serial converter now attached to ttyUSB0 [ 8176.268023] printk: console [ttyUSB0] enabled [ 8186.512841] usb 2-1: USB disconnect, device number 5 [ 8186.513131] printk: console [ttyUSB0] disabled [ 8186.513340] xr_serial ttyUSB0: xr_serial converter now disconnected from ttyUSB0 [ 8186.513376] xr_serial 2-1:1.1: device disconnected
Quoted 1 line
Thanks for testing, and thanks to Greg and Manivannan for the review.
Now applied.
Johan
Quoted 8 lines · Johan Hovold
Claiming the sibling control interface is a bit more involved and specifically requires adding support to USB-serial core for managing either interface being unbound first, something which could otherwise lead to a NULL-pointer dereference.
Similarly, additional infrastructure is also needed to handle suspend properly.
Quoted 7 lines
Since the driver currently isn't actually using the control interface, we can defer this for now by simply not claiming the control interface.
Fixes c2d405aa86b4 ("USB: serial: add MaxLinear/Exar USB to Serial driver")Reported-by Mauro Carvalho Chehab <[email protected]>Cc Manivannan Sadhasivam <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Thanks,
Mani
Quoted 57 lines
--- drivers/usb/serial/xr_serial.c | 25 ------------------------- 1 file changed, 25 deletions(-)drivers/usb/serial/xr_serial.c +0
-25@@ -545,37 +545,13 @@ static void xr_close(struct usb_serial_port *port)static int xr_probe (struct usb_serial *serial,const struct usb_device_id *id) { -struct usb_driver *driver = serial->type->usb_driver; -struct usb_interface *control_interface; -int ret; -/* Don't bind to control interface */ if (serial->interface->cur_altsetting->desc.bInterfaceNumber ==0 )return -ENODEV ; -/* But claim the control interface during data interface probe */ - control_interface =usb_ifnum_to_if (serial->dev,0 ); -if (!control_interface) -return -ENODEV ; - - ret =usb_driver_claim_interface (driver, control_interface,NULL ); -if (ret) { -dev_err (&serial->interface->dev,"Failed to claim control interface\n" ); -return ret; - } -return 0 ; } -static void xr_disconnect (struct usb_serial *serial) -{ -struct usb_driver *driver = serial->type->usb_driver; -struct usb_interface *control_interface; - - control_interface =usb_ifnum_to_if (serial->dev,0 ); -usb_driver_release_interface (driver, control_interface); -} -static const struct usb_device_id id_table[] = { {USB_DEVICE (0x04e2 ,0x1410 ) },/* XR21V141X */ { }@@ -590,7 +566,6 @@ static struct usb_serial_driver xr_device = { .id_table = id_table, .num_ports = 1, .probe = xr_probe, - .disconnect = xr_disconnect, .open = xr_open, .close = xr_close, .break_ctl = xr_break_ctl,Signature · 1 line
2.26.2