linux-kernel - Returning ACPI_BATTERY_VALUE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 16 Oct 2010 15:13:22 +0100
From:	Sitsofe Wheeler <sitsofe@...oo.com>
To:	Alexey Starikovskiy <astarikovskiy@...e.de>,
	Len Brown <lenb@...nel.org>, Zhang Rui <rui.zhang@...el.com>
Cc:	Matthew Garrett <mjg@...hat.com>,
	David Zeuthen <davidz@...hat.com>,
	Richard Hughes <richard@...hsie.com>,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Returning ACPI_BATTERY_VALUE_UNKNOWN to userspace

Hi,

I have an EeePC 900 with a battery/BIOS that does not report the rate at
which it charges/discharges. When I look at
/proc/acpi/battery/BAT0/state this is what is reported:

present:                 yes
capacity state:          ok
charging state:          charging
present rate:            unknown
remaining capacity:      3120 mAh
present voltage:         7889 mV

However looking at /sys/class/power_supply/BAT0/current_now reports:

-1000

Why -1000? I think it's because it's -1 * 1000 == -1000! In
drivers/acpi/battery.c, ACPI_BATTERY_VALUE_UNKNOWN is defined as being
0xFFFFFFFF. As rate_now is a signed int variable when it is assigned
ACPI_BATTERY_VALUE_UNKNOWN its value is -1. However, before the value is
returned via sysfs it is multiplied by 1000:
http://lxr.linux.no/linux+v2.6.35.7/drivers/acpi/battery.c#L222
(http://lxr.linux.no/linux+v2.6.35.7/drivers/acpi/battery.c#L524 shows
that acpi_battery_get_property will be called via sysfs).

If the above is a correct interpretation, this behaviour was introduced
when sysfs battery support was added in commit
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d7380965752505951668e85de59c128d1d6fd21f
so it has effectively been always been this way.

However, looking at the code for the userspace power reporting tool
upower shows that it is not expecting to test against -1000 - it is
trying to test against 0xffff:
http://cgit.freedesktop.org/DeviceKit/upower/tree/src/linux/up-device-supply.c?id=UPOWER_0_9_6#n583
. Unfortunately, it's not clear that testing 0xffff is ever the right
thing to do. I wrote the following test program:

#include <stdio.h>
#include <math.h>
int main(void) {
	double minusone = -1;
	double sysfs = -1000;
	double hex_kernel = (int) 0xffffffff;
	double hex_tested = 0xffff;
	double energy_rate = fabs(sysfs / 1000000.0);
	double energy_rate_minusone = fabs(minusone / 1000000.0);
	printf("%f %f %f %f %f %f\n", minusone, sysfs, hex_kernel, hex_tested, energy_rate, energy_rate_minusone);
	return 0;
}

Which output the following:

-1.000000 -1000.000000 -1.000000 65535.000000 0.001000 0.000001

Given that at least upower (which is already deployed) will need to be
changed, I'm unsure as to where fixes for this should go. Was it really
the intent for userspace to test for -1000 instead of -1 to determine
an unknown rate?

-- 
Sitsofe | http://sucs.org/~sits/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/