[PATCH] Fix possible null ptr dereference

[PATCH] Fix possible null ptr dereference

[donald.d.dugger]

Code in `pci_link.c' is calling the internal routine `acpi_ut_evaluate_object'
which is dangerous given that it is passing a NULL pointer when it should
be passing a pointer to a real object.  The patch corrects the issue by
having the code call the external routine `acpi_evaluate_object', which
correctly handles a NULL pointer.

Signed-off-by: Don Dugger <donald.d.dugger@intel.com>


----- cut here for patch.d/acpi_null-1017.patch -----
diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
index cf47805..65bf4fa 100644
--- a/drivers/acpi/pci_link.c
+++ b/drivers/acpi/pci_link.c
@@ -709,7 +709,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
 			  acpi_device_bid(link->device)));
 
 	if (link->refcnt == 0) {
-		acpi_ut_evaluate_object(link->device->handle, "_DIS", 0, NULL);
+		acpi_evaluate_object(link->device->handle, "_DIS", NULL, NULL);
 	}
 	mutex_unlock(&acpi_link_lock);
 	return (link->irq.active);
@@ -773,7 +773,7 @@ static int acpi_pci_link_add(struct acpi_device *device)
 
       end:
 	/* disable all links -- to be activated on use */
-	acpi_ut_evaluate_object(device->handle, "_DIS", 0, NULL);
+	acpi_evaluate_object(device->handle, "_DIS", NULL, NULL);
 	mutex_unlock(&acpi_link_lock);
 
 	if (result)

Re: [PATCH] Fix possible null ptr dereference

[Len Brown]

applied.

thanks,
-Len

On Fri, 17 Oct 2008, donald.d.dugger@intel.com wrote:

> Code in `pci_link.c' is calling the internal routine `acpi_ut_evaluate_object'
> which is dangerous given that it is passing a NULL pointer when it should
> be passing a pointer to a real object.  The patch corrects the issue by
> having the code call the external routine `acpi_evaluate_object', which
> correctly handles a NULL pointer.
> 
> Signed-off-by: Don Dugger <donald.d.dugger@intel.com>
> 
> 
> ----- cut here for patch.d/acpi_null-1017.patch -----
> diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
> index cf47805..65bf4fa 100644
> --- a/drivers/acpi/pci_link.c
> +++ b/drivers/acpi/pci_link.c
> @@ -709,7 +709,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
>  			  acpi_device_bid(link->device)));
>  
>  	if (link->refcnt == 0) {
> -		acpi_ut_evaluate_object(link->device->handle, "_DIS", 0, NULL);
> +		acpi_evaluate_object(link->device->handle, "_DIS", NULL, NULL);
>  	}
>  	mutex_unlock(&acpi_link_lock);
>  	return (link->irq.active);
> @@ -773,7 +773,7 @@ static int acpi_pci_link_add(struct acpi_device *device)
>  
>        end:
>  	/* disable all links -- to be activated on use */
> -	acpi_ut_evaluate_object(device->handle, "_DIS", 0, NULL);
> +	acpi_evaluate_object(device->handle, "_DIS", NULL, NULL);
>  	mutex_unlock(&acpi_link_lock);
>  
>  	if (result)
> 

RE: [PATCH] Fix possible null ptr dereference

[Moore, Robert]

In general, the internal ACPICA functions do not perform nearly as much parameter validation as the external functions. The Host OS should not be calling any of the ACPICA internal functions for this and a few other good reasons -- such as the fact that internal functions can disappear, be renamed, or have the parameters changed without warning at any time.

It would probably be a good idea to audit Linux for the use of internal ACPICA functions and fix these bugs.

Bob



>-----Original Message-----
>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>owner@vger.kernel.org] On Behalf Of donald.d.dugger@intel.com
>Sent: Friday, October 17, 2008 7:50 AM
>To: linux-acpi@vger.kernel.org
>Cc: bjorn.helgaas@hp.com; akpm@linux-foundation.org; astarikovskiy@suse.de;
>lenb@kernel.org
>Subject: [PATCH] Fix possible null ptr dereference
>
>Code in `pci_link.c' is calling the internal routine
>`acpi_ut_evaluate_object'
>which is dangerous given that it is passing a NULL pointer when it should
>be passing a pointer to a real object.  The patch corrects the issue by
>having the code call the external routine `acpi_evaluate_object', which
>correctly handles a NULL pointer.
>
>Signed-off-by: Don Dugger <donald.d.dugger@intel.com>
>
>
>----- cut here for patch.d/acpi_null-1017.patch -----
>diff --git a/drivers/acpi/pci_link.c b/drivers/acpi/pci_link.c
>index cf47805..65bf4fa 100644
>--- a/drivers/acpi/pci_link.c
>+++ b/drivers/acpi/pci_link.c
>@@ -709,7 +709,7 @@ int acpi_pci_link_free_irq(acpi_handle handle)
>                          acpi_device_bid(link->device)));
>
>        if (link->refcnt == 0) {
>-               acpi_ut_evaluate_object(link->device->handle, "_DIS", 0,
>NULL);
>+               acpi_evaluate_object(link->device->handle, "_DIS", NULL,
>NULL);
>        }
>        mutex_unlock(&acpi_link_lock);
>        return (link->irq.active);
>@@ -773,7 +773,7 @@ static int acpi_pci_link_add(struct acpi_device
>*device)
>
>       end:
>        /* disable all links -- to be activated on use */
>-       acpi_ut_evaluate_object(device->handle, "_DIS", 0, NULL);
>+       acpi_evaluate_object(device->handle, "_DIS", NULL, NULL);
>        mutex_unlock(&acpi_link_lock);
>
>        if (result)
>--
>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] Fix possible null ptr dereference

