toshiba-wmi: Fix loading the driver on non Toshiba laptops

PATCH In review Azael Avalos 2 participants 6 messages Aug 25, 2016 – Oct 21, 2016

Subsystems (2)

x86 platform drivers Toshiba WMI hotkeys driver

lore

Share & tools

lore

Azael Avalos

author maintainer

2016-08-25 18:51 UTC #0 lore raw

PATCH 1/1 toshiba-wmi: Fix loading the driver on non Toshiba laptops

Bug 150611 uncovered that the WMI ID used by the toshiba-wmi driver is not Toshiba specific, and as such, the driver was being loaded on non Toshiba laptops too.

This patch adds a DMI matching list checking for TOSHIBA as the vendor, refusing to load if it is not.

Also the WMI GUID was renamed, dropping the TOSHIBA_ prefix, to better reflect that such GUID is not a Toshiba specific one.

Cc <[email protected]> # 4.4+

Signed-off-by Azael Avalos <[email protected]>

---
Hi Darren,

I was waiting on input from the bug above, but haven't received an answer (as of yet), so I decided to send this to the mailing list for feedback as to whether this is the correct approach for this issue.

 drivers/platform/x86/toshiba-wmi.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

drivers/platform/x86/toshiba-wmi.c +19 -7

index feac457..0c92887 100644
@@ -24,14 +24,15 @@
 #include <linux/acpi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/dmi.h>
 
 MODULE_AUTHOR("Azael Avalos");
 MODULE_DESCRIPTION("Toshiba WMI Hotkey Driver");
 MODULE_LICENSE("GPL");
 
-#define TOSHIBA_WMI_EVENT_GUID	"59142400-C6A3-40FA-BADB-8A2652834100"
+#define WMI_EVENT_GUID	"59142400-C6A3-40FA-BADB-8A2652834100"
 
-MODULE_ALIAS("wmi:"TOSHIBA_WMI_EVENT_GUID);
+MODULE_ALIAS("wmi:"WMI_EVENT_GUID);
 
 static struct input_dev *toshiba_wmi_input_dev;

@@ -63,6 +64,16 @@ static void toshiba_wmi_notify(u32 value, void *context)
 	kfree(response.pointer);
 }
 
+static struct dmi_system_id toshiba_wmi_dmi_table[] __initdata = {
+	{
+		.ident = "Toshiba laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+		},
+	},
+	{}
+};
+
 static int __init toshiba_wmi_input_setup(void)
 {
 	acpi_status status;

@@ -81,7 +92,7 @@ static int __init toshiba_wmi_input_setup(void)
 	if (err)
 		goto err_free_dev;
 
-	status = wmi_install_notify_handler(TOSHIBA_WMI_EVENT_GUID,
+	status = wmi_install_notify_handler(WMI_EVENT_GUID,
 					    toshiba_wmi_notify, NULL);
 	if (ACPI_FAILURE(status)) {
 		err = -EIO;

@@ -95,7 +106,7 @@ static int __init toshiba_wmi_input_setup(void)
 	return 0;
 
  err_remove_notifier:
-	wmi_remove_notify_handler(TOSHIBA_WMI_EVENT_GUID);
+	wmi_remove_notify_handler(WMI_EVENT_GUID);
  err_free_keymap:
 	sparse_keymap_free(toshiba_wmi_input_dev);
  err_free_dev:

@@ -105,7 +116,7 @@ static int __init toshiba_wmi_input_setup(void)
 
 static void toshiba_wmi_input_destroy(void)
 {
-	wmi_remove_notify_handler(TOSHIBA_WMI_EVENT_GUID);
+	wmi_remove_notify_handler(WMI_EVENT_GUID);
 	sparse_keymap_free(toshiba_wmi_input_dev);
 	input_unregister_device(toshiba_wmi_input_dev);
 }

@@ -114,7 +125,8 @@ static int __init toshiba_wmi_init(void)
 {
 	int ret;
 
-	if (!wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
+	if (!wmi_has_guid(WMI_EVENT_GUID) ||
+	    !dmi_check_system(toshiba_wmi_dmi_table))
 		return -ENODEV;
 
 	ret = toshiba_wmi_input_setup();

@@ -130,7 +142,7 @@ static int __init toshiba_wmi_init(void)
 
 static void __exit toshiba_wmi_exit(void)
 {
-	if (wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
+	if (wmi_has_guid(WMI_EVENT_GUID))
 		toshiba_wmi_input_destroy();
 }

Signature · 1 line

2.9.3

Darren Hart

2016-08-28 17:00 UTC #1 lore raw

Quoted 12 lines · Azael Avalos

Bug 150611 uncovered that the WMI ID used by the toshiba-wmi driver is not Toshiba specific, and as such, the driver was being loaded on non Toshiba laptops too.
This patch adds a DMI matching list checking for TOSHIBA as the vendor, refusing to load if it is not.
Also the WMI GUID was renamed, dropping the TOSHIBA_ prefix, to better reflect that such GUID is not a Toshiba specific one.
Cc <[email protected]> # 4.4+
Signed-off-by Azael Avalos <[email protected]>

Quoted 7 lines

---
Hi Darren,
I was waiting on input from the bug above, but haven't received an answer (as of yet), so I decided to send this to the mailing list for feedback as to whether this is the correct approach for this issue.

Thanks. Let's see if Carlos (wmi.c author) has an opinion...

Carlos?

Quoted 93 lines

 drivers/platform/x86/toshiba-wmi.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

drivers/platform/x86/toshiba-wmi.c +19 -7

index feac457..0c92887 100644
@@ -24,14 +24,15 @@
 #include <linux/acpi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/dmi.h>
 
 MODULE_AUTHOR("Azael Avalos");
 MODULE_DESCRIPTION("Toshiba WMI Hotkey Driver");
 MODULE_LICENSE("GPL");
 
-#define TOSHIBA_WMI_EVENT_GUID	"59142400-C6A3-40FA-BADB-8A2652834100"
+#define WMI_EVENT_GUID	"59142400-C6A3-40FA-BADB-8A2652834100"
 
-MODULE_ALIAS("wmi:"TOSHIBA_WMI_EVENT_GUID);
+MODULE_ALIAS("wmi:"WMI_EVENT_GUID);
 
 static struct input_dev *toshiba_wmi_input_dev;

@@ -63,6 +64,16 @@ static void toshiba_wmi_notify(u32 value, void *context)
 	kfree(response.pointer);
 }
 
+static struct dmi_system_id toshiba_wmi_dmi_table[] __initdata = {
+	{
+		.ident = "Toshiba laptop",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+		},
+	},
+	{}
+};
+
 static int __init toshiba_wmi_input_setup(void)
 {
 	acpi_status status;

@@ -81,7 +92,7 @@ static int __init toshiba_wmi_input_setup(void)
 	if (err)
 		goto err_free_dev;
 
-	status = wmi_install_notify_handler(TOSHIBA_WMI_EVENT_GUID,
+	status = wmi_install_notify_handler(WMI_EVENT_GUID,
 					    toshiba_wmi_notify, NULL);
 	if (ACPI_FAILURE(status)) {
 		err = -EIO;

@@ -95,7 +106,7 @@ static int __init toshiba_wmi_input_setup(void)
 	return 0;
 
  err_remove_notifier:
-	wmi_remove_notify_handler(TOSHIBA_WMI_EVENT_GUID);
+	wmi_remove_notify_handler(WMI_EVENT_GUID);
  err_free_keymap:
 	sparse_keymap_free(toshiba_wmi_input_dev);
  err_free_dev:

@@ -105,7 +116,7 @@ static int __init toshiba_wmi_input_setup(void)
 
 static void toshiba_wmi_input_destroy(void)
 {
-	wmi_remove_notify_handler(TOSHIBA_WMI_EVENT_GUID);
+	wmi_remove_notify_handler(WMI_EVENT_GUID);
 	sparse_keymap_free(toshiba_wmi_input_dev);
 	input_unregister_device(toshiba_wmi_input_dev);
 }

@@ -114,7 +125,8 @@ static int __init toshiba_wmi_init(void)
 {
 	int ret;
 
-	if (!wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
+	if (!wmi_has_guid(WMI_EVENT_GUID) ||
+	    !dmi_check_system(toshiba_wmi_dmi_table))
 		return -ENODEV;
 
 	ret = toshiba_wmi_input_setup();

@@ -130,7 +142,7 @@ static int __init toshiba_wmi_init(void)
 
 static void __exit toshiba_wmi_exit(void)
 {
-	if (wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
+	if (wmi_has_guid(WMI_EVENT_GUID))
 		toshiba_wmi_input_destroy();
 }

Signature · 1 line

2.9.3

Signature · 2 lines

Darren Hart
Intel Open Source Technology Center

Azael Avalos

author maintainer

2016-10-12 16:26 UTC #2 lore raw

*ping*

2016-08-28 11:00 GMT-06:00 Darren Hart <[email protected]>:

Quoted 16 lines · Azael Avalos

Quoted 15 lines
Bug 150611 uncovered that the WMI ID used by the toshiba-wmi driver is not Toshiba specific, and as such, the driver was being loaded on non Toshiba laptops too.
This patch adds a DMI matching list checking for TOSHIBA as the vendor, refusing to load if it is not.
Also the WMI GUID was renamed, dropping the TOSHIBA_ prefix, to better reflect that such GUID is not a Toshiba specific one.
Cc <[email protected]> # 4.4+
Signed-off-by Azael Avalos <[email protected]>
---
Hi Darren,

Quoted 8 lines

Quoted 4 lines
I was waiting on input from the bug above, but haven't received an answer (as of yet), so I decided to send this to the mailing list for feedback as to whether this is the correct approach for this issue.
Thanks. Let's see if Carlos (wmi.c author) has an opinion...
Carlos?

Any input on this?

Quoted 98 lines

Quoted 91 lines


 drivers/platform/x86/toshiba-wmi.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

drivers/platform/x86/toshiba-wmi.c +19 -8

index feac457..0c92887 100644
@@ -24,14 +24,15 @@
 #include <linux/acpi.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
+#include <linux/dmi.h>
 
 MODULE_AUTHOR("Azael Avalos");
 MODULE_DESCRIPTION("Toshiba WMI Hotkey Driver");
 MODULE_LICENSE("GPL");
 
-#define TOSHIBA_WMI_EVENT_GUID       "59142400-C6A3-40FA-BADB-8A2652834100"
+#define WMI_EVENT_GUID       "59142400-C6A3-40FA-BADB-8A2652834100"
 
-MODULE_ALIAS("wmi:"TOSHIBA_WMI_EVENT_GUID);
+MODULE_ALIAS("wmi:"WMI_EVENT_GUID);
 
 static struct input_dev *toshiba_wmi_input_dev;

@@ -63,6 +64,16 @@ static void toshiba_wmi_notify(u32 value, void *context)
      kfree(response.pointer);
 }
 
+static struct dmi_system_id toshiba_wmi_dmi_table[] __initdata = {
+     {
+             .ident = "Toshiba laptop",
+             .matches = {
+                     DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+             },
+     },
+     {}
+};
+
 static int __init toshiba_wmi_input_setup(void)
 {
      acpi_status status;

@@ -81,7 +92,7 @@ static int __init toshiba_wmi_input_setup(void)
      if (err)
              goto err_free_dev;
 
-     status = wmi_install_notify_handler(TOSHIBA_WMI_EVENT_GUID,
+     status = wmi_install_notify_handler(WMI_EVENT_GUID,
                                          toshiba_wmi_notify, NULL);
      if (ACPI_FAILURE(status)) {
              err = -EIO;

@@ -95,7 +106,7 @@ static int __init toshiba_wmi_input_setup(void)
      return 0;
 
  err_remove_notifier:
-     wmi_remove_notify_handler(TOSHIBA_WMI_EVENT_GUID);
+     wmi_remove_notify_handler(WMI_EVENT_GUID);
  err_free_keymap:
      sparse_keymap_free(toshiba_wmi_input_dev);
  err_free_dev:

@@ -105,7 +116,7 @@ static int __init toshiba_wmi_input_setup(void)
 
 static void toshiba_wmi_input_destroy(void)
 {
-     wmi_remove_notify_handler(TOSHIBA_WMI_EVENT_GUID);
+     wmi_remove_notify_handler(WMI_EVENT_GUID);
      sparse_keymap_free(toshiba_wmi_input_dev);
      input_unregister_device(toshiba_wmi_input_dev);
 }

@@ -114,7 +125,8 @@ static int __init toshiba_wmi_init(void)
 {
      int ret;
 
-     if (!wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
+     if (!wmi_has_guid(WMI_EVENT_GUID) ||
+         !dmi_check_system(toshiba_wmi_dmi_table))
              return -ENODEV;
 
      ret = toshiba_wmi_input_setup();

@@ -130,7 +142,7 @@ static int __init toshiba_wmi_init(void)
 
 static void __exit toshiba_wmi_exit(void)
 {
-     if (wmi_has_guid(TOSHIBA_WMI_EVENT_GUID))
+     if (wmi_has_guid(WMI_EVENT_GUID))
              toshiba_wmi_input_destroy();
 }
 
--

Quoted 3 lines

2.9.3

--
Darren Hart
Intel Open Source Technology Center

Signature · 1 line

-- El mundo apesta y vosotros apestais tambien --

Darren Hart

2016-10-19 20:26 UTC #3 lore raw

Quoted 24 lines · Azael Avalos

*ping*
2016-08-28 11:00 GMT-06:00 Darren Hart <[email protected]>:
Quoted 21 lines · Azael Avalos
Quoted 19 lines
Bug 150611 uncovered that the WMI ID used by the toshiba-wmi driver is not Toshiba specific, and as such, the driver was being loaded on non Toshiba laptops too.
This patch adds a DMI matching list checking for TOSHIBA as the vendor, refusing to load if it is not.
Also the WMI GUID was renamed, dropping the TOSHIBA_ prefix, to better reflect that such GUID is not a Toshiba specific one.
Cc <[email protected]> # 4.4+
Signed-off-by Azael Avalos <[email protected]>
---
Hi Darren,
I was waiting on input from the bug above, but haven't received an answer (as of yet), so I decided to send this to the mailing list for feedback as to whether this is the correct approach for this issue.

Quoted 5 lines

Quoted 3 lines
Thanks. Let's see if Carlos (wmi.c author) has an opinion...
Carlos?
Any input on this?

Looks like a sane workaround to me. Queued to fixes.

Want to going to stable?

Signature · 2 lines

Darren Hart
Intel Open Source Technology Center

Azael Avalos

author maintainer

2016-10-19 23:27 UTC #4 lore raw

Hi Darren,

2016-10-19 14:26 GMT-06:00 Darren Hart <[email protected]>:

Quoted 25 lines · Azael Avalos

Quoted 24 lines
*ping*
2016-08-28 11:00 GMT-06:00 Darren Hart <[email protected]>:
Quoted 21 lines · Azael Avalos
Quoted 19 lines
Bug 150611 uncovered that the WMI ID used by the toshiba-wmi driver is not Toshiba specific, and as such, the driver was being loaded on non Toshiba laptops too.
This patch adds a DMI matching list checking for TOSHIBA as the vendor, refusing to load if it is not.
Also the WMI GUID was renamed, dropping the TOSHIBA_ prefix, to better reflect that such GUID is not a Toshiba specific one.
Cc <[email protected]> # 4.4+
Signed-off-by Azael Avalos <[email protected]>
---
Hi Darren,
I was waiting on input from the bug above, but haven't received an answer (as of yet), so I decided to send this to the mailing list for feedback as to whether this is the correct approach for this issue.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Thanks. Let's see if Carlos (wmi.c author) has an opinion...
Carlos?
Any input on this?
Looks like a sane workaround to me. Queued to fixes.

Thanks.

Quoted 2 lines

Want to going to stable?

If possible, yes, as this issue affects other laptop manufacturers, we may never know if someone might try to load an older kernel on an affected laptop and end up with the toshiba_wmi loaded instead of the actual laptop support module, however, I'm not sure if this is a "big" issue to bother stable, if not, simply drop the cc to stable for me please.

Quoted 3 lines

--
Darren Hart
Intel Open Source Technology Center

Cheers
Azael

Signature · 1 line

-- El mundo apesta y vosotros apestais tambien --

Darren Hart

2016-10-21 05:01 UTC #5 lore raw

Quoted 6 lines · Azael Avalos

Hi Darren,
2016-10-19 14:26 GMT-06:00 Darren Hart <[email protected]>:
Quoted 1 line

Quoted 8 lines

Quoted 1 line
Want to going to stable?
If possible, yes, as this issue affects other laptop manufacturers, we may never know if someone might try to load an older kernel on an affected laptop and end up with the toshiba_wmi loaded instead of the actual laptop support module, however, I'm not sure if this is a "big" issue to bother stable, if not, simply drop the cc to stable for me please.

My addled brain somehow skipped right over the Cc stable in your original patch, I saw it later :-) Kept it.

Signature · 2 lines

Darren Hart
Intel Open Source Technology Center