All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] HID: lenovo: Fix false positive errors on setting tp10ubkbd LEDs
@ 2021-02-15 20:38 Hans de Goede
  2021-02-15 20:38 ` [PATCH 2/5] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Hans de Goede @ 2021-02-15 20:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: Hans de Goede, linux-input

Fix the error check in lenovo_led_set_tp10ubkbd(), on success
hid_hw_raw_request() returns the number of bytes send. So we should
check for (ret != 3) rather then for (ret != 0).

Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index c6c8e20f3e8d..69d709439676 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -75,7 +75,7 @@ static void lenovo_led_set_tp10ubkbd(struct hid_device *hdev, u8 led_code,
 	data->led_report[2] = value ? TP10UBKBD_LED_ON : TP10UBKBD_LED_OFF;
 	ret = hid_hw_raw_request(hdev, data->led_report[0], data->led_report, 3,
 				 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
-	if (ret)
+	if (ret != 3)
 		hid_err(hdev, "Set LED output report error: %d\n", ret);
 
 	mutex_unlock(&data->led_report_mutex);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/5] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness
  2021-02-15 20:38 [PATCH 1/5] HID: lenovo: Fix false positive errors on setting tp10ubkbd LEDs Hans de Goede
@ 2021-02-15 20:38 ` Hans de Goede
  2021-02-15 20:38 ` [PATCH 3/5] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event() Hans de Goede
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-02-15 20:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: Hans de Goede, linux-input

The lenovo_led_brightness_set function may sleep, so we should have the
the led_class_dev's brightness_set_blocking callback point to it, rather
then the regular brightness_set callback.

When toggle through sysfs this is not a problem, but the brightness_set
callback may be called from atomic context when using LED-triggers.

Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 69d709439676..9b80b1685b53 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -777,7 +777,7 @@ static enum led_brightness lenovo_led_brightness_get(
 				: LED_OFF;
 }
 
-static void lenovo_led_brightness_set(struct led_classdev *led_cdev,
+static int lenovo_led_brightness_set(struct led_classdev *led_cdev,
 			enum led_brightness value)
 {
 	struct device *dev = led_cdev->dev->parent;
@@ -802,6 +802,8 @@ static void lenovo_led_brightness_set(struct led_classdev *led_cdev,
 		lenovo_led_set_tp10ubkbd(hdev, tp10ubkbd_led[led_nr], value);
 		break;
 	}
+
+	return 0;
 }
 
 static int lenovo_register_leds(struct hid_device *hdev)
@@ -822,7 +824,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 
 	data->led_mute.name = name_mute;
 	data->led_mute.brightness_get = lenovo_led_brightness_get;
-	data->led_mute.brightness_set = lenovo_led_brightness_set;
+	data->led_mute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_mute.dev = &hdev->dev;
 	ret = led_classdev_register(&hdev->dev, &data->led_mute);
 	if (ret < 0)
@@ -830,7 +832,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 
 	data->led_micmute.name = name_micm;
 	data->led_micmute.brightness_get = lenovo_led_brightness_get;
-	data->led_micmute.brightness_set = lenovo_led_brightness_set;
+	data->led_micmute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_micmute.dev = &hdev->dev;
 	ret = led_classdev_register(&hdev->dev, &data->led_micmute);
 	if (ret < 0) {
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/5] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event()
  2021-02-15 20:38 [PATCH 1/5] HID: lenovo: Fix false positive errors on setting tp10ubkbd LEDs Hans de Goede
  2021-02-15 20:38 ` [PATCH 2/5] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
