Re: [patch] pci-acpi: handle multiple _OSC

[Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>]

Shaohua Li wrote:
> On Fri, 2008-05-09 at 10:40 -0700, Jesse Barnes wrote:
>> On Friday, May 09, 2008 12:43 am Kenji Kaneshige wrote:
>>> -static u32 ctrlset_buf[3] = {0, 0, 0};
>>> -static u32 global_ctrlsets = 0;
>>> +#define MAX_ACPI_OSC 30 /* Should be enough */
>>> +static struct acpi_osc_data {
>>> +	acpi_handle handle;
>>> +	u32 ctrlset_buf[3];
>>> +	u32 global_ctrlsets;
>>> +} acpi_osc_data_array[MAX_ACPI_OSC];
>> Could this just be a linked list of OSC objects instead?
> fixed. patch is against Kenji's ?"PCI ACPI: fix uninitialized variable
> in __pci_osc_support_set" patch.
> 

I found a problem.

> @@ -201,19 +232,25 @@ acpi_status pci_osc_control_set(acpi_han
>  {
>  	acpi_status	status;
>  	u32		ctrlset;
> +	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
> +
> +	if (!osc_data) {
> +		printk(KERN_ERR "acpi osc data array is full\n");
> +		return AE_ERROR;
> +	}

The pci_osc_control_set() function can be called for the ACPI object
that doesn't have _OSC method. In this case, acpi_get_osc_data() would
allocate a useless memory region. To avoid this, we need to check the
existence of _OSC before calling acpi_get_osc_data(). Here is a patch
to fix this problem. It is against your patch.

---
 drivers/pci/pci-acpi.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6.26-rc2/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.26-rc2.orig/drivers/pci/pci-acpi.c
+++ linux-2.6.26-rc2/drivers/pci/pci-acpi.c
@@ -232,8 +232,14 @@ acpi_status pci_osc_control_set(acpi_han
 {
 	acpi_status	status;
 	u32		ctrlset;
-	struct acpi_osc_data *osc_data = acpi_get_osc_data(handle);
+	acpi_handle tmp;
+	struct acpi_osc_data *osc_data;
+
+	status = acpi_get_handle(handle, "_OSC", &tmp);
+	if (ACPI_FAILURE(status))
+		return status;
 
+	osc_data = acpi_get_osc_data(handle);
 	if (!osc_data) {
 		printk(KERN_ERR "acpi osc data array is full\n");
 		return AE_ERROR;