public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* samsung-galaxybook writes to a int via a u8*
@ 2025-12-28 11:55 Gianni Ceccarelli
  2025-12-28 20:16 ` Armin Wolf
  0 siblings, 1 reply; 4+ messages in thread
From: Gianni Ceccarelli @ 2025-12-28 11:55 UTC (permalink / raw)
  To: Joshua Grisham; +Cc: platform-driver-x86, linux-kernel

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/platform/x86/samsung-galaxybook.c#n450

`val->intval` is an int (see
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/power_supply.h#n228
), so writing to it via a `u8*` produces weird results, for example:

    $ cat /sys/class/power_supply/BAT1/charge_control_end_threshold
    78497792
    $ grep END_THRESHOLD /sys/class/power_supply/BAT1/uevent
    POWER_SUPPLY_CHARGE_CONTROL_END_THRESHOLD=-962691840

The least-significant byte of numbers values contains the expected
value:

    $ perl -E 'say 78497792 & 0xFF'
    0
    $ perl -E 'say -962691840 & 0xFF'
    0

even after changing the threshold:
    
    # echo 90 >
/sys/class/power_supply/BAT1/charge_control_end_threshold $ cat
/sys/class/power_supply/BAT1/charge_control_end_threshold 78497882
    $ grep END_THRESHOLD /sys/class/power_supply/BAT1/uevent
    POWER_SUPPLY_CHARGE_CONTROL_END_THRESHOLD=-966918822
    $ perl -E 'say 78497882 & 0xFF'
    90
    $ perl -E 'say -966918822 & 0xFF'
    90

I guess the code could be changed to:

    u8 byteval;
    err = charge_control_end_threshold_acpi_get(galaxybook, &byteval);
    if (err)
       return err;
    val->intval = byteval;

Hope this helps.

-- 
	Dakkar - <Mobilis in mobile>
	GPG public key fingerprint = A071 E618 DD2C 5901 9574
	                             6FE2 40EA 9883 7519 3F88
	                    key id = 0x75193F88


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

* Re: samsung-galaxybook writes to a int via a u8*
  2025-12-28 11:55 samsung-galaxybook writes to a int via a u8* Gianni Ceccarelli
@ 2025-12-28 20:16 ` Armin Wolf
  2025-12-28 21:06   ` Gianni Ceccarelli
  0 siblings, 1 reply; 4+ messages in thread
From: Armin Wolf @ 2025-12-28 20:16 UTC (permalink / raw)
  To: Gianni Ceccarelli, Joshua Grisham; +Cc: platform-driver-x86, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]

Am 28.12.25 um 12:55 schrieb Gianni Ceccarelli:

> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/platform/x86/samsung-galaxybook.c#n450
>
> `val->intval` is an int (see
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/linux/power_supply.h#n228
> ), so writing to it via a `u8*` produces weird results, for example:
>
>      $ cat /sys/class/power_supply/BAT1/charge_control_end_threshold
>      78497792
>      $ grep END_THRESHOLD /sys/class/power_supply/BAT1/uevent
>      POWER_SUPPLY_CHARGE_CONTROL_END_THRESHOLD=-962691840
>
> The least-significant byte of numbers values contains the expected
> value:
>
>      $ perl -E 'say 78497792 & 0xFF'
>      0
>      $ perl -E 'say -962691840 & 0xFF'
>      0
>
> even after changing the threshold:
>      
>      # echo 90 >
> /sys/class/power_supply/BAT1/charge_control_end_threshold $ cat
> /sys/class/power_supply/BAT1/charge_control_end_threshold 78497882
>      $ grep END_THRESHOLD /sys/class/power_supply/BAT1/uevent
>      POWER_SUPPLY_CHARGE_CONTROL_END_THRESHOLD=-966918822
>      $ perl -E 'say 78497882 & 0xFF'
>      90
>      $ perl -E 'say -966918822 & 0xFF'
>      90
>
> I guess the code could be changed to:
>
>      u8 byteval;
>      err = charge_control_end_threshold_acpi_get(galaxybook, &byteval);
>      if (err)
>         return err;
>      val->intval = byteval;
>
> Hope this helps.
>
Thanks for your report, this pointer cast indeed seems to be the root cause of the strange values
returned by charge_control_end_threshold. I attached a patch for you to test that implements you suggestion.

Thanks,
Armin Wolf

[-- Attachment #2: 0001-platform-x86-samsung-galaxybook-Fix-problematic-poin.patch --]
[-- Type: text/x-patch, Size: 1986 bytes --]

From cf438f40c8e954afadd7cb4a6c808bf55072561b Mon Sep 17 00:00:00 2001
From: Armin Wolf <W_Armin@gmx.de>
Date: Sun, 28 Dec 2025 21:02:18 +0100
Subject: [PATCH] platform/x86: samsung-galaxybook: Fix problematic pointer
 cast

A user reported that reading the charge threshold on his device
results in very strange values (like 78497792) being returned.
The reason for this seems to be the fact that the driver casts
the int pointer to an u8 pointer, leaving the last 3 bytes of
the destination uninitialized. Fix this by using a temporary
variable instead.

Fixes: 56f529ce4370 ("platform/x86: samsung-galaxybook: Add samsung-galaxybook driver")
Reported-by: Gianni Ceccarelli <dakkar@thenautilus.net>
Closes: https://lore.kernel.org/platform-driver-x86/20251228115556.14362d66@thenautilus.net/
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/samsung-galaxybook.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/samsung-galaxybook.c b/drivers/platform/x86/samsung-galaxybook.c
index 3c13e13d4885..755cb82bdb60 100644
--- a/drivers/platform/x86/samsung-galaxybook.c
+++ b/drivers/platform/x86/samsung-galaxybook.c
@@ -442,12 +442,13 @@ static int galaxybook_battery_ext_property_get(struct power_supply *psy,
 					       union power_supply_propval *val)
 {
 	struct samsung_galaxybook *galaxybook = ext_data;
+	u8 value;
 	int err;
 
 	if (psp != POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD)
 		return -EINVAL;
 
-	err = charge_control_end_threshold_acpi_get(galaxybook, (u8 *)&val->intval);
+	err = charge_control_end_threshold_acpi_get(galaxybook, &value);
 	if (err)
 		return err;
 
@@ -455,8 +456,10 @@ static int galaxybook_battery_ext_property_get(struct power_supply *psy,
 	 * device stores "no end threshold" as 0 instead of 100;
 	 * if device has 0, report 100
 	 */
-	if (val->intval == 0)
-		val->intval = 100;
+	if (value == 0)
+		value = 100;
+
+	val->intval = value;
 
 	return 0;
 }
-- 
2.52.0


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

* Re: samsung-galaxybook writes to a int via a u8*
  2025-12-28 20:16 ` Armin Wolf
@ 2025-12-28 21:06   ` Gianni Ceccarelli
  2025-12-28 21:50     ` Armin Wolf
  0 siblings, 1 reply; 4+ messages in thread
From: Gianni Ceccarelli @ 2025-12-28 21:06 UTC (permalink / raw)
  To: Armin Wolf; +Cc: Joshua Grisham, platform-driver-x86, linux-kernel

On 2025-12-28 Armin Wolf <W_Armin@gmx.de> wrote:
> Thanks for your report, this pointer cast indeed seems to be the root
> cause of the strange values returned by charge_control_end_threshold.
> I attached a patch for you to test that implements you suggestion.

I confirm that the patch works.

-- 
	Dakkar - <Mobilis in mobile>
	GPG public key fingerprint = A071 E618 DD2C 5901 9574
	                             6FE2 40EA 9883 7519 3F88
	                    key id = 0x75193F88


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

* Re: samsung-galaxybook writes to a int via a u8*
  2025-12-28 21:06   ` Gianni Ceccarelli
@ 2025-12-28 21:50     ` Armin Wolf
  0 siblings, 0 replies; 4+ messages in thread
From: Armin Wolf @ 2025-12-28 21:50 UTC (permalink / raw)
  To: Gianni Ceccarelli; +Cc: Joshua Grisham, platform-driver-x86, linux-kernel

Am 28.12.25 um 22:06 schrieb Gianni Ceccarelli:

> On 2025-12-28 Armin Wolf <W_Armin@gmx.de> wrote:
>> Thanks for your report, this pointer cast indeed seems to be the root
>> cause of the strange values returned by charge_control_end_threshold.
>> I attached a patch for you to test that implements you suggestion.
> I confirm that the patch works.
>
Thank you, i just submitted said patch for inclusion into the mainline kernel.

Thanks,
Armin Wolf


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

end of thread, other threads:[~2025-12-28 21:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-28 11:55 samsung-galaxybook writes to a int via a u8* Gianni Ceccarelli
2025-12-28 20:16 ` Armin Wolf
2025-12-28 21:06   ` Gianni Ceccarelli
2025-12-28 21:50     ` Armin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox