On Wed, 28 Aug 2013, Philippe De Swert wrote:
> Since USB_SS_PORT_LS_U0 is 0x0000 the & operation with the port state would
> always be 0. Thus the if would never be true. The code tries to check if the
> port is enabled and link is active (U0 state). This means the port_status
> should be 0x0001, so the right check is an | with USB_SS_PORT_LS_U0.
>
> Found with coverity: CID 744367, CID 145679
>
> Signed-off-by: Philippe De Swert <[email protected]>
> ---
> drivers/usb/gadget/dummy_hcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
> index 5c506fb..22cd01c 100644
> --- a/drivers/usb/gadget/dummy_hcd.c
> +++ b/drivers/usb/gadget/dummy_hcd.c
> @@ -313,7 +313,7 @@ static void set_link_state_by_speed(struct dummy_hcd
> *dum_hcd)
> (USB_PORT_STAT_C_CONNECTION << 16);
> if ((dum_hcd->port_status &
> USB_PORT_STAT_ENABLE) == 1 &&
> - (dum_hcd->port_status &
> + (dum_hcd->port_status |
> USB_SS_PORT_LS_U0) == 1 &&
> dum_hcd->rh_state != DUMMY_RH_SUSPENDED)
> dum_hcd->active = 1;
It's odd that Coverity found this bug but didn't find the bug
immediately above: Since USB_PORT_STAT_ENABLE is 0x0002, the first
comparison in this "if" statement can never succeed.
The test against USB_PORT_STAT_ENABLE should be != 0, not == 1.
The test against USB_SS_PORT_LS_U0 is dead code, because the driver
currently does not support USB-3 suspend. Aside from that, the
proposed fix is wrong. The correct test is
(dum_hcd->port_status & USB_SS_PORT_LINK_STATE) ==
USB_SS_PORT_LS_U0
Alan Stern
--
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
