[PATCH] trival: fix acpi_hest_firmware_first_pci() caused oops

[PATCH] trival: fix acpi_hest_firmware_first_pci() caused oops

[Shaohua Li]

acpi_hest_firmware_first_pci() could be called when acpi is disabled
and cause system oops.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
 drivers/acpi/hest.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/hest.c b/drivers/acpi/hest.c
index 4bb18c9..1c527a1 100644
--- a/drivers/acpi/hest.c
+++ b/drivers/acpi/hest.c
@@ -123,6 +123,10 @@ int acpi_hest_firmware_first_pci(struct pci_dev *pci)
 {
 	acpi_status status = AE_NOT_FOUND;
 	struct acpi_table_header *hest = NULL;
+
+	if (acpi_disabled)
+		return 0;
+
 	status = acpi_get_table(ACPI_SIG_HEST, 1, &hest);
 
 	if (ACPI_SUCCESS(status)) {
-- 
1.6.3.3

Re: [PATCH] trival: fix acpi_hest_firmware_first_pci() caused oops

[Len Brown]

wouldn't it be better for acpi_get_table()
to return failure for acpi=off?

thanks,
Len Brown, Intel Open Source Technology Center

On Thu, 8 Apr 2010, Shaohua Li wrote:

> acpi_hest_firmware_first_pci() could be called when acpi is disabled
> and cause system oops.
> 
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> ---
>  drivers/acpi/hest.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/acpi/hest.c b/drivers/acpi/hest.c
> index 4bb18c9..1c527a1 100644
> --- a/drivers/acpi/hest.c
> +++ b/drivers/acpi/hest.c
> @@ -123,6 +123,10 @@ int acpi_hest_firmware_first_pci(struct pci_dev *pci)
>  {
>  	acpi_status status = AE_NOT_FOUND;
>  	struct acpi_table_header *hest = NULL;
> +
> +	if (acpi_disabled)
> +		return 0;
> +
>  	status = acpi_get_table(ACPI_SIG_HEST, 1, &hest);
>  
>  	if (ACPI_SUCCESS(status)) {
> -- 
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] trival: fix acpi_hest_firmware_first_pci() caused oops

[Shaohua Li]

On Tue, Apr 20, 2010 at 11:01:21PM +0800, Len Brown wrote:
Hmm, the mail is sent at Apr 20, I received it at Apr 29 :(

> wouldn't it be better for acpi_get_table()
> to return failure for acpi=off?
but acpi_disabled is a linux variable while acpi_get_table is ACPICA code.

Thanks,
Shaohua

> 
> On Thu, 8 Apr 2010, Shaohua Li wrote:
> 
> > acpi_hest_firmware_first_pci() could be called when acpi is disabled
> > and cause system oops.
> > 
> > Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> > ---
> >  drivers/acpi/hest.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/acpi/hest.c b/drivers/acpi/hest.c
> > index 4bb18c9..1c527a1 100644
> > --- a/drivers/acpi/hest.c
> > +++ b/drivers/acpi/hest.c
> > @@ -123,6 +123,10 @@ int acpi_hest_firmware_first_pci(struct pci_dev *pci)
> >  {
> >  	acpi_status status = AE_NOT_FOUND;
> >  	struct acpi_table_header *hest = NULL;
> > +
> > +	if (acpi_disabled)
> > +		return 0;
> > +
> >  	status = acpi_get_table(ACPI_SIG_HEST, 1, &hest);
> >  
> >  	if (ACPI_SUCCESS(status)) {
> > -- 
> > 1.6.3.3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 

Re: [PATCH] trival: fix acpi_hest_firmware_first_pci() caused oops

[Len Brown]

> > wouldn't it be better for acpi_get_table()
> > to return failure for acpi=off?

> but acpi_disabled is a linux variable while acpi_get_table is ACPICA code.

For the acpi_disabled case, acpi_get_table() will return AE_NOT_FOUND
because acpi_gbl_root_table_list.count == 0,
and leave the table pointer unchanged at NULL, yes?

Looking at the other callers of acpi_get_table(),
they all seem to be called when acpi is enabled
and they know the table in question is going to be there.
So I guess it is consistent for hest.c to know this too.
But it looks like acpi_get_table() should fail gracefully,
so why do we take a fault?

-Len