[PATCH 0/3] platform/x86: dell-laptop: Fix keyboard backlight and charge threshold handling

[PATCH 0/3] platform/x86: dell-laptop: Fix keyboard backlight and charge threshold handling

[Rosen Penev]

Three fixes for dell-laptop: keyboard backlight timeouts that overflow
during unit conversion were silently accepted, an unread keyboard state
was used at init when the SMBIOS call failed, and the charge end
threshold was parsed as unsigned into an int.

These patches were developed with the help of an AI coding assistant
(Claude Opus 5.5): it reviewed the drivers, found the issues and drafted
the fixes and changelogs. I reviewed every change and I am responsible
for them.

All patches build cleanly with W=1.

Tested on a Dell Precision M4800 running v7.2.15, using the equivalent
backports built as out-of-tree modules:
 - writing 4424214d / 18989323d to stop_timeout set 4m / 2m before
   patch 1 and is rejected with -EINVAL after it; 5m still works
 - charge_control_end_threshold rejects -1 and 101, accepts 85
The kbd_get_state() failure path of patch 2 was not triggered.


Rosen Penev (3):
  platform/x86: dell-laptop: Reject keyboard backlight timeouts that
    overflow
  platform/x86: dell-laptop: Don't use an unread keyboard state at init
  platform/x86: dell-laptop: Parse the charge end threshold as an int

 drivers/platform/x86/dell/dell-laptop.c | 30 ++++++++++++++++++-------
 1 file changed, 22 insertions(+), 8 deletions(-)


base-commit: 2800868f63227c8237d70c41e70d441c919620d6
-- 
2.56.0

[PATCH 1/3] platform/x86: dell-laptop: Reject keyboard backlight timeouts that overflow

[Rosen Penev]

kbd_led_timeout_store() converts a timeout that is too large for its
unit to seconds with plain int multiplication. A large enough value
wraps, and if the wrapped number happens to be a valid timeout it is
accepted. On a Precision M4800, writing "4424214d" (over 12000 years)
to stop_timeout sets the timeout to 4 minutes, and "18989323d" sets it
to 2 minutes.

Convert with check_mul_overflow() and return -EINVAL on overflow.

Fixes: 02b2aaaa57ab ("platform: x86: dell-laptop: Add support for keyboard backlight")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/platform/x86/dell/dell-laptop.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index 89e85c7f7132..bee347e1d60c 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -25,6 +25,7 @@
 #include <linux/sysfs.h>
 #include <linux/acpi.h>
 #include <linux/mm.h>
+#include <linux/overflow.h>
 #include <linux/i8042.h>
 #include <linux/debugfs.h>
 #include <linux/seq_file.h>
@@ -1554,6 +1555,7 @@ static ssize_t kbd_led_timeout_store(struct device *dev,
 	struct kbd_state state;
 	bool convert;
 	int value;
+	int mult;
 	int ret;
 	char ch;
 	u8 unit;
@@ -1602,16 +1604,23 @@ static ssize_t kbd_led_timeout_store(struct device *dev,
 		/* Convert value from current units to seconds */
 		switch (unit) {
 		case KBD_TIMEOUT_DAYS:
-			value *= 24;
-			fallthrough;
+			mult = 24 * 60 * 60;
+			break;
 		case KBD_TIMEOUT_HOURS:
-			value *= 60;
-			fallthrough;
+			mult = 60 * 60;
+			break;
 		case KBD_TIMEOUT_MINUTES:
-			value *= 60;
-			unit = KBD_TIMEOUT_SECONDS;
+			mult = 60;
+			break;
+		default:
+			mult = 1;
+			break;
 		}
 
+		if (check_mul_overflow(value, mult, &value))
+			return -EINVAL;
+		unit = KBD_TIMEOUT_SECONDS;
+
 		if (quirks && quirks->needs_kbd_timeouts) {
 			for (i = 0; quirks->kbd_timeouts[i] != -1; i++) {
 				if (value <= quirks->kbd_timeouts[i]) {
-- 
2.56.0

[PATCH 2/3] platform/x86: dell-laptop: Don't use an unread keyboard state at init

[Rosen Penev]

kbd_init_info() ignores the return value of kbd_get_state(). If the
call fails, the uninitialized state on the stack is used to set
kbd_previous_level and kbd_previous_mode_bit, which are later sent to
the BIOS when triggers are enabled or the backlight is turned back on.

Return the error instead. kbd_get_info() has already filled in
kbd_info at that point, so clear it as well: level control through
kbd_get_state() cannot work, and kbd_init() then still registers the
LED if at least two keyboard backlight tokens are available, which
works without it.

Fixes: 02b2aaaa57ab ("platform: x86: dell-laptop: Add support for keyboard backlight")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/platform/x86/dell/dell-laptop.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index bee347e1d60c..171728d63c81 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -1455,7 +1455,12 @@ static inline int kbd_init_info(void)
 	    dell_smbios_find_token(KBD_LED_AC_TOKEN))
 		kbd_timeout_ac_supported = true;
 
-	kbd_get_state(&state);
+	ret = kbd_get_state(&state);
+	if (ret) {
+		/* Leave level control to the SMBIOS tokens, if there are any */
+		memset(&kbd_info, 0, sizeof(kbd_info));
+		return ret;
+	}
 
 	/* NOTE: timeout value is stored in 6 bits so max value is 63 */
 	if (kbd_info.seconds > 63)
-- 
2.56.0

[PATCH 3/3] platform/x86: dell-laptop: Parse the charge end threshold as an int

[Rosen Penev]

charge_control_end_threshold_store() passes a pointer to an int to
kstrtouint(), which only builds because the kernel disables
-Wpointer-sign, and makes the following "end < 0" check dead code.
Use kstrtoint() like charge_control_start_threshold_store(). Values
outside 0 to 100 are rejected either way.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/platform/x86/dell/dell-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index 171728d63c81..2ebfef8e5bbe 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -2385,7 +2385,7 @@ static ssize_t charge_control_end_threshold_store(struct device *dev,
 {
 	int ret, end;
 
-	ret = kstrtouint(buf, 10, &end);
+	ret = kstrtoint(buf, 10, &end);
 	if (ret)
 		return ret;
 	if (end < 0 || end > 100)
-- 
2.56.0