Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756175AbdDRVOh (ORCPT ); Tue, 18 Apr 2017 17:14:37 -0400 Received: from mail-oi0-f66.google.com ([209.85.218.66]:36788 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755643AbdDRVOb (ORCPT ); Tue, 18 Apr 2017 17:14:31 -0400 MIME-Version: 1.0 In-Reply-To: <20170418163325.GC25405@fury> References: <20170412230854.GA11963@fury> <20170417231051.GB7183@fury> <3317007.TCiWpp4m4U@aspire.rjw.lan> <20170418163325.GC25405@fury> From: "Rafael J. Wysocki" Date: Tue, 18 Apr 2017 23:14:29 +0200 X-Google-Sender-Auth: CjtQpMNJDUKYaINUaO8xisF_Om4 Message-ID: Subject: Re: RFC: WMI Enhancements To: Darren Hart Cc: "Rafael J. Wysocki" , Andy Lutomirski , Len Brown , =?UTF-8?Q?Pali_Roh=C3=A1r?= , Corentin Chary , Mario Limonciello , Andy Lutomirski , Andy Shevchenko , LKML , platform-driver-x86@vger.kernel.org, "linux-pm@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8837 Lines: 158 On Tue, Apr 18, 2017 at 6:33 PM, Darren Hart wrote: > On Tue, Apr 18, 2017 at 03:07:06PM +0200, Rafael Wysocki wrote: >> On Monday, April 17, 2017 04:10:51 PM Darren Hart wrote: >> > On Mon, Apr 17, 2017 at 03:03:29PM -0700, Andy Lutomirski wrote: >> > > On Fri, Apr 14, 2017 at 4:05 PM, Darren Hart wrote: >> > > > On Sat, Apr 15, 2017 at 12:45:30AM +0200, Rafael Wysocki wrote: >> > > >> On Wednesday, April 12, 2017 04:08:54 PM Darren Hart wrote: >> > > >> > Hi All, >> > > >> > >> > > >> > There are a few parallel efforts involving the Windows Management >> > > >> > Instrumentation (WMI)[1] and dependent/related drivers. I'd like to have a round of >> > > >> > discussion among those of you that have been involved in this space before we >> > > >> > decide on a direction. >> > > >> > >> > > >> > The WMI support in the kernel today fairly narrowly supports a handful of >> > > >> > systems. Andy L. has a work-in-progress series [2] which converts wmi into a >> > > >> > platform device and a proper bus, providing devices for dependent drivers to >> > > >> > bind to, and a mechanism for sibling devices to communicate with each other. >> > > >> > I've reviewed the series and feel like the approach is sound, I plan to carry >> > > >> > this series forward and merge it (with Andy L's permission). >> > > >> > >> > > >> > Are there any objections to this? >> > > >> > >> > > >> > In Windows, applications interact with WMI more or less directly. We don't do >> > > >> > this in Linux currently, although it has been discussed in the past [3]. Some >> > > >> > vendors will work around this by performing SMI/SMM, which is inefficient at >> > > >> > best. Exposing WMI methods to userspace would bring parity to WMI for Linux and >> > > >> > Windows. >> > > >> > >> > > >> > There are two principal concerns I'd appreciate your thoughts on: >> > > >> > >> > > >> > a) As an undiscoverable interface (you need to know the method signatures ahead >> > > >> > of time), universally exposing every WMI "device" to userspace seems like "a bad >> > > >> > idea" from a security and stability perspective. While access would certainly be >> > > >> > privileged, it seems more prudent to make this exposure opt-in. We also handle >> > > >> > some of this with kernel drivers and exposing those "devices" to userspace would >> > > >> > enable userspace and the kernel to fight over control. So - if we expose WMI >> > > >> > devices to userspace, I believe this should be done on a case by case basis, >> > > >> > opting in, and not by default as part of the WMI driver (although it can provide >> > > >> > the mechanism for a sub-driver to use), and possibly a devmode to do so by >> > > >> > default. >> > > >> >> > > >> A couple of loose thoughts here. >> > > >> >> > > >> In principle there could be a "generic default WMI driver" or similar that would >> > > >> "claim" all WMI "devices" that have not been "claimed" by anyone else and would >> > > >> simply expose them to user space somehow (e.g. using a chardev interface). >> > > >> >> > > >> Then, depending on how that thing is implemented, opt-in etc should be possible >> > > >> too. >> > > >> >> > > > >> > > > I think we agree this would be an ideal approach. >> > > > >> > > > As we look into this more, it is becoming clear that the necessary functionality >> > > > is not nicely divided into GUIDs for what is necessary in userspace and what is >> > > > handled in the kernel. A single WMI METHOD GUID may be needed by userspace for >> > > > certain functionality, while the kernel drivers may use it for something else. >> > > > >> > > > :-( >> > > > >> > > > The input to a WMI method is just a buffer, so it is very free form. One >> > > > approach Mario has mentioned was to audit the user space WMI METHOD calls in the >> > > > kernel platform drivers and reject those calls with arguments matching those >> > > > issued by the kernel driver. This is likely to be complex and error prone in my >> > > > opinion. However, I have not yet thought of another means to meet the >> > > > requirement of having disjoint feature sets for userspace and kernel space via a >> > > > mechanism that was effectively designed to be used solely from user space with >> > > > vendor defined method signatures. >> > > > >> > > > Next step is to look at just how complex it would be to audit the method calls >> > > > the kernel currently uses. >> > > >> > > I'm wondering whether it's really worth it. We already allow doing >> > > darned near anything using dcdbas. Maybe the world won't end if we >> > > expose a complete-ish ioctl interface to WMI. >> >> I guess the world wouldn't end then (it has not ended for far more serious >> reasons so far after all), but this also doesn't feel entirely right. >> >> For one, if something is used inside of the kernel (by drivers etc), then >> allowing user space to use the same thing directly is a recipe for unsupportable >> mess IMO. > > I don't disagree. Unforuntately, the mechanism wasn't designed for this kind of > mixed usage from what I can determine, so it doesn't lend itself to separation. > We could kick out all the WMI drivers and encourage vendor/platform specific > system daemons which read WMI and injected events and configured LEDs through > sysfs, thus eliminating the user/kernel conflict - but it would only leave us > with the problem of multiple userspace daemons competing for the same WMI > METHODs -- and yeah, nobody's going for that :-D Yeah, surely no one. :-) >> >> > > Also, dcdbas is, to put it mildly, a bit ridiculous. It seems to be a >> > > seriously awkward sysfs interface that allows you to, drumroll please, >> > > issue outb and inb instructions. It doesn't even check that it's >> > > running on a Dell system. It might be nice to deprecate it some day >> > > in favor of a real interface. I'd consider a low-level WMI ioctl >> > > interface to be a vast improvement. >> > > >> > >> > I've been reluctantly arriving here as well. Given that every WMI interface will >> > be vendor specific, and non-discoverable, it seems unlikely developers will >> > eagerly duplicate kernel functionality in user-space. And if they do, it will >> > affect very few platforms. >> > >> > I still think it makes sense to only expose a WMI interface by default on some >> > matching criteria. It could be DMI related, but I'd like to know if the UID is >> > possible as well (it depends on how vendors use the UID, if consistently, not at >> > all, etc.) Otherwise, the interface would not be enabled unless the user >> > explicitly requests it via a module parameter or similar. >> >> To me, that should be the bare minimum, but I really think that mutual exclusion >> between user space and the kernel needs to be ensured somehow when the >> interface is enabled too. >> >> This looks similar to exposing _DSM functionality for certain device to user >> space where some functions of the _DSM in question are already in use by >> kernel code. In that case I would think about an interface with a function >> granularity (so it would check the GUID and the function and possibly the >> ordering with respect to the other functions too before invoking the _DSM >> on behalf of user space). > > This is also what I would consider to be ideal, but the mechanism doesn't lend > itself to that level of granularity. WMI methods are not guaranteed to be broken > up into sufficiently granular functionality that we can filter based on method > ID. We would most likely end up in the position of having to audit the input > buffer of every WMI call. > > For example, we can filter things the ASUS WMI Keyboard Filter method, but > others are less specific, like Device Set, Bios Status, Device Status, Device > Policy, etc. > > What we could do is make that the vendor's problem instead of the kernel's > problem. Consider: > > * wmi.c adds method evaluation wrappers > * add a wmi evaluation mutex > * update *wmi.c drivers to use the new wrappers > * platform drivers (dell-wmi.c, asus-wmi.c, etc.) must explicitly request > wmi.c to export the wmi chardev > * platform drivers must explicitly whitelist each method ID to be exported > - they can automate this in a loop evaluating the wmi block if they wish > * platform drivers *may* register a wmi evaluation filter which allows them > to audit the method id and input buffer to ensure it doesn't conflict with > in-kernel usage (their usage). > > I believe this is a reasonable compromise, and it places the burden on the > platform drivers, and therefor on the vendors (in the best case) or the > individual platform driver maintainers for less cooperative vendors. This > contains any resulting exposure to the platforms which explicitly request it. That would work for me. Thanks, Rafael