linux-kernel - Re: drivers: usb: dwc3: A question...

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e5742c34-170c-0e7b-f3bf-55fbf151b458@ti.com>
Date:   Thu, 9 Feb 2017 10:45:46 +0200
From:   Roger Quadros <rogerq@...com>
To:     "Gustavo A. R. Silva" <garsilva@...eddedor.com>,
        <balbi@...nel.org>, Greg KH <greg@...ah.com>
CC:     <linux-usb@...r.kernel.org>, <linux-omap@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Peter Senna Tschudin <peter.senna@...il.com>
Subject: Re: drivers: usb: dwc3: A question...

Hi,

On 09/02/17 10:14, Gustavo A. R. Silva wrote:
> Hello everybody,
> 
> I ran into the following piece of code at drivers/usb/dwc3/dwc3-omap.c:218 (linux-next)
> 
> 218static void dwc3_omap_set_mailbox(struct dwc3_omap *omap,
> 219        enum omap_dwc3_vbus_id_status status)
> 220{
> 221        int     ret;
> 222        u32     val;
> 223
> 224        switch (status) {
> 225        case OMAP_DWC3_ID_GROUND:
> 226                if (omap->vbus_reg) {
> 227                        ret = regulator_enable(omap->vbus_reg);
> 228                        if (ret) {
> 229                                dev_err(omap->dev, "regulator enable failed\n");
> 230                                return;
> 231                        }
> 232                }
> 233
> 234                val = dwc3_omap_read_utmi_ctrl(omap);
> 235                val &= ~USBOTGSS_UTMI_OTG_CTRL_IDDIG;
> 236                dwc3_omap_write_utmi_ctrl(omap, val);
> 237                break;
> 238
> 239        case OMAP_DWC3_VBUS_VALID:
> 240                val = dwc3_omap_read_utmi_ctrl(omap);
> 241                val &= ~USBOTGSS_UTMI_OTG_CTRL_SESSEND;
> 242                val |= USBOTGSS_UTMI_OTG_CTRL_VBUSVALID
> 243                                | USBOTGSS_UTMI_OTG_CTRL_SESSVALID;
> 244                dwc3_omap_write_utmi_ctrl(omap, val);
> 245                break;
> 246
> 247        case OMAP_DWC3_ID_FLOAT:
> 248                if (omap->vbus_reg)
> 249                        regulator_disable(omap->vbus_reg);
> 250                val = dwc3_omap_read_utmi_ctrl(omap);
> 251                val |= USBOTGSS_UTMI_OTG_CTRL_IDDIG;
> 252                dwc3_omap_write_utmi_ctrl(omap, val);
> 253
> 254        case OMAP_DWC3_VBUS_OFF:
> 255                val = dwc3_omap_read_utmi_ctrl(omap);
> 256                val &= ~(USBOTGSS_UTMI_OTG_CTRL_SESSVALID
> 257                                | USBOTGSS_UTMI_OTG_CTRL_VBUSVALID);
> 258                val |= USBOTGSS_UTMI_OTG_CTRL_SESSEND;
> 259                dwc3_omap_write_utmi_ctrl(omap, val);
> 260                break;
> 261
> 262        default:
> 263                dev_WARN(omap->dev, "invalid state\n");
> 264        }
> 265}
> 
> The thing is that the case for OMAP_DWC3_ID_FLOAT is not terminated by a break statement, and it falls through to the next case OMAP_DWC3_VBUS_OFF.
> 
> My question here is if for any reason this code is intentional?
> 
> In case it is not, I will write a patch to fix this, but first it would be great to hear any comment about it.

You are right that it is a mistake. Thanks for pointing out.
I had already submitted a patch
https://patchwork.kernel.org/patch/9532195/

Felipe,
I think we should pick it up regardless of the rest of the series and mark for -stable.

Please do add
Reported-by: Gustavo A. R. Silva <garsilva@...eddedor.com>
Thanks :)

cheers,
-roger