Picon

[PATCH 3/3] ACPI: thinkpad-acpi: fix LED handling on older ThinkPads

The less tested codepaths for LED handling, used on ThinkPads 570, 600e/x,
770e, 770x, A21e, A2xm/p, T20-22, X20 and maybe a few others, would write
data to kernel memory it had no business touching, for leds number 3 and
above.  If one is lucky, that illegal write would cause an OOPS, but
chances are it would silently corrupt a byte.

The problem was introduced in commit af116101, "ACPI: thinkpad-acpi: add
sysfs led class support to thinkpad leds (v3.2)".

Fix the bug by refactoring the entire code to be far more obvious on what
it wants to do.  Also do some defensive "constification".

Issue reported by Karol Lewandowski <lmctlx <at> gmail.com> (he's an lucky guy
and got an OOPS instead of silent corruption :-) ).

Root cause of the OOPS identified by Adrian Bunk <bunk <at> kernel.org>.
Thanks, Adrian!

Signed-off-by: Henrique de Moraes Holschuh <hmh <at> hmh.eng.br>
Tested-by: Karol Lewandowski <lmctlx <at> gmail.com>
Cc: Adrian Bunk <bunk <at> kernel.org>
---
 drivers/misc/thinkpad_acpi.c |   55 +++++++++++++++++++++--------------------
 1 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index 58973da..b596929 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
 <at>  <at>  -3853,7 +3853,7  <at>  <at>  static const char * const tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
(Continue reading)

Picon

[GIT PATCH] thinkpad-acpi bug fixes for 2.6.26-rc (v2)

Len,

This patchset has my current thinkpad-acpi queue for 2.6.26-rc, rebased
to properly apply to latest Linus.  It includes two fixes, one of them
being a regression from 2.6.25 that bites users of older ThinkPads.

Please send them to Linus for mainline merge.

As usual, the patch set is available at:
git://repo.or.cz/linux-2.6/linux-acpi-2.6/ibm-acpi-2.6.git for-upstream/acpi-release

Shortlog:

Henrique de Moraes Holschuh (3):
      SW_RADIO to SW_RFKILL_ALL rename
      ACPI: thinkpad-acpi: fix initialization error paths
      ACPI: thinkpad-acpi: fix LED handling on older ThinkPads

Thanks!

--

-- 
  "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-acpi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Picon

[PATCH 1/3] SW_RADIO to SW_RFKILL_ALL rename

Rename SW_RADIO to SW_RFKILL_ALL in thinkpad-acpi code and docs, following
5adad0133907790c50283bf03271d920d6897043 "Input: rename SW_RADIO to
SW_RFKILL_ALL".

Signed-off-by: Henrique de Moraes Holschuh <hmh <at> hmh.eng.br>
---
 Documentation/laptops/thinkpad-acpi.txt |    2 +-
 drivers/misc/thinkpad_acpi.c            |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/laptops/thinkpad-acpi.txt b/Documentation/laptops/thinkpad-acpi.txt
index 01c6c3d..64b3f14 100644
--- a/Documentation/laptops/thinkpad-acpi.txt
+++ b/Documentation/laptops/thinkpad-acpi.txt
 <at>  <at>  -503,7 +503,7  <at>  <at>  generate input device EV_KEY events.
 In addition to the EV_KEY events, thinkpad-acpi may also issue EV_SW
 events for switches:

-SW_RADIO	T60 and later hardare rfkill rocker switch
+SW_RFKILL_ALL	T60 and later hardare rfkill rocker switch
 SW_TABLET_MODE	Tablet ThinkPads HKEY events 0x5009 and 0x500A

 Non hot-key ACPI HKEY event map:
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index a0ce0b2..f81e048 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
 <at>  <at>  -1293,7 +1293,7  <at>  <at>  static void tpacpi_input_send_radiosw(void)
 		mutex_lock(&tpacpi_inputdev_send_mutex);

(Continue reading)

Picon

[PATCH 2/3] ACPI: thinkpad-acpi: fix initialization error paths

Rework some subdriver init and exit handlers, in order to fix some
initialization error paths that were missing, or broken.

