lkml.org 
[lkml]   [2017]   [Oct]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH v6 05/14] platform/x86: dell-wmi-descriptor: split WMI descriptor into it's own driver
Date
> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Tuesday, October 10, 2017 3:18 AM
> To: Limonciello, Mario <Mario_Limonciello@Dell.com>
> Cc: dvhart@infradead.org; Andy Shevchenko <andy.shevchenko@gmail.com>;
> LKML <linux-kernel@vger.kernel.org>; platform-driver-x86@vger.kernel.org;
> Andy Lutomirski <luto@kernel.org>; quasisec@google.com; rjw@rjwysocki.net;
> mjg59@google.com; hch@lst.de; Greg KH <greg@kroah.com>
> Subject: Re: [PATCH v6 05/14] platform/x86: dell-wmi-descriptor: split WMI
> descriptor into it's own driver
>
> On Monday 09 October 2017 17:51:43 Mario Limonciello wrote:
> > +bool dell_wmi_get_interface_version(u32 *version)
> > +{
> > + struct descriptor_priv *priv;
> > +
> > + priv = list_first_entry_or_null(&wmi_list,
> > + struct descriptor_priv,
> > + list);
> > + if (!priv)
> > + return false;
>
> This introduce race condition. In case callee of this function is
> executed before binding 8D9DDCBC-A997-11DA-B012-B622A1EF5492 WMI device
> into this dell-wmi-descriptor driver, then this function
> dell_wmi_get_interface_version() returns false. Even it is running on
> supported machine.
>
> Loading of modulus and also binding devices to drivers (registered by
> modules) is is asynchronous.
>

I suppose you're right, module dependency ordering doesn't necessarily
guarantee binding. I'll add in some deferred probing and binding checking.

> In case everything is linked into vmlinux binary, this problem could
> happen too. Binding & initialization of WMI device to this driver can be
> postponed and callee (dell-wmi) would get false.

>
> > + *version = priv->interface_version;
> > + return true;
> > +}
> > +EXPORT_SYMBOL_GPL(dell_wmi_get_interface_version);
> > +
> > +bool dell_wmi_get_size(u32 *size)
> > +{
> > + struct descriptor_priv *priv;
> > +
> > + priv = list_first_entry_or_null(&wmi_list,
> > + struct descriptor_priv,
> > + list);
> > + if (!priv)
> > + return false;
>
> And same there.
>
> > + *size = priv->size;
> > + return true;
> > +}
> > +EXPORT_SYMBOL_GPL(dell_wmi_get_size);
>
> --
> Pali Rohár
> pali.rohar@gmail.com
\
 
 \ /
  Last update: 2017-10-10 15:43    [W:0.069 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site