linux-kernel - Re: [PATCH v5 13/14] platform/x86: wmi: create character devices when requested by drivers

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

Message-ID: <20171007123804.GB5667@kroah.com>
Date:   Sat, 7 Oct 2017 14:38:04 +0200
From:   Greg KH <greg@...ah.com>
To:     Mario.Limonciello@...l.com
Cc:     dvhart@...radead.org, andy.shevchenko@...il.com,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        luto@...nel.org, quasisec@...gle.com, pali.rohar@...il.com,
        rjw@...ysocki.net, mjg59@...gle.com, hch@....de
Subject: Re: [PATCH v5 13/14] platform/x86: wmi: create character devices
 when requested by drivers

On Sat, Oct 07, 2017 at 11:59:52AM +0000, Mario.Limonciello@...l.com wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:greg@...ah.com]
> > Sent: Saturday, October 7, 2017 2:35 AM
> > To: Limonciello, Mario <Mario_Limonciello@...l.com>
> > Cc: dvhart@...radead.org; Andy Shevchenko <andy.shevchenko@...il.com>;
> > LKML <linux-kernel@...r.kernel.org>; platform-driver-x86@...r.kernel.org;
> > Andy Lutomirski <luto@...nel.org>; quasisec@...gle.com;
> > pali.rohar@...il.com; rjw@...ysocki.net; mjg59@...gle.com; hch@....de
> > Subject: Re: [PATCH v5 13/14] platform/x86: wmi: create character devices when
> > requested by drivers
> > 
> > On Fri, Oct 06, 2017 at 11:59:57PM -0500, Mario Limonciello wrote:
> > > For WMI operations that are only Set or Query read or write sysfs
> > > attributes created by WMI vendor drivers make sense.
> > >
> > > For other WMI operations that are run on Method, there needs to be a
> > > way to guarantee to userspace that the results from the method call
> > > belong to the data request to the method call.  Sysfs attributes don't
> > > work well in this scenario because two userspace processes may be
> > > competing at reading/writing an attribute and step on each other's
> > > data.
> > >
> > > When a WMI vendor driver declares an ioctl callback in the wmi_driver
> > > the WMI bus driver will create a character device that maps to that
> > > function.
> > >
> > > That character device will correspond to this path:
> > > /dev/wmi/$driver
> > >
> > > The WMI bus driver will interpret the IOCTL calls, test them for
> > > a valid instance and pass them on to the vendor driver to run.
> > >
> > > This creates an implicit policy that only driver per character
> > > device.  If a module matches multiple GUID's, the wmi_devices
> > > will need to be all handled by the same wmi_driver if the same
> > > character device is used.
> > >
> > > The WMI vendor drivers will be responsible for managing access to
> > > this character device and proper locking on it.
> > >
> > > When a WMI vendor driver is unloaded the WMI bus driver will clean
> > > up the character device.
> > 
> > What prevents the vendor driver from being unloaded while the ioctl is
> > being called in it?  I don't see any protection here from that at all :(
> > 
> 
> In my driver I take a mutex that blocks unloading while running ioctl, 
> but you mean you want one in the bus driver more generally, OK.

No, you need to increment the module reference count before calling into
it, which prevents anyone from even attempting to unload it.  Much
simpler than trying to rely on a mutex that is not obviously documented
that this is what it does (hint, sharing a mutex across modules is not a
good idea...)


thanks,

greg k-h