Re: [Bug #10670] BUG: linux-2.6.26-rc1 oops atthinkpad_acpi:led_set_status

From: Adrian Bunk
Date: Wed May 14 2008 - 19:32:26 EST


On Mon, May 12, 2008 at 01:17:53AM -0300, Henrique de Moraes Holschuh wrote:
> On Sun, 11 May 2008, Rafael J. Wysocki wrote:
> > This message has been generated automatically as a part of a report
> > of recent regressions.
> >
> > The following bug entry is on the current list of known regressions
> > from 2.6.25. Please verify if it still should be listed.
> >
> >
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=10670
> > Subject : BUG: linux-2.6.26-rc1 oops at thinkpad_acpi:led_set_status
> > Submitter : Karol Lewandowski <lmctlx@xxxxxxxxx>
> > Date : 2008-05-08 23:12 (4 days old)
> > References : http://marc.info/?l=linux-kernel&amp;m=121028841527994&amp;w=4
> > Handled-By : Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
>
> It is related to:
>
> 1. Debian gcc in stable being used to compile the kernel
> 2. unmodified led_set_status() function on thinkpad-acpi.c
> 3. kernel compiled in "optimize for size" mode
> 4. stack frames NOT being enabled.
>
> Change one, and the bug is gone.
>
> AFAIK, it means Debian stable's gcc is generating bad code.
>...


No, your code is buggy:


#define TPACPI_LED_NUMLEDS 8
static enum led_status_t tpacpi_led_state_cache[TPACPI_LED_NUMLEDS];


static int led_write(char *buf)
{
...
if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 7)
return -EINVAL;
...
rc = led_set_status(led, s);
...
}

static int led_set_status(unsigned int led, enum led_status_t ledstatus)
{
...
switch (led_supported) {
...
case TPACPI_LED_OLD:
/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20 */
-----> led = 1 << led;
rc = ec_write(TPACPI_LED_EC_HLMS, led);
if (rc >= 0)
rc = ec_write(TPACPI_LED_EC_HLBL,
led * led_exp_hlbl[ledstatus]);
if (rc >= 0)
rc = ec_write(TPACPI_LED_EC_HLCL,
led * led_exp_hlcl[ledstatus]);
break;
...
if (!rc)
tpacpi_led_state_cache[led] = ledstatus;
... ^^^
}


According to the assembler code "led" is in register EBX, and in the
trace the value of EBX is 0x80.

0x80 = 1 << 7

What happens when you write to tpacpi_led_state_cache[0x80] is
undefined, and it's not a surprise that random changes let the
bug seem to disappear.


cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

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