[Len Brown]

> In general, the internal ACPICA functions do not perform nearly as much parameter validation as the external functions. The Host OS should not be calling any of the ACPICA internal functions for this and a few other good reasons -- such as the fact that internal functions can disappear, be renamed, or have the parameters changed without warning at any time.
> 
> It would probably be a good idea to audit Linux for the use of internal ACPICA functions and fix these bugs.

I think the way to do this is to clean up the headers
so that code outside of ACPICA doesn't even see
the declarations of the internal ACPICA functions.

Right now everything includes everything, so the headers can't help us.

-Len

RE: [PATCH] Fix possible null ptr dereference

[Moore, Robert]

It's on the TBD list.

Not true that everything includes everything, however. But everything is in the same directory.

How would you organize things?


>-----Original Message-----
>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>owner@vger.kernel.org] On Behalf Of Len Brown
>Sent: Friday, October 17, 2008 1:55 PM
>To: Moore, Robert
>Cc: Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com;
>akpm@linux-foundation.org; astarikovskiy@suse.de; Lin, Ming M
>Subject: RE: [PATCH] Fix possible null ptr dereference
>
>
>> In general, the internal ACPICA functions do not perform nearly as much
>parameter validation as the external functions. The Host OS should not be
>calling any of the ACPICA internal functions for this and a few other good
>reasons -- such as the fact that internal functions can disappear, be
>renamed, or have the parameters changed without warning at any time.
>>
>> It would probably be a good idea to audit Linux for the use of internal
>ACPICA functions and fix these bugs.
>
>I think the way to do this is to clean up the headers
>so that code outside of ACPICA doesn't even see
>the declarations of the internal ACPICA functions.
>
>Right now everything includes everything, so the headers can't help us.
>
>-Len
>--
>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

ACPICA header files

[Moore, Robert]

The actual "public" acpica headers are:

acexcep.h    // ACPICA exceptions
actypes.h    // ACPICA data types
actbl.h      // ACPI table definitions
acpixf.h     // Prototypes for ACPICA interfaces (host-to-ACPICA)
acpiosxf.h   // Prototypes for OSL interfaces (ACPICA-to-host)

