usbip: vhci_hcd: fix NULL deref and NR_HCS upper bound
Subsystems (2)
cover usbip: vhci_hcd: fix NULL deref and NR_HCS upper bound
These two patches fix a NULL pointer dereference in vhci_sysfs.c triggered when a VHCI host controller fails to probe, and tighten the Kconfig upper bound for USBIP_VHCI_NR_HCS to reflect the real maximum imposed by USB_MAXBUS.
Tested on Ubuntu 6.8.0-110-generic (6.8.12) on a Dell PowerEdge R640, issue is reproducible with CONFIG_USBIP_VHCI_NR_HCS > 32 or potentially lower if there is already other USB hardware connected.
Adrian Wowk (2):
usbip: vhci_hcd: fix NULL deref in status_show_vhci
usbip: vhci_hcd: reduce CONFIG_USBIP_VHCI_NR_HCS upper bound to 32
drivers/usb/usbip/Kconfig | 2 +- drivers/usb/usbip/vhci_sysfs.c | 52 +++++++++++++++++++--------------- 2 files changed, 30 insertions(+), 24 deletions(-)
Signature · 1 line
2.53.0
PATCH 1/2 usbip: vhci_hcd: fix NULL deref in status_show_vhci
platform_get_drvdata() can return NULL if a VHCI host controller's probe failed (e.g. due to USB bus number exhaustion). status_show_vhci() checked for a NULL pdev but not for a NULL hcd returned by platform_get_drvdata(). Passing NULL to hcd_to_vhci_hcd() does not return NULL - it returns a pointer offset of 0x260, causing a NULL pointer dereference when that value is subsequently dereferenced.
Add a NULL check on hcd before calling hcd_to_vhci_hcd(). Move status_show_not_ready() above status_show_vhci() to make it callable from the new error path without a forward declaration.
--- drivers/usb/usbip/vhci_sysfs.c | 52 +++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 23 deletions(-)
drivers/usb/usbip/vhci_sysfs.c +29 -23
@@ -59,6 +59,29 @@ static void port_show_vhci(char **out, int hub, int port, struct vhci_device *vd *out +=sprintf (*out,"\n" ); } +static ssize_t status_show_not_ready (int pdev_nr,char *out) +{ +char *s = out; +int i =0 ; + +for (i =0 ; i <VHCI_HC_PORTS ; i++) { + out +=sprintf (out,"hs %04u %03u " , + (pdev_nr *VHCI_PORTS ) + i, +VDEV_ST_NOTASSIGNED ); + out +=sprintf (out,"000 00000000 0000000000000000 0-0" ); + out +=sprintf (out,"\n" ); + } + +for (i =0 ; i <VHCI_HC_PORTS ; i++) { + out +=sprintf (out,"ss %04u %03u " , + (pdev_nr *VHCI_PORTS ) +VHCI_HC_PORTS + i, +VDEV_ST_NOTASSIGNED ); + out +=sprintf (out,"000 00000000 0000000000000000 0-0" ); + out +=sprintf (out,"\n" ); + } +return out - s; +} +/* Sysfs entry to show port status */ static ssize_t status_show_vhci (int pdev_nr,char *out) {
@@ -76,6 +99,12 @@ static ssize_t status_show_vhci(int pdev_nr, char *out) } hcd =platform_get_drvdata (pdev); + +if (!hcd) { +usbip_dbg_vhci_sysfs ("show status error (hcd is NULL)\n" ); +return status_show_not_ready (pdev_nr, out); + } + vhci_hcd =hcd_to_vhci_hcd (hcd); vhci = vhci_hcd->vhci;
@@ -104,29 +133,6 @@ static ssize_t status_show_vhci(int pdev_nr, char *out)return out - s; } -static ssize_t status_show_not_ready (int pdev_nr,char *out) -{ -char *s = out; -int i =0 ; - -for (i =0 ; i <VHCI_HC_PORTS ; i++) { - out +=sprintf (out,"hs %04u %03u " , - (pdev_nr *VHCI_PORTS ) + i, -VDEV_ST_NOTASSIGNED ); - out +=sprintf (out,"000 00000000 0000000000000000 0-0" ); - out +=sprintf (out,"\n" ); - } - -for (i =0 ; i <VHCI_HC_PORTS ; i++) { - out +=sprintf (out,"ss %04u %03u " , - (pdev_nr *VHCI_PORTS ) +VHCI_HC_PORTS + i, -VDEV_ST_NOTASSIGNED ); - out +=sprintf (out,"000 00000000 0000000000000000 0-0" ); - out +=sprintf (out,"\n" ); - } -return out - s; -} -static int status_name_to_id (const char *name) {char *c;
Signature · 1 line
2.53.0
Re: [PATCH 1/2] usbip: vhci_hcd: fix NULL deref in status_show_vhci
Quoted 7 lines · Adrian Wowk
platform_get_drvdata()can return NULL if a VHCI host controller's probe failed (e.g. due to USB bus number exhaustion).status_show_vhci()checked for a NULL pdev but not for a NULL hcd returned byplatform_get_drvdata(). Passing NULL tohcd_to_vhci_hcd()does not return NULL - it returns a pointer offset of0x260, causing a NULL pointer dereference when that value is subsequently dereferenced.
Quoted 9 lines
Add a NULL check on hcd before calling
hcd_to_vhci_hcd(). Movestatus_show_not_ready()abovestatus_show_vhci()to make it callable from the new error path without a forward declaration.Signed-off-by Adrian Wowk <[email protected]>--- drivers/usb/usbip/vhci_sysfs.c | 52 +++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 23 deletions(-)
Looks good to me.
Greg, Please pick this up.
thanks,
-- Shuah
PATCH 2/2 usbip: vhci_hcd: reduce CONFIG_USBIP_VHCI_NR_HCS upper bound to 32
In reply to Adrian WowkEach VHCI HC instance registers two USB buses (one HS, one SS). USB_MAXBUS in drivers/usb/core/hcd.c is hard-coded to 64, giving an effective maximum of 32 VHCI HC instances (32 * 2 = 64 buses).
The Kconfig range for USBIP_VHCI_NR_HCS currently allows up to 128, which will cause probe failures for any HC instance beyond the 32nd. These probe failures trigger the NULL pointer dereference fixed in the previous commit.
Reduce the upper bound to 32 to reflect the real maximum imposed by USB_MAXBUS. Note that probe failures can still occur below this limit if real hardware has already claimed enough USB bus numbers, making the NULL check fix necessary regardless.
--- drivers/usb/usbip/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/usbip/Kconfig +1 -1
@@ -40,7 +40,7 @@ config USBIP_VHCI_HC_PORTS config USBIP_VHCI_NR_HCS int "Number of USB/IP virtual host controllers" - range 1 128 + range 1 32 default 1 depends on USBIP_VHCI_HCD help
Signature · 1 line
2.53.0
Re: [PATCH 2/2] usbip: vhci_hcd: reduce CONFIG_USBIP_VHCI_NR_HCS upper bound to 32
Quoted 24 lines · Adrian Wowk
Each VHCI HC instance registers two USB buses (one HS, one SS).
USB_MAXBUSindrivers/usb/core/hcd.cis hard-coded to 64, giving an effective maximum of 32 VHCI HC instances (32 * 2 = 64 buses).The
Kconfigrange forUSBIP_VHCI_NR_HCScurrently allows up to 128, which will cause probe failures for any HC instance beyond the 32nd. These probe failures trigger the NULL pointer dereference fixed in the previous commit.Reduce the upper bound to 32 to reflect the real maximum imposed by
USB_MAXBUS. Note that probe failures can still occur below this limit if real hardware has already claimed enough USB bus numbers, making the NULL check fix necessary regardless.Signed-off-by Adrian Wowk <[email protected]>--- drivers/usb/usbip/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/usb/usbip/Kconfig +0
-0
drivers/usb/usbip/Kconfig config USBIP_VHCI_HC_PORTS · line 46
Quoted 8 lines
drivers/usb/usbip/Kconfig +1
-1@@ -40,7 +40,7 @@ config USBIP_VHCI_HC_PORTS config USBIP_VHCI_NR_HCS int "Number of USB/IP virtual host controllers" - range 1 128 + range 1 32 default 1 depends on USBIP_VHCI_HCD help
Looks good to me.
Greg, Please pick this up.
thanks,
-- Shuah