linux-kernel - Re: [RFC][PATCH] ACPI: Ignore invalid

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:	Fri, 4 May 2012 09:52:17 -0400
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	Marco Aurelio da Costa <costa@...ic.com>
Cc:	Len Brown <lenb@...nel.org>, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH] ACPI: Ignore invalid _PSS entries, but use valid
 ones

On Fri, May 04, 2012 at 10:46:01AM -0300, Marco Aurelio da Costa wrote:
> From: Marco Aurelio da Costa <costa@...ic.com>
> Signed-off-by: Marco Aurelio da Costa <costa@...ic.com>
> 
> The EliteBook 8560W has non-initialized entries in its _PSS ACPI
> table. Instead of bailing out when the first non-initialized entry is
> found, ignore it and use only  the valid entries. Only bail out if there
> is no valid entry at all.

Is that safe? Meaning re-use the other CPU's _PSS states? Perhaps the
warning at the end should say: "Trying to compensate by using the
other CPU's PSS state).

> 
> ---
> --- linux-3.3.3/drivers/acpi/processor_perflib.c.orig   2012-04-24
> 22:18:23.288041268 +0200
> +++ linux-3.3.3/drivers/acpi/processor_perflib.c        2012-04-24
> 22:19:25.912042603 +0200
> @@ -311,6 +311,7 @@ static int acpi_processor_get_performanc
>        struct acpi_buffer state = { 0, NULL };
>        union acpi_object *pss = NULL;
>        int i;
> +       int last_invalid = -1;
> 
> 
>        status = acpi_evaluate_object(pr->handle, "_PSS", NULL, &buffer);
> @@ -374,12 +375,30 @@ static int acpi_processor_get_performanc
>                        printk(KERN_ERR FW_BUG PREFIX
>                               "Invalid BIOS _PSS frequency: 0x%llx MHz\n",
>                               px->core_frequency);
> -                       result = -EFAULT;
> -                       kfree(pr->performance->states);
> -                       goto end;
> +                       if (-1 == last_invalid)

Swap it around or just do it this way:

if (last_invalid < 0)

> +                               last_invalid = i;
> +               } else {
> +                       if (last_invalid != -1) {

if (last_invalid >= 0)

> +                               /*
> +                                * Copy this valid entry over last_invalid entry
> +                                */
> +                               memcpy(&(pr->performance->states[last_invalid]),
> +                                      px, sizeof(struct acpi_processor_px));
> +                               ++last_invalid;
> +                       }
>                }
>        }
> 
> +       if (0 == last_invalid) {

So if _PSS that is missing is at CPU2, this own't print it.

I think you want 'if (last_invalid >= 0)'

> +               printk(KERN_ERR FW_BUG PREFIX
> +                      "No valid BIOS _PSS frequency found\n");

And you should mention which CPU has it busted - as there are
some that are working.


> +               result = -EFAULT;
> +               kfree(pr->performance->states);
> +       }
> +
> +       if (last_invalid > 0)

Don't you want 'last_invalid >= 0' ?

> +               pr->performance->state_count = last_invalid;
> +
>       end:
>        kfree(buffer.pointer);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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/