https://bugzilla.kernel.org/show_bug.cgi?id=14998





--- Comment #35 from Vegard Nossum <[email protected]>  2010-04-22 
21:13:00 ---
(By the way, I forgot to say: Thanks for the testing effort and the report!)

The actual problem here is that acpi_bus_receive_event() does a full memcpy
between two structures:

int acpi_bus_receive_event(struct acpi_bus_event *event)
{
        ...
        memcpy(event, entry, sizeof(struct acpi_bus_event));
        ...
}

Clearly, since kmemcheck complains about a read, at least the "entry" pointer
is tracked by kmemcheck and partially uninitialized. What about the "event"
pointer? It comes from the caller -- acpi_system_read_event():

acpi_system_read_event(struct file *file, char __user * buffer, size_t count,
                       loff_t * ppos)
{               
        ...
        struct acpi_bus_event event;
        ...
        result = acpi_bus_receive_event(&event);
        ...
}

So the actual problem is that "event" was allocated on the stack and is not
tracked by kmemcheck, so copying something uninitialized into it will trigger
the report.

I am in two minds about the proposed fix. The currently posted patch is
obviously the less intrusive one in terms of both correctness and churn, but
here are some alternative suggestions:

1. Copy the struct "by hand" and use strncpy() for the strings to avoid the
uninitialized areas.

2. Allocate the event with kmalloc() instead of on the stack and free it after
the call to acpi_bus_receive_event().

3. Allocate the event with kmalloc() and pass ownership of the structure to
acpi_bus_receive_event() to avoid the copying altogether.


Vegard

--- Comment #36 from Christian Casteyde <[email protected]>  
2010-04-22 21:51:33 ---
Just to confirm, I redit an addr2kine with rip as you mentioned: it is indeed
in memcpy that the problem occurs:

/usr/src/linux-2.6.33/arch/x86/include/asm/string_64.h:12
/usr/src/linux/drivers/acpi/bus.c:598

        spin_lock_irqsave(&acpi_bus_event_lock, flags);
        if (!list_empty(&acpi_bus_event_list)) {
                entry = list_entry(acpi_bus_event_list.next,
                                   struct acpi_bus_event, node);
                list_del(&entry->node);
        }
        spin_unlock_irqrestore(&acpi_bus_event_lock, flags);

        if (!entry)
                return -ENODEV;

here --->        memcpy(event, entry, sizeof(struct acpi_bus_event));

        kfree(entry);

        return 0;
}

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

------------------------------------------------------------------------------
_______________________________________________
acpi-bugzilla mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/acpi-bugzilla

Reply via email to