And the entire contents of the include/platform directory

Would it make sense (would it be helpful) to leave these public headers in the current acpi include directory and to move everything else to a new directory?

How about include/internal.

We could keep the existing acpi.h file and have it only pull in the files above, so the disruption to existing acpica users would be minimal. The only code that would be affected would be code that is cheating by using ACPICA internal interfaces.

It may be the case that some internal functions are actually useful to the ACPI-related device drivers. I don't have any issue with making these interfaces public (and beefing up the parameter validation, etc.)

Comments?
Thanks,
Bob



-----Original Message-----
From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Moore, Robert
Sent: Friday, October 17, 2008 2:50 PM
To: Len Brown
Cc: Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com; akpm@linux-foundation.org; astarikovskiy@suse.de; Lin, Ming M
Subject: RE: [PATCH] Fix possible null ptr dereference

It's on the TBD list.

Not true that everything includes everything, however. But everything is in the same directory.

How would you organize things?


>-----Original Message-----
>From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>owner@vger.kernel.org] On Behalf Of Len Brown
>Sent: Friday, October 17, 2008 1:55 PM
>To: Moore, Robert
>Cc: Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com;
>akpm@linux-foundation.org; astarikovskiy@suse.de; Lin, Ming M
>Subject: RE: [PATCH] Fix possible null ptr dereference
>
>
>> In general, the internal ACPICA functions do not perform nearly as much
>parameter validation as the external functions. The Host OS should not be
>calling any of the ACPICA internal functions for this and a few other good
>reasons -- such as the fact that internal functions can disappear, be
>renamed, or have the parameters changed without warning at any time.
>>
>> It would probably be a good idea to audit Linux for the use of internal
>ACPICA functions and fix these bugs.
>
>I think the way to do this is to clean up the headers
>so that code outside of ACPICA doesn't even see
>the declarations of the internal ACPICA functions.
>
>Right now everything includes everything, so the headers can't help us.
>
>-Len
>--
>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
--
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: ACPICA header files

[Alexey Starikovskiy]

The problem with linux acpi headers is that all drivers include <linux/acpi.h>, which will include private headers.
We need to remove all private headers from this public one.

Moore, Robert wrote:
> The actual "public" acpica headers are:
> 
> acexcep.h    // ACPICA exceptions
> actypes.h    // ACPICA data types
> actbl.h      // ACPI table definitions
> acpixf.h     // Prototypes for ACPICA interfaces (host-to-ACPICA)
> acpiosxf.h   // Prototypes for OSL interfaces (ACPICA-to-host)
> 
> And the entire contents of the include/platform directory
> 
> Would it make sense (would it be helpful) to leave these public headers in the current acpi include directory and to move everything else to a new directory?
> 
> How about include/internal.
> 
> We could keep the existing acpi.h file and have it only pull in the files above, so the disruption to existing acpica users would be minimal. The only code that would be affected would be code that is cheating by using ACPICA internal interfaces.
> 
> It may be the case that some internal functions are actually useful to the ACPI-related device drivers. I don't have any issue with making these interfaces public (and beefing up the parameter validation, etc.)
> 
> Comments?
> Thanks,
> Bob
> 
> 
> 
> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Moore, Robert
> Sent: Friday, October 17, 2008 2:50 PM
> To: Len Brown
> Cc: Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com; akpm@linux-foundation.org; astarikovskiy@suse.de; Lin, Ming M
> Subject: RE: [PATCH] Fix possible null ptr dereference
> 
> It's on the TBD list.
> 
> Not true that everything includes everything, however. But everything is in the same directory.
> 
> How would you organize things?
> 
> 
>> -----Original Message-----
>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>> owner@vger.kernel.org] On Behalf Of Len Brown
>> Sent: Friday, October 17, 2008 1:55 PM
>> To: Moore, Robert
>> Cc: Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com;
>> akpm@linux-foundation.org; astarikovskiy@suse.de; Lin, Ming M
>> Subject: RE: [PATCH] Fix possible null ptr dereference
>>
>>
>>> In general, the internal ACPICA functions do not perform nearly as much
>> parameter validation as the external functions. The Host OS should not be
>> calling any of the ACPICA internal functions for this and a few other good
>> reasons -- such as the fact that internal functions can disappear, be
>> renamed, or have the parameters changed without warning at any time.
>>> It would probably be a good idea to audit Linux for the use of internal
>> ACPICA functions and fix these bugs.
>>
>> I think the way to do this is to clean up the headers
>> so that code outside of ACPICA doesn't even see
>> the declarations of the internal ACPICA functions.
>>
>> Right now everything includes everything, so the headers can't help us.
>>
>> -Len
>> --
>> 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
> --
> 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: ACPICA header files

