usbip: only dump valid port status
PATCH 1/3 usbip: only dump valid port status
From: Márton Németh <[email protected]>
The wIndex parameter of vhci_hub_control() is always zero when the request type is GetHubDescriptor, see drivers/usb/core/hub.c::get_hub_descriptor() and Universal Serial Bus Specification revision 2.0 (April 27, 2000) Section 11.24.2.5 "Get Hub Descriptor". This means the variable rhport will not contain any useful index. Only use valid rhport values.
--- drivers/staging/usbip/vhci_hcd.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
drivers/staging/usbip/vhci_hcd.c +5 -2
@@ -464,8 +464,11 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,if (usbip_dbg_flag_vhci_rh) {pr_debug ("port %d\n" , rhport); -dump_port_status (prev_port_status[rhport]); -dump_port_status (dum->port_status[rhport]); +/* Only dump valid port status */ +if (0 <= rhport) { +dump_port_status (prev_port_status[rhport]); +dump_port_status (dum->port_status[rhport]); + } }usbip_dbg_vhci_rh (" bye\n" );
Signature · 1 line
1.7.2.5
Quoted 10 lines · Németh Márton
From: Márton Németh <[email protected]>
The wIndex parameter of
vhci_hub_control()is always zero when the request type is GetHubDescriptor,
---drivers/staging/usbip/vhci_hcd.c +0
-0@@ -464,8 +464,11 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,if (usbip_dbg_flag_vhci_rh) {pr_debug ("port %d\n" , rhport);
drivers/staging/usbip/vhci_hcd.c static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, · line 474
Quoted 10 lines
drivers/staging/usbip/vhci_hcd.c +5
-2-dump_port_status (prev_port_status[rhport]); -dump_port_status (dum->port_status[rhport]); +/* Only dump valid port status */ +if (0 <= rhport) { +dump_port_status (prev_port_status[rhport]); +dump_port_status (dum->port_status[rhport]); + } }usbip_dbg_vhci_rh (" bye\n" );
Linux relatively rarely uses constant before variable in a comparison.
ie: please use "if (rhport > 0)" instead
From: Márton Németh <[email protected]>
The wIndex parameter of vhci_hub_control() is always zero when the request type is GetHubDescriptor, see drivers/usb/core/hub.c::get_hub_descriptor() and Universal Serial Bus Specification revision 2.0 (April 27, 2000) Section 11.24.2.5 "Get Hub Descriptor". This means the variable rhport will not contain any useful index. Only use valid rhport values.
--- drivers/staging/usbip/vhci_hcd.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
drivers/staging/usbip/vhci_hcd.c +5 -2
@@ -464,8 +464,11 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,if (usbip_dbg_flag_vhci_rh) {pr_debug ("port %d\n" , rhport); -dump_port_status (prev_port_status[rhport]); -dump_port_status (dum->port_status[rhport]); +/* Only dump valid port status */ +if (rhport >=0 ) { +dump_port_status (prev_port_status[rhport]); +dump_port_status (dum->port_status[rhport]); + } }usbip_dbg_vhci_rh (" bye\n" );
--
1.7.2.5
Quoted 18 lines · Németh Márton
From: Márton Németh <[email protected]>
The wIndex parameter of
vhci_hub_control()is always zero when the request type is GetHubDescriptor, see drivers/usb/core/hub.c::get_hub_descriptor() and Universal Serial Bus Specification revision 2.0 (April 27, 2000) Section 11.24.2.5 "Get Hub Descriptor". This means the variable rhport will not contain any useful index. Only use valid rhport values.Signed-off-by Márton Németh <[email protected]>--- drivers/staging/usbip/vhci_hcd.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)drivers/staging/usbip/vhci_hcd.c +0
-0
drivers/staging/usbip/vhci_hcd.c static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, · line 471
Quoted 10 lines
drivers/staging/usbip/vhci_hcd.c +5
-2@@ -464,8 +464,11 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,if (usbip_dbg_flag_vhci_rh) {pr_debug ("port %d\n" , rhport); -dump_port_status (prev_port_status[rhport]); -dump_port_status (dum->port_status[rhport]); +/* Only dump valid port status */ +if (rhport >=0 ) { +dump_port_status (prev_port_status[rhport]); +dump_port_status (dum->port_status[rhport]); + }
Umm, you mean to use "rhport > 0" yes?
Quoted 20 lines · Joe Perches
Quoted 19 lines · Németh Márton
From: Márton Németh <[email protected]>
The wIndex parameter of
vhci_hub_control()is always zero when the request type is GetHubDescriptor, see drivers/usb/core/hub.c::get_hub_descriptor() and Universal Serial Bus Specification revision 2.0 (April 27, 2000) Section 11.24.2.5 "Get Hub Descriptor". This means the variable rhport will not contain any useful index. Only use valid rhport values.Signed-off-by Márton Németh <[email protected]>--- drivers/staging/usbip/vhci_hcd.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)drivers/staging/usbip/vhci_hcd.c +0
-0@@ -464,8 +464,11 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
Quoted 11 lines
drivers/staging/usbip/vhci_hcd.c static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, · line 471
Quoted 9 lines
drivers/staging/usbip/vhci_hcd.c +5
-2if (usbip_dbg_flag_vhci_rh) {pr_debug ("port %d\n" , rhport); -dump_port_status (prev_port_status[rhport]); -dump_port_status (dum->port_status[rhport]); +/* Only dump valid port status */ +if (rhport >=0 ) { +dump_port_status (prev_port_status[rhport]); +dump_port_status (dum->port_status[rhport]); + }Umm, you mean to use "rhport > 0" yes?
No, it is really (rhport >= 0). The wIndex parameter is zero, the rhport is wIndex - 1. So rhport zero is valid, rhport -1 is not valid.
Regards,
Márton Németh
Quoted 8 lines · Németh Márton
From: Márton Németh <[email protected]>
The wIndex parameter of
vhci_hub_control()is always zero when the request type is GetHubDescriptor, see drivers/usb/core/hub.c::get_hub_descriptor() and Universal Serial Bus Specification revision 2.0 (April 27, 2000) Section 11.24.2.5 "Get Hub Descriptor". This means the variable rhport will not contain any useful index. Only use valid rhport values.
Is it possible for wValue to be zero if the request type is is SetPortFeature or ClearPortFeature? If so then we should probably add some more checks to handle that.
regards,
dan carpenter