write thermal trip_points totally broken in 2.6.10-rc3-mm1 - patch attached

Hi,

The:
char *limit_string;
...
limit_string = kmalloc(ACPI_THERMAL_MAX_LIMIT_STR_LEN, GFP_KERNEL);
...

if (!tz || (count > sizeof(limit_string) - 1)) {
                ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid argument\n"));

cannot work at all (if you want to write more than 4/8 bytes into the file :) ) and I doubt that it makes much sense to allocate memory at this place.

I just reverted changes in acpi_thermal_write_trip_points, testet -> works:

I just realize the "/sbin/poweroff" call, I thought this has been thrown out and moved to userspace apps via thermal event notification (acpid, ...)?

@@ -64,7 +64,6 @@
 #define ACPI_THERMAL_PATH_POWEROFF     "/sbin/poweroff"
 
 #define ACPI_THERMAL_MAX_ACTIVE        10
-#define ACPI_THERMAL_MAX_LIMIT_STR_LEN 65
 
 #define KELVIN_TO_CELSIUS(t)    (long)(((long)t-2732>=0) ? 

((long)t-2732+5)/10 : ((long)t-2732-5)/10)
#define CELSIUS_TO_KELVIN(t) ((t+273)*10)
@@ -900,33 +899,21 @@

        struct seq_file         *m = (struct seq_file *)file->private_data;
        struct acpi_thermal     *tz = (struct acpi_thermal *)m->private;

-       char                    *limit_string;
+       char                    limit_string[65] = {'\0'};
        int                     num, critical, hot, passive;
-       int                     *active;
+       int                     active[ACPI_THERMAL_MAX_ACTIVE];
        int                     i = 0;

        ACPI_FUNCTION_TRACE("acpi_thermal_write_trip_points");

- limit_string = kmalloc(ACPI_THERMAL_MAX_LIMIT_STR_LEN, GFP_KERNEL);
- if(!limit_string)
- return_VALUE(-ENOMEM);
-
- memset(limit_string, 0, ACPI_THERMAL_MAX_LIMIT_STR_LEN);
-
- active = kmalloc(ACPI_THERMAL_MAX_ACTIVE *sizeof(int), GFP_KERNEL);
- if(!active)
- return_VALUE(-ENOMEM);
-

        if (!tz || (count > sizeof(limit_string) - 1)) {
                ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid argument\n"));
-               count = -EINVAL;
-               goto end;
+               return_VALUE(-EINVAL);
        }

        if (copy_from_user(limit_string, buffer, count)) {
                ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid data\n"));
-               count = -EFAULT;
-               goto end;
+               return_VALUE(-EFAULT);
        }

        limit_string[count] = '\0';
@@ -937,8 +924,7 @@
                                &active[5], &active[6], &active[7], 
&active[8], &active[9]);
        if(!(num >=5 && num < (ACPI_THERMAL_MAX_ACTIVE + 3))) {
                ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid data format\n"));
-               count = -EINVAL;
-               goto end;
+               return_VALUE(-EINVAL);
        }

        tz->trips.critical.temperature = CELSIUS_TO_KELVIN(critical);
@@ -950,9 +936,6 @@
                tz->trips.active[i].temperature = 
CELSIUS_TO_KELVIN(active[i]);
        }

-end:
- kfree(active);

-       kfree(limit_string);
        return_VALUE(count);

}

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now. http://productguide.itmanagersjournal.com/

The original patch was to get acpi off the "make checkstack" radar. I think we should fix it per below rather than discard it.

thanks,
-Len

===== drivers/acpi/thermal.c 1.41 vs edited =====

@@ -917,7 +917,7 @@
 	if(!active)
 		return_VALUE(-ENOMEM);
 
-	if (!tz || (count > sizeof(limit_string) - 1)) {
+	if (!tz || (count > ACPI_THERMAL_MAX_LIMIT_STR_LEN - 1)) {
 		ACPI_DEBUG_PRINT((ACPI_DB_ERROR, "Invalid argument\n"));
 		count = -EINVAL;
 		goto end;

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/