linux-kernel - Re: drivers: usb: musb: question about missing break in switch

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 9 Feb 2017 07:36:44 -0600
From:   Bin Liu <b-liu@...com>
To:     "Gustavo A. R. Silva" <garsilva@...eddedor.com>
CC:     Greg KH <greg@...ah.com>, <linux-usb@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Peter Senna Tschudin <peter.senna@...il.com>
Subject: Re: drivers: usb: musb: question about missing break in switch

On Thu, Feb 09, 2017 at 02:37:34AM -0600, Gustavo A. R. Silva wrote:
> Hello everybody,
> 
> I ran into the following piece of code at
> drivers/usb/musb/musb_core.c:1854 (linux-next)
> 
> 1854/*
> 1855 * Check the musb devctl session bit to determine if we want to
> 1856 * allow PM runtime for the device. In general, we want to keep things
> 1857 * active when the session bit is set except after host disconnect.
> 1858 *
> 1859 * Only called from musb_irq_work. If this ever needs to get called
> 1860 * elsewhere, proper locking must be implemented for musb->session.
> 1861 */
> 1862static void musb_pm_runtime_check_session(struct musb *musb)
> 1863{
> 1864        u8 devctl, s;
> 1865        int error;
> 1866
> 1867        devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
> 1868
> 1869        /* Handle session status quirks first */
> 1870        s = MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV |
> 1871                MUSB_DEVCTL_HR;
> 1872        switch (devctl & ~s) {
> 1873        case MUSB_QUIRK_B_INVALID_VBUS_91:
> 1874                if (musb->quirk_retries--) {
> 1875                        musb_dbg(musb,
> 1876                                 "Poll devctl on invalid vbus,
> assume no session");
> 1877                        schedule_delayed_work(&musb->irq_work,
> 1878                                              msecs_to_jiffies(1000));
> 1879
> 1880                        return;
> 1881                }
> 1882        case MUSB_QUIRK_A_DISCONNECT_19:
> 1883                if (musb->quirk_retries--) {
> 1884                        musb_dbg(musb,
> 1885                                 "Poll devctl on possible host
> mode disconnect");
> 1886                        schedule_delayed_work(&musb->irq_work,
> 1887                                              msecs_to_jiffies(1000));
> 1888
> 1889                        return;
> 1890                }
> 1891                if (!musb->session)
> 1892                        break;
> 1893                musb_dbg(musb, "Allow PM on possible host mode
> disconnect");
> 1894                pm_runtime_mark_last_busy(musb->controller);
> 1895                pm_runtime_put_autosuspend(musb->controller);
> 1896                musb->session = false;
> 1897                return;
> 1898        default:
> 1899                break;
> 1900        }
> 1901
> 1902        /* No need to do anything if session has not changed */
> 1903        s = devctl & MUSB_DEVCTL_SESSION;
> 1904        if (s == musb->session)
> 1905                return;
> 1906
> 1907        /* Block PM or allow PM? */
> 1908        if (s) {
> 1909                musb_dbg(musb, "Block PM on active session:
> %02x", devctl);
> 1910                error = pm_runtime_get_sync(musb->controller);
> 1911                if (error < 0)
> 1912                        dev_err(musb->controller, "Could not
> enable: %i\n",
> 1913                                error);
> 1914                musb->quirk_retries = 3;
> 1915        } else {
> 1916                musb_dbg(musb, "Allow PM with no session: %02x", devctl);
> 1917                pm_runtime_mark_last_busy(musb->controller);
> 1918                pm_runtime_put_autosuspend(musb->controller);
> 1919        }
> 1920
> 1921        musb->session = s;
> 1922}
> 
> The thing is that the case for MUSB_QUIRK_B_INVALID_VBUS_91 is not
> terminated by a break statement, and it falls through to the next
> case MUSB_QUIRK_A_DISCONNECT_19, in case "if
> (musb->quirk_retries--)" turns to be false.
> 
> My question here is if this code is intentional?

Yes, it is. For both MUSB_QUIRK_B_INVALID_VBUS_91 and
MUSB_QUIRK_A_DISCONNECT_19 cases, we first do retries, then if
musb->session is set, we allow PM.

Regards,
-Bin.