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, 11 Apr 2019 18:12:30 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     Yu Chen <chenyu56@...wei.com>
Cc:     Linux USB List <linux-usb@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, lkml <linux-kernel@...r.kernel.org>,
        Zhuangluan Su <suzhuangluan@...ilicon.com>,
        Kongfei <kongfei@...ilicon.com>,
        "Liuyu (R)" <liuyu712@...ilicon.com>, wanghu17@...ilicon.com,
        butao <butao@...ilicon.com>, chenyao11@...wei.com,
        fangshengzhou@...ilicon.com,
        Li Pengcheng <lipengcheng8@...wei.com>,
        songxiaowei@...ilicon.com, YiPing Xu <xuyiping@...ilicon.com>,
        xuyoujun4@...wei.com, Yudongbin <yudongbin@...ilicon.com>,
        Zang Leigang <zangleigang@...ilicon.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>
Subject: Re: [PATCH v5 09/13] usb: roles: Add usb role switch notifier.

On Thu, Mar 28, 2019 at 9:14 PM Yu Chen <chenyu56@...wei.com> wrote:
>
> This patch adds notifier for drivers want to be informed of the usb role
> switch.
>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> Cc: Hans de Goede <hdegoede@...hat.com>
> Cc: Andy Shevchenko <andy.shevchenko@...il.com>
> Cc: John Stultz <john.stultz@...aro.org>
> Suggested-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> Signed-off-by: Yu Chen <chenyu56@...wei.com>

Hey Yu Chen!
   Thanks again for sending this patch out! As mentioned in my
comments with the other patches, I've got one proposal I wanted to
share to try to avoid  state initialization races that I've seen with
this patchset.

> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> index f45d8df5cfb8..e2caaa665d6e 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -58,6 +61,20 @@ int usb_role_switch_set_role(struct usb_role_switch *sw, enum usb_role role)
>  }
>  EXPORT_SYMBOL_GPL(usb_role_switch_set_role);
>
> +int usb_role_switch_register_notifier(struct usb_role_switch *sw,
> +                                     struct notifier_block *nb)
> +{
> +       return blocking_notifier_chain_register(&sw->nh, nb);
> +}
> +EXPORT_SYMBOL_GPL(usb_role_switch_register_notifier);

As noted earlier, one issue I've seen is that the hisi_hikey_usb
driver's notifier may not get called early enough to receive
notification of the initial usb state.

It seems like on registration here, we should take the lock, read the
role state and immediately call the notifier to properly initialize
it? I suspect that should close the window for any state races around
driver probe timings and

Does that make sense? I have roughly prototyped this but need to do
additional testing.

thanks
-john

Powered by blists - more mailing lists