@ 2021-02-15 20:38 ` Hans de Goede
  2021-02-15 20:38 ` [PATCH 4/5] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE Hans de Goede
  2021-02-15 20:38 ` [PATCH 5/5] HID: lenovo: Set default_trigger-s for the mute and micmute LEDs Hans de Goede
  3 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-02-15 20:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: Hans de Goede, linux-input

The HID lenovo probe function only attaches drvdata to one of the
USB interfaces, but lenovo_event() will get called for all USB interfaces
to which hid-lenovo is bound.

This allows a malicious device to fake being a device handled by
hid-lenovo, which generates events for which lenovo_event() has
special handling (and thus dereferences hid_get_drvdata()) on another
interface triggering a NULL pointer exception.

Add a check for hid_get_drvdata() returning NULL, avoiding this
possible NULL pointer exception.

Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 9b80b1685b53..50ae770a4fb1 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -498,6 +498,9 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
 static int lenovo_event(struct hid_device *hdev, struct hid_field *field,
 		struct hid_usage *usage, __s32 value)
 {
+	if (!hid_get_drvdata(hdev))
+		return 0;
+
 	switch (hdev->product) {
 	case USB_DEVICE_ID_LENOVO_CUSBKBD:
 	case USB_DEVICE_ID_LENOVO_CBTKBD:
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 4/5] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE
  2021-02-15 20:38 [PATCH 1/5] HID: lenovo: Fix false positive errors on setting tp10ubkbd LEDs Hans de Goede
  2021-02-15 20:38 ` [PATCH 2/5] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
  2021-02-15 20:38 ` [PATCH 3/5] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event() Hans de Goede
@ 2021-02-15 20:38 ` Hans de Goede
  2021-02-15 20:38 ` [PATCH 5/5] HID: lenovo: Set default_trigger-s for the mute and micmute LEDs Hans de Goede
  3 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-02-15 20:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: Hans de Goede, linux-input

Mapping the mic-mute button to KEY_MICMUTE is technically correct but
KEY_MICMUTE translates to a scancode of 256 (248 + 8) under X,
which does not fit in 8 bits, so it does not work.

Because of this userspace is expecting KEY_F20 instead,
theoretically KEY_MICMUTE should work under Wayland but even
there it does not work, because the desktop-environment is
listening only for KEY_F20 and not for KEY_MICMUTE.

Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 50ae770a4fb1..045c06ba0ab8 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -33,6 +33,9 @@
 
 #include "hid-ids.h"
 
+/* Userspace expects F20 for mic-mute KEY_MICMUTE does not work */
+#define LENOVO_KEY_MICMUTE KEY_F20
+
 struct lenovo_drvdata {
 	u8 led_report[3]; /* Must be first for proper alignment */
 	int led_state;
@@ -126,7 +129,7 @@ static int lenovo_input_mapping_tpkbd(struct hid_device *hdev,
 	if (usage->hid == (HID_UP_BUTTON | 0x0010)) {
 		/* This sub-device contains trackpoint, mark it */
 		hid_set_drvdata(hdev, (void *)1);
-		map_key_clear(KEY_MICMUTE);
+		map_key_clear(LENOVO_KEY_MICMUTE);
 		return 1;
 	}
 	return 0;
@@ -141,7 +144,7 @@ static int lenovo_input_mapping_cptkbd(struct hid_device *hdev,
 	    (usage->hid & HID_USAGE_PAGE) == HID_UP_LNVENDOR) {
 		switch (usage->hid & HID_USAGE) {
 		case 0x00f1: /* Fn-F4: Mic mute */
-			map_key_clear(KEY_MICMUTE);
+			map_key_clear(LENOVO_KEY_MICMUTE);
 			return 1;
 		case 0x00f2: /* Fn-F5: Brightness down */
 			map_key_clear(KEY_BRIGHTNESSDOWN);
@@ -231,7 +234,7 @@ static int lenovo_input_mapping_tp10_ultrabook_kbd(struct hid_device *hdev,
 			map_key_clear(KEY_FN_ESC);
 			return 1;
 		case 9: /* Fn-F4: Mic mute */
-			map_key_clear(KEY_MICMUTE);
+			map_key_clear(LENOVO_KEY_MICMUTE);
 			return 1;
 		case 10: /* Fn-F7: Control panel */
 			map_key_clear(KEY_CONFIG);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 5/5] HID: lenovo: Set default_trigger-s for the mute and micmute LEDs
  2021-02-15 20:38 [PATCH 1/5] HID: lenovo: Fix false positive errors on setting tp10ubkbd LEDs Hans de Goede
                   ` (2 preceding siblings ...)
  2021-02-15 20:38 ` [PATCH 4/5] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE Hans de Goede
@ 2021-02-15 20:38 ` Hans de Goede
  3 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-02-15 20:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: Hans de Goede, linux-input

The mute and mic-mute LEDs should be automatically turned on/off based
on the audio-cards mixer settings.

Add the standardized default-trigger names for this, so that the alsa
code can turn the LEDs on/off as appropriate (on supported audo cards).

This brings the mute/mic-mute LED support inline with the thinkpad_acpi
support for the same LEDs in keyboards directly connected to the
laptop's embedded-controller.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-lenovo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 045c06ba0ab8..c82e328f5310 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -829,6 +829,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 	snprintf(name_micm, name_sz, "%s:amber:micmute", dev_name(&hdev->dev));
 
 	data->led_mute.name = name_mute;
+	data->led_mute.default_trigger = "audio-mute";
 	data->led_mute.brightness_get = lenovo_led_brightness_get;
 	data->led_mute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_mute.dev = &hdev->dev;
@@ -837,6 +838,7 @@ static int lenovo_register_leds(struct hid_device *hdev)
 		return ret;
 
 	data->led_micmute.name = name_micm;
+	data->led_micmute.default_trigger = "audio-micmute";
 	data->led_micmute.brightness_get = lenovo_led_brightness_get;
 	data->led_micmute.brightness_set_blocking = lenovo_led_brightness_set;
 	data->led_micmute.dev = &hdev->dev;
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-02-15 20:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-15 20:38 [PATCH 1/5] HID: lenovo: Fix false positive errors on setting tp10ubkbd LEDs Hans de Goede
2021-02-15 20:38 ` [PATCH 2/5] HID: lenovo: Use brightness_set_blocking callback for setting LEDs brightness Hans de Goede
2021-02-15 20:38 ` [PATCH 3/5] HID: lenovo: Check hid_get_drvdata() returns non NULL in lenovo_event() Hans de Goede
2021-02-15 20:38 ` [PATCH 4/5] HID: lenovo: Map mic-mute button to KEY_F20 instead of KEY_MICMUTE Hans de Goede
2021-02-15 20:38 ` [PATCH 5/5] HID: lenovo: Set default_trigger-s for the mute and micmute LEDs Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.