[Moore, Robert]

Right. We would change acpi.h to only pull in the public headers.

This would only break code that is cheating.

-----Original Message-----
From: Alexey Starikovskiy [mailto:astarikovskiy@suse.de]
Sent: Saturday, October 18, 2008 10:42 AM
To: Moore, Robert
Cc: Len Brown; Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com; akpm@linux-foundation.org; Lin, Ming M
Subject: Re: ACPICA header files

The problem with linux acpi headers is that all drivers include <linux/acpi.h>, which will include private headers.
We need to remove all private headers from this public one.

Moore, Robert wrote:
> The actual "public" acpica headers are:
>
> acexcep.h    // ACPICA exceptions
> actypes.h    // ACPICA data types
> actbl.h      // ACPI table definitions
> acpixf.h     // Prototypes for ACPICA interfaces (host-to-ACPICA)
> acpiosxf.h   // Prototypes for OSL interfaces (ACPICA-to-host)
>
> And the entire contents of the include/platform directory
>
> Would it make sense (would it be helpful) to leave these public headers in the current acpi include directory and to move everything else to a new directory?
>
> How about include/internal.
>
> We could keep the existing acpi.h file and have it only pull in the files above, so the disruption to existing acpica users would be minimal. The only code that would be affected would be code that is cheating by using ACPICA internal interfaces.
>
> It may be the case that some internal functions are actually useful to the ACPI-related device drivers. I don't have any issue with making these interfaces public (and beefing up the parameter validation, etc.)
>
> Comments?
> Thanks,
> Bob
>
>
>
> -----Original Message-----
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Moore, Robert
> Sent: Friday, October 17, 2008 2:50 PM
> To: Len Brown
> Cc: Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com; akpm@linux-foundation.org; astarikovskiy@suse.de; Lin, Ming M
> Subject: RE: [PATCH] Fix possible null ptr dereference
>
> It's on the TBD list.
>
> Not true that everything includes everything, however. But everything is in the same directory.
>
> How would you organize things?
>
>
>> -----Original Message-----
>> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
>> owner@vger.kernel.org] On Behalf Of Len Brown
>> Sent: Friday, October 17, 2008 1:55 PM
>> To: Moore, Robert
>> Cc: Dugger, Donald D; linux-acpi@vger.kernel.org; bjorn.helgaas@hp.com;
>> akpm@linux-foundation.org; astarikovskiy@suse.de; Lin, Ming M
>> Subject: RE: [PATCH] Fix possible null ptr dereference
>>
>>
>>> In general, the internal ACPICA functions do not perform nearly as much
>> parameter validation as the external functions. The Host OS should not be
>> calling any of the ACPICA internal functions for this and a few other good
>> reasons -- such as the fact that internal functions can disappear, be
>> renamed, or have the parameters changed without warning at any time.
>>> It would probably be a good idea to audit Linux for the use of internal
>> ACPICA functions and fix these bugs.
>>
>> I think the way to do this is to clean up the headers
>> so that code outside of ACPICA doesn't even see
>> the declarations of the internal ACPICA functions.
>>
>> Right now everything includes everything, so the headers can't help us.
>>
>> -Len
>> --
>> 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
> --
> 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