linux-kernel - Re: [Bug #10670] BUG: linux-2.6.26-rc1 oops at thinkpad_acpi:led_set

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:	Thu, 15 May 2008 02:30:37 +0300
From:	Adrian Bunk <bunk@...nel.org>
To:	Henrique de Moraes Holschuh <hmh@....eng.br>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Karol Lewandowski <lmctlx@...il.com>
Subject: Re: [Bug #10670] BUG: linux-2.6.26-rc1 oops at
	thinkpad_acpi:led_set_status

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@...il.com>
> > 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@....eng.br>
> 
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/