Hitting those bugs should be extremely rare in the real world, but should
that happen, thinkpad-acpi would fail to dealocate some resources and a
reboot might well be needed to be able to load the driver again.

Signed-off-by: Henrique de Moraes Holschuh <hmh <at> hmh.eng.br>
---
 drivers/misc/thinkpad_acpi.c |  435 ++++++++++++++++++++++--------------------
 1 files changed, 229 insertions(+), 206 deletions(-)

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index f81e048..58973da 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
 <at>  <at>  -1921,6 +1921,29  <at>  <at>  static struct attribute *hotkey_mask_attributes[] __initdata = {
 	&dev_attr_hotkey_wakeup_hotunplug_complete.attr,
 };

+static void hotkey_exit(void)
+{
+#ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
+	hotkey_poll_stop_sync();
+#endif
+
+	if (hotkey_dev_attributes)
+		delete_attr_set(hotkey_dev_attributes, &tpacpi_pdev->dev.kobj);
+
+	kfree(hotkey_keycode_map);
(Continue reading)

Len Brown | 4 Jun 06:57 2008

Re: [PATCH 3/3] ACPI: thinkpad-acpi: fix LED handling on older ThinkPads

oh i see -- i had a conflict b/c i applied this to a branch
which didn't have the recent upstream const patch.

applied.

thanks,
-len

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Len Brown | 4 Jun 06:42 2008

Re: [PATCH 3/3] ACPI: thinkpad-acpi: fix LED handling on older ThinkPads

this one didn't apply on top of the other two?

On Tue, 3 Jun 2008, Henrique de Moraes Holschuh wrote:

> The less tested codepaths for LED handling, used on ThinkPads 570, 600e/x,
> 770e, 770x, A21e, A2xm/p, T20-22, X20 and maybe a few others, would write
> data to kernel memory it had no business touching, for leds number 3 and
> above.  If one is lucky, that illegal write would cause an OOPS, but
> chances are it would silently corrupt a byte.
> 
> The problem was introduced in commit af116101, "ACPI: thinkpad-acpi: add
> sysfs led class support to thinkpad leds (v3.2)".
> 
> Fix the bug by refactoring the entire code to be far more obvious on what
> it wants to do.  Also do some defensive "constification".
> 
> Issue reported by Karol Lewandowski <lmctlx <at> gmail.com> (he's an lucky guy
> and got an OOPS instead of silent corruption :-) ).
> 
> Root cause of the OOPS identified by Adrian Bunk <bunk <at> kernel.org>.
> Thanks, Adrian!
> 
> Signed-off-by: Henrique de Moraes Holschuh <hmh <at> hmh.eng.br>
> Tested-by: Karol Lewandowski <lmctlx <at> gmail.com>
> Cc: Adrian Bunk <bunk <at> kernel.org>
> ---
>  drivers/misc/thinkpad_acpi.c |   55 +++++++++++++++++++++--------------------
>  1 files changed, 28 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
(Continue reading)

Henrique de Moraes Holschuh | 13 Jun 06:24 2008
Picon

thinkpad-acpi release 0.21-20080612 uploaded to ibm-acpi.sf.net

I have released version 0.21-20080612 of thinkpad-acpi through the
sourceforge.net release system.

Patches are available for 2.6.23, 2.6.24, 2.6.25 and 2.6.26-rc6 at:
http://sourceforge.net/project/showfiles.php?group_id=117042&package_id=230205

git users can get it directly from tags in:
git://repo.or.cz/linux-2.6/linux-acpi-2.6/ibm-acpi-2.6.git

This is likely to be the last thinkpad-acpi release for the 2.6.23 kernel.

This release includes support for the rfkill subsystem in thinkpad-acpi,
which I hope to submit for mainline 2.6.27.

It also includes a full backport of rfkill changes and enhancements that may
be merged for 2.6.27, without which thinkpad-acpi cannot support the kernel
rfkill subsystem.

This release includes a *VERY* important fix to the LED class code for older
thinkpads (like the A-series) which uses the LED_OLD access method.  Without
the fix, the thinkpad-acpi LED code can corrupt a single byte of kernel
memory at some very specific locations, with unknown consequences (lucky
people, like the one who reported the bug, would get a kernel OOPS. I have
no idea what could happen to unlucky people).

