Commit 500de3dd46ac9f9ae9d124634c68907b7d50d2cb

Authored by Thomas Renninger
Committed by Matthew Garrett
1 parent de4f10466e

acpi ec_sys: Be more cautious about ec write access

- Set Kconfig option default n
- Only allow root to read/write io file (sever bug!)
- Introduce write support module param -> default off
- Properly clean up if any debugfs files cannot be created

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: mjg59@srcf.ucam.org
CC: platform-driver-x86@vger.kernel.org
CC: linux-acpi@vger.kernel.org
CC: astarikovskiy@suse.de
Signed-off-by: Matthew Garrett <mjg@redhat.com>

Showing 2 changed files with 32 additions and 10 deletions Side-by-side Diff

drivers/acpi/Kconfig
... ... @@ -106,14 +106,19 @@
106 106  
107 107 config ACPI_EC_DEBUGFS
108 108 tristate "EC read/write access through /sys/kernel/debug/ec"
109   - default y
  109 + default n
110 110 help
111 111 Say N to disable Embedded Controller /sys/kernel/debug interface
112 112  
  113 + Be aware that using this interface can confuse your Embedded
  114 + Controller in a way that a normal reboot is not enough. You then
  115 + have to power of your system, and remove the laptop battery for
  116 + some seconds.
113 117 An Embedded Controller typically is available on laptops and reads
114 118 sensor values like battery state and temperature.
115   - The kernel access the EC through ACPI parsed code provided by BIOS
116   - tables.
  119 + The kernel accesses the EC through ACPI parsed code provided by BIOS
  120 + tables. This option allows to access the EC directly without ACPI
  121 + code being involved.
117 122 Thus this option is a debug option that helps to write ACPI drivers
118 123 and can be used to identify ACPI code or EC firmware bugs.
119 124  
drivers/acpi/ec_sys.c
... ... @@ -17,6 +17,11 @@
17 17 MODULE_DESCRIPTION("ACPI EC sysfs access driver");
18 18 MODULE_LICENSE("GPL");
19 19  
  20 +static bool write_support;
  21 +module_param(write_support, bool, 0644);
  22 +MODULE_PARM_DESC(write_support, "Dangerous, reboot and removal of battery may "
  23 + "be needed.");
  24 +
20 25 #define EC_SPACE_SIZE 256
21 26  
22 27 struct sysdev_class acpi_ec_sysdev_class = {
... ... @@ -102,6 +107,8 @@
102 107 {
103 108 struct dentry *dev_dir;
104 109 char name[64];
  110 + mode_t mode = 0400;
  111 +
105 112 if (ec_device_count == 0) {
106 113 acpi_ec_debugfs_dir = debugfs_create_dir("ec", NULL);
107 114 if (!acpi_ec_debugfs_dir)
108 115  
109 116  
... ... @@ -111,17 +118,27 @@
111 118 sprintf(name, "ec%u", ec_device_count);
112 119 dev_dir = debugfs_create_dir(name, acpi_ec_debugfs_dir);
113 120 if (!dev_dir) {
114   - if (ec_device_count == 0)
115   - debugfs_remove_recursive(acpi_ec_debugfs_dir);
116   - /* TBD: Proper cleanup for multiple ECs */
  121 + if (ec_device_count != 0)
  122 + goto error;
117 123 return -ENOMEM;
118 124 }
119 125  
120   - debugfs_create_x32("gpe", 0444, dev_dir, (u32 *)&first_ec->gpe);
121   - debugfs_create_bool("use_global_lock", 0444, dev_dir,
122   - (u32 *)&first_ec->global_lock);
123   - debugfs_create_file("io", 0666, dev_dir, ec, &acpi_ec_io_ops);
  126 + if (!debugfs_create_x32("gpe", 0444, dev_dir, (u32 *)&first_ec->gpe))
  127 + goto error;
  128 + if (!debugfs_create_bool("use_global_lock", 0444, dev_dir,
  129 + (u32 *)&first_ec->global_lock))
  130 + goto error;
  131 +
  132 + if (write_support)
  133 + mode = 0600;
  134 + if (!debugfs_create_file("io", mode, dev_dir, ec, &acpi_ec_io_ops))
  135 + goto error;
  136 +
124 137 return 0;
  138 +
  139 +error:
  140 + debugfs_remove_recursive(acpi_ec_debugfs_dir);
  141 + return -ENOMEM;
125 142 }
126 143  
127 144 static int __init acpi_ec_sys_init(void)