linux-kernel - RE: [PATCH v3 RESEND 4/4] usb: core: add phy notify connect and disconnect

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] [day] [month] [year] [list]

Date:   Thu, 7 Dec 2023 10:43:54 +0000
From:   Stanley Chang[昌育德] 
        <stanley_chang@...ltek.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     Vinod Koul <vkoul@...nel.org>, Johan Hovold <johan@...nel.org>,
        "Kishon Vijay Abraham I" <kishon@...nel.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Rob Herring <robh@...nel.org>,
        Jinjie Ruan <ruanjinjie@...wei.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Roy Luo <royluo@...gle.com>,
        Flavio Suligoi <f.suligoi@...m.it>,
        Ricardo Cañuelo 
        <ricardo.canuelo@...labora.com>,
        "Heikki Krogerus" <heikki.krogerus@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: RE: [PATCH v3 RESEND 4/4] usb: core: add phy notify connect and disconnect

Hi Greg,

> > +int usb_phy_roothub_notify_connect(struct usb_phy_roothub
> > +*phy_roothub, int port) {
> > +     struct usb_phy_roothub *roothub_entry;
> > +     struct list_head *head;
> > +     int err;
> > +
> > +     if (!phy_roothub)
> > +             return 0;
> 
> How can phy_roothub ever be NULL?
> 
This is possible. If the host no use generic phy, then usb_phy_roothub_alloc will return NULL.
And other callbacks also follow this rule.

> > +
> > +     head = &phy_roothub->list;
> > +
> > +     list_for_each_entry(roothub_entry, head, list) {
> > +             err = phy_notify_connect(roothub_entry->phy, port);
> > +             if (err)
> > +                     return err;
> > +     }
> >
> 
> You walk a list with no locking at all?  That does not seem right at all.

The lock seems unnecessary.
And other similar APIs also don't use any locks.

> Also, this is a new function that is exported with no documentation?
> Please fix.
> 
Okay, I will add.

Thanks,
Stanley