Enjoy!

--

-- 
  "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
(Continue reading)

Evgeni Golov | 13 Jun 17:00 2008
Picon

Re: thinkpad-acpi release 0.21-20080612 uploaded to ibm-acpi.sf.net

On Fri, 13 Jun 2008 01:24:56 -0300 Henrique de Moraes Holschuh wrote:

> I have released version 0.21-20080612 of thinkpad-acpi through the

Running 2.6.26-rc6 on a X31, I get Bluetooth autoenabled during boot.
That does not happen with 2.6.26-rc5 (and the included tpacpi).
I did not yet have the time to test if this also happens with
2.6.26-rc6 without tpacpi 0.21-20080612, but wanted to let you know
ASAP.

Regards
Evgeni

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
Henrique de Moraes Holschuh | 13 Jun 18:51 2008
Picon

Re: thinkpad-acpi release 0.21-20080612 uploaded to ibm-acpi.sf.net

On Fri, 13 Jun 2008, Evgeni Golov wrote:
> On Fri, 13 Jun 2008 01:24:56 -0300 Henrique de Moraes Holschuh wrote:
> 
> > I have released version 0.21-20080612 of thinkpad-acpi through the
> 
> Running 2.6.26-rc6 on a X31, I get Bluetooth autoenabled during boot.
> That does not happen with 2.6.26-rc5 (and the included tpacpi).
> I did not yet have the time to test if this also happens with
> 2.6.26-rc6 without tpacpi 0.21-20080612, but wanted to let you know
> ASAP.

There are going to be differences to the initial state of WWAN and
Bluetooth, now.  rfkill will set that state, and it does not depend on the
thinkpad firmware defaults (however, if the radio is blocked by any hardware
switch, it will NOT turn on).  Default state for rfkill is to ENABLE radios
for now (which I don't agree with, but I didn't change it).  One of the
changes I made to rfkill is to let you set (with a module parameter) that
initial state to DISABLED.

Basically, now there is something else talking to thinkpad-acpi to set WWAN
and Bluetooth radio state, so you may observe different behaviour.  As long
as it is coherent behaviour, that's fine (but see below).

So, please check if you can enable/disable the radios properly.  If you can,
and what changed was the initial state only, it is all fine (but we can
track down the reason for the initial state change if you want.  I find it
interesting that it is now "disabled" for you, I distinctly remember it was
supposed to be "enabled" unless you override that yourself).

It *is* possible that we have a logic inversion bug (enable when it should
(Continue reading)

sargentd | 13 Jun 20:44 2008
Picon

Re: [ibm-acpi-devel] thinkpad-acpi release 0.21-20080612 uploaded to ibm-acpi.sf.net

>> Running 2.6.26-rc6 on a X31, I get Bluetooth autoenabled during boot.
>> That does not happen with 2.6.26-rc5 (and the included tpacpi).
>> I did not yet have the time to test if this also happens with
>> 2.6.26-rc6 without tpacpi 0.21-20080612, but wanted to let you know
>> ASAP.
>
> There are going to be differences to the initial state of WWAN and
> Bluetooth, now.  rfkill will set that state, and it does not depend on the
> thinkpad firmware defaults (however, if the radio is blocked by any
> hardware
> switch, it will NOT turn on).  Default state for rfkill is to ENABLE
> radios
> for now (which I don't agree with, but I didn't change it).  One of the
> changes I made to rfkill is to let you set (with a module parameter) that
> initial state to DISABLED.

Okay, that brings some light :)
So giving default_state=0 to the rfkill module should bring it in my
wanted "first-always-off" state.

> Basically, now there is something else talking to thinkpad-acpi to set
> WWAN
> and Bluetooth radio state, so you may observe different behaviour.  As
> long
> as it is coherent behaviour, that's fine (but see below).
>
> So, please check if you can enable/disable the radios properly.  If you
> can,
> and what changed was the initial state only, it is all fine (but we can
> track down the reason for the initial state change if you want.  I find it
(Continue reading)


Gmane