linux-kernel - Re: [ibm-acpi-devel] [PATCH 1/4] thinkpad_acpi: Add support for controlling charge thresholds

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Dec 2013 10:12:31 -0200
From:	Henrique de Moraes Holschuh <hmh@....eng.br>
To:	Julian Andres Klode <jak@...-linux.org>,
	Henrique de Moraes Holschuh <ibm-acpi@....eng.br>,
	Matthew Garrett <matthew.garrett@...ula.com>,
	"open list:THINKPAD ACPI EXT..." 
	<ibm-acpi-devel@...ts.sourceforge.net>,
	"open list:THINKPAD ACPI EXT..." 
	<platform-driver-x86@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [ibm-acpi-devel] [PATCH 1/4] thinkpad_acpi: Add support for
 controlling charge thresholds

On Tue, 31 Dec 2013, Julian Andres Klode wrote:
> We might be able to work around this by simple setting stop = start
> if a new write causes the stop threshold to be below the start
> threshold. But this also seems ugly.

It is the safest way, but the correct pseudo-code would be, assuiming
unsigned:

when someone changes start:

if (start > 99)
	start = 99;
get_current_stop_treshold(&stop);
if (start > stop)
	stop = start + 1;
set_thresholds(start, stop);


when someone changes stop:

if (stop == 0 || stop > 100)
	stop = 100;
get_current_start_threshold(&start);
if (start > stop)
	start = stop - 1;
set_thresholds(start, stop);

And write the stop threshold mod 100 at the low-level "send thresholds to
the firmware" routine.

> Writing the value 0 effectively means setting the stop threshold to 100%.
> The EC itself does not accept the value 100. I don't know why they choose
> to implement it this way. So the code should not accept 0 as an input
> value and replace an input of 100 with 0.

Yes.  This behaviour exists since start/stop thresholds were first exported
outside the EC, in the X30 or thereabouts.

> > For thinkpads, I believe the EC firmware changes the other threshold so that
> > the boundary condition stop > start is always valid.  But I never tried it
> > with a direct EC write to validate this.  If it becomes important, I can
> > check -- but I'd still prefer to enforce sanity at the driver level just in
> > case.  Don't tempt the gremilins, for they live at boundary conditions and
> > their sleep is light indeed.
> 
> It does not seem to do this here.

Then it is best if we enforce it in the driver.  I've crashed thinkpad ECs
before, *don't tempt the gremilins* indeed.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
--
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/