| Message ID | f1e7105994dc973dea649f1fb6b0c58b0760c31b.1507958847.git.mario.limonciello@dell.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Darren Hart |
| Headers | show |
Hi Mario,
[auto build test WARNING on platform-drivers-x86/for-next]
[also build test WARNING on next-20171016]
[cannot apply to v4.14-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Mario-Limonciello/Introduce-support-for-Dell-SMBIOS-over-WMI/20171017-103109
base: git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git for-next
config: i386-randconfig-x002-201742 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/ioport.h:12:0,
from include/linux/acpi.h:25,
from drivers/platform/x86/wmi.c:36:
drivers/platform/x86/wmi.c: In function 'match_ioctl':
drivers/platform/x86/wmi.c:861:14: error: 'struct wmi_driver' has no member named 'compat_ioctl'; did you mean 'unlocked_ioctl'?
if (wdriver->compat_ioctl)
^
include/linux/compiler.h:156:30: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/platform/x86/wmi.c:861:3: note: in expansion of macro 'if'
if (wdriver->compat_ioctl)
^~
drivers/platform/x86/wmi.c:861:14: error: 'struct wmi_driver' has no member named 'compat_ioctl'; did you mean 'unlocked_ioctl'?
if (wdriver->compat_ioctl)
^
include/linux/compiler.h:156:42: note: in definition of macro '__trace_if'
if (__builtin_constant_p(!!(cond)) ? !!(cond) : \
^~~~
>> drivers/platform/x86/wmi.c:861:3: note: in expansion of macro 'if'
if (wdriver->compat_ioctl)
^~
drivers/platform/x86/wmi.c:861:14: error: 'struct wmi_driver' has no member named 'compat_ioctl'; did you mean 'unlocked_ioctl'?
if (wdriver->compat_ioctl)
^
include/linux/compiler.h:167:16: note: in definition of macro '__trace_if'
______r = !!(cond); \
^~~~
>> drivers/platform/x86/wmi.c:861:3: note: in expansion of macro 'if'
if (wdriver->compat_ioctl)
^~
drivers/platform/x86/wmi.c:862:17: error: 'struct wmi_driver' has no member named 'compat_ioctl'; did you mean 'unlocked_ioctl'?
ret = wdriver->compat_ioctl(&wblock->dev, cmd, arg);
^~
At top level:
drivers/platform/x86/wmi.c:877:13: warning: 'wmi_compat_ioctl' defined but not used [-Wunused-function]
static long wmi_compat_ioctl(struct file *filp, unsigned int cmd,
^~~~~~~~~~~~~~~~
vim +/if +861 drivers/platform/x86/wmi.c
802
803 static long match_ioctl(struct file *filp, unsigned int cmd, unsigned long arg,
804 int compat)
805 {
806 struct wmi_ioctl_buffer __user *input =
807 (struct wmi_ioctl_buffer __user *) arg;
808 struct wmi_driver *wdriver = NULL;
809 struct wmi_block *wblock = NULL;
810 struct wmi_block *next = NULL;
811 const char *driver_name;
812 u64 size;
813 int ret;
814
815 if (_IOC_TYPE(cmd) != WMI_IOC)
816 return -ENOTTY;
817
818 driver_name = filp->f_path.dentry->d_iname;
819
820 list_for_each_entry_safe(wblock, next, &wmi_block_list, list) {
821 wdriver = container_of(wblock->dev.dev.driver,
822 struct wmi_driver, driver);
823 if (!wdriver)
824 continue;
825 if (strcmp(driver_name, wdriver->driver.name) == 0)
826 break;
827 }
828
829 if (!wdriver)
830 return -ENODEV;
831
832 /* make sure we're not calling a higher instance than exists*/
833 if (_IOC_NR(cmd) >= wblock->gblock.instance_count)
834 return -EINVAL;
835
836 /* check that required buffer size was declared by driver */
837 if (!wblock->req_buf_size) {
838 dev_err(&wblock->dev.dev, "Required buffer size not set\n");
839 return -EINVAL;
840 }
841 if (get_user(size, &input->length)) {
842 dev_dbg(&wblock->dev.dev, "Read length from user failed\n");
843 return -EFAULT;
844 }
845 /* if it's too small, abort */
846 if (size < wblock->req_buf_size) {
847 dev_err(&wblock->dev.dev,
848 "Buffer %lld too small, need at least %lld\n",
849 size, wblock->req_buf_size);
850 return -EINVAL;
851 }
852 /* if it's too big, warn, driver will only use what is needed */
853 if (size > wblock->req_buf_size)
854 dev_warn(&wblock->dev.dev,
855 "Buffer %lld is bigger than required %lld\n",
856 size, wblock->req_buf_size);
857
858 if (!try_module_get(wdriver->driver.owner))
859 return -EBUSY;
860 if (compat) {
> 861 if (wdriver->compat_ioctl)
862 ret = wdriver->compat_ioctl(&wblock->dev, cmd, arg);
863 else
864 ret = -ENODEV;
865 } else
866 ret = wdriver->unlocked_ioctl(&wblock->dev, cmd, arg);
867 module_put(wdriver->driver.owner);
868
869 return ret;
870 }
871 static long wmi_unlocked_ioctl(struct file *filp, unsigned int cmd,
872 unsigned long arg)
873 {
874 return match_ioctl(filp, cmd, arg, 0);
875 }
876
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/MAINTAINERS b/MAINTAINERS index 8fd2f0d2ddf6..84afbf8ef7d5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -384,6 +384,7 @@ ACPI WMI DRIVER L: platform-driver-x86@vger.kernel.org S: Orphan F: drivers/platform/x86/wmi.c +F: include/uapi/linux/wmi.h AD1889 ALSA SOUND DRIVER M: Thibaut Varene <T-Bone@parisc-linux.org> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index 63d01f98bf4c..d1742d392cd1 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -38,12 +38,15 @@ #include <linux/init.h> #include <linux/kernel.h> #include <linux/list.h> +#include <linux/miscdevice.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/types.h> +#include <linux/uaccess.h> #include <linux/uuid.h> #include <linux/wmi.h> +#include <uapi/linux/wmi.h> ACPI_MODULE_NAME("wmi"); MODULE_AUTHOR("Carlos Corbacho"); @@ -69,6 +72,7 @@ struct wmi_block { struct wmi_device dev; struct list_head list; struct guid_block gblock; + struct miscdevice misc_dev; struct acpi_device *acpi_device; wmi_notify_handler handler; void *handler_data; @@ -796,12 +800,119 @@ static int wmi_dev_match(struct device *dev, struct device_driver *driver) return 0; } +static long match_ioctl(struct file *filp, unsigned int cmd, unsigned long arg, + int compat) +{ + struct wmi_ioctl_buffer __user *input = + (struct wmi_ioctl_buffer __user *) arg; + struct wmi_driver *wdriver = NULL; + struct wmi_block *wblock = NULL; + struct wmi_block *next = NULL; + const char *driver_name; + u64 size; + int ret; + + if (_IOC_TYPE(cmd) != WMI_IOC) + return -ENOTTY; + + driver_name = filp->f_path.dentry->d_iname; + + list_for_each_entry_safe(wblock, next, &wmi_block_list, list) { + wdriver = container_of(wblock->dev.dev.driver, + struct wmi_driver, driver); + if (!wdriver) + continue; + if (strcmp(driver_name, wdriver->driver.name) == 0) + break; + } + + if (!wdriver) + return -ENODEV; + + /* make sure we're not calling a higher instance than exists*/ + if (_IOC_NR(cmd) >= wblock->gblock.instance_count) + return -EINVAL; + + /* check that required buffer size was declared by driver */ + if (!wblock->req_buf_size) { + dev_err(&wblock->dev.dev, "Required buffer size not set\n"); + return -EINVAL; + } + if (get_user(size, &input->length)) { + dev_dbg(&wblock->dev.dev, "Read length from user failed\n"); + return -EFAULT; + } + /* if it's too small, abort */ + if (size < wblock->req_buf_size) { + dev_err(&wblock->dev.dev, + "Buffer %lld too small, need at least %lld\n", + size, wblock->req_buf_size); + return -EINVAL; + } + /* if it's too big, warn, driver will only use what is needed */ + if (size > wblock->req_buf_size) + dev_warn(&wblock->dev.dev, + "Buffer %lld is bigger than required %lld\n", + size, wblock->req_buf_size); + + if (!try_module_get(wdriver->driver.owner)) + return -EBUSY; + if (compat) { + if (wdriver->compat_ioctl) + ret = wdriver->compat_ioctl(&wblock->dev, cmd, arg); + else + ret = -ENODEV; + } else + ret = wdriver->unlocked_ioctl(&wblock->dev, cmd, arg); + module_put(wdriver->driver.owner); + + return ret; +} +static long wmi_unlocked_ioctl(struct file *filp, unsigned int cmd, + unsigned long arg) +{ + return match_ioctl(filp, cmd, arg, 0); +} + +static long wmi_compat_ioctl(struct file *filp, unsigned int cmd, + unsigned long arg) +{ + return match_ioctl(filp, cmd, arg, 1); +} + +static const struct file_operations wmi_fops = { + .owner = THIS_MODULE, + .unlocked_ioctl = wmi_unlocked_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = wmi_compat_ioctl, +#endif +}; + static int wmi_dev_probe(struct device *dev) { struct wmi_block *wblock = dev_to_wblock(dev); struct wmi_driver *wdriver = container_of(dev->driver, struct wmi_driver, driver); int ret = 0; + char *buf; + + /* driver wants a character device made */ + if (wdriver->unlocked_ioctl) { + buf = kmalloc(strlen(wdriver->driver.name) + 4, GFP_KERNEL); + if (!buf) + return -ENOMEM; + sprintf(buf, "wmi/%s", wdriver->driver.name); + wblock->misc_dev.minor = MISC_DYNAMIC_MINOR; + wblock->misc_dev.name = buf; + wblock->misc_dev.fops = &wmi_fops; + wblock->misc_dev.mode = 0444; + ret = misc_register(&wblock->misc_dev); + if (ret) { + dev_warn(dev, "failed to register char dev: %d", ret); + kfree(buf); + return -ENOMEM; + } + } if (ACPI_FAILURE(wmi_method_enable(wblock, 1))) dev_warn(dev, "failed to enable device -- probing anyway\n"); @@ -822,6 +933,11 @@ static int wmi_dev_remove(struct device *dev) container_of(dev->driver, struct wmi_driver, driver); int ret = 0; + if (wdriver->unlocked_ioctl) { + misc_deregister(&wblock->misc_dev); + kfree(wblock->misc_dev.name); + } + if (wdriver->remove) ret = wdriver->remove(dev_to_wdev(dev)); diff --git a/include/linux/wmi.h b/include/linux/wmi.h index a9a72a4c5ed8..1f52dc49a896 100644 --- a/include/linux/wmi.h +++ b/include/linux/wmi.h @@ -50,6 +50,12 @@ struct wmi_driver { int (*probe)(struct wmi_device *wdev); int (*remove)(struct wmi_device *wdev); void (*notify)(struct wmi_device *device, union acpi_object *data); + long (*unlocked_ioctl)(struct wmi_device *wdev, unsigned int cmd, + unsigned long arg); +#ifdef CONFIG_COMPAT + long (*compat_ioctl)(struct wmi_device *wdev, unsigned int cmd, + unsigned long arg); +#endif }; extern int __must_check __wmi_driver_register(struct wmi_driver *driver, diff --git a/include/uapi/linux/wmi.h b/include/uapi/linux/wmi.h new file mode 100644 index 000000000000..7e52350ac9b3 --- /dev/null +++ b/include/uapi/linux/wmi.h @@ -0,0 +1,26 @@ +/* + * User API methods for ACPI-WMI mapping driver + * + * Copyright (C) 2017 Dell, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#ifndef _UAPI_LINUX_WMI_H +#define _UAPI_LINUX_WMI_H + +#include <linux/types.h> + +/* WMI bus will filter all WMI vendor driver requests through this IOC */ +#define WMI_IOC 'W' + +/* All ioctl requests through WMI should declare their size followed by + * relevant data objects + */ +struct wmi_ioctl_buffer { + __u64 length; + __u8 data[]; +}; + +#endif
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. Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> --- MAINTAINERS | 1 + drivers/platform/x86/wmi.c | 116 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/wmi.h | 6 +++ include/uapi/linux/wmi.h | 26 ++++++++++ 4 files changed, 149 insertions(+) create mode 100644 include/uapi/linux/wmi.h