Patchwork ACPICA BZ 1035 fixes segfault with too many ^^^^^^ prefixes. (LP: #1205714)

login
register
mail settings
Submitter Colin King
Date July 27, 2013, 8:36 p.m.
Message ID <1374957413-14273-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/262529/
State Accepted
Headers show

Comments

Colin King - July 27, 2013, 8:36 p.m.
From: Colin Ian King <colin.king@canonical.com>

While doing some regression checks I found a particular DSDT that caused
ACPICA to segfault on compilation when too many ^^^^^ prefixes are found.
This was fixed on Friday by Robert Moore and I'd like the changes applied
to fwts.  Original bug: https://bugs.acpica.org/show_bug.cgi?id=1035

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpica/source/compiler/aslload.c              | 4 ++++
 src/acpica/source/components/namespace/nsaccess.c | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)
Alex Hung - July 29, 2013, 8:22 a.m.
On 07/28/2013 04:36 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> While doing some regression checks I found a particular DSDT that caused
> ACPICA to segfault on compilation when too many ^^^^^ prefixes are found.
> This was fixed on Friday by Robert Moore and I'd like the changes applied
> to fwts.  Original bug: https://bugs.acpica.org/show_bug.cgi?id=1035
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpica/source/compiler/aslload.c              | 4 ++++
>   src/acpica/source/components/namespace/nsaccess.c | 4 ++--
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/acpica/source/compiler/aslload.c b/src/acpica/source/compiler/aslload.c
> index 338cf1c..27bd254 100644
> --- a/src/acpica/source/compiler/aslload.c
> +++ b/src/acpica/source/compiler/aslload.c
> @@ -559,6 +559,10 @@ LdNamespace1Begin (
>                               ACPI_TYPE_LOCAL_SCOPE,
>                               ACPI_IMODE_LOAD_PASS1, Flags,
>                               WalkState, &(Node));
> +                if (ACPI_FAILURE (Status))
> +                {
> +                    return_ACPI_STATUS (Status);
> +                }

Hi Colin,

Do you think we can remove the curly brackets?

>
>                   /*
>                    * However, this is an error -- primarily because the MS
> diff --git a/src/acpica/source/components/namespace/nsaccess.c b/src/acpica/source/components/namespace/nsaccess.c
> index c09e11b..d0e70f6 100644
> --- a/src/acpica/source/components/namespace/nsaccess.c
> +++ b/src/acpica/source/components/namespace/nsaccess.c
> @@ -520,8 +520,8 @@ AcpiNsLookup (
>                       /* Current scope has no parent scope */
>
>                       ACPI_ERROR ((AE_INFO,
> -                        "ACPI path has too many parent prefixes (^) "
> -                        "- reached beyond root node"));
> +                        "%s: Path has too many parent prefixes (^) "
> +                        "- reached beyond root node", Pathname));
>                       return_ACPI_STATUS (AE_NOT_FOUND);
>                   }
>               }
>
Colin King - July 29, 2013, 9:32 a.m.
On 29/07/13 09:22, Alex Hung wrote:
> On 07/28/2013 04:36 AM, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> While doing some regression checks I found a particular DSDT that caused
>> ACPICA to segfault on compilation when too many ^^^^^ prefixes are found.
>> This was fixed on Friday by Robert Moore and I'd like the changes applied
>> to fwts.  Original bug: https://bugs.acpica.org/show_bug.cgi?id=1035
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   src/acpica/source/compiler/aslload.c              | 4 ++++
>>   src/acpica/source/components/namespace/nsaccess.c | 4 ++--
>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/acpica/source/compiler/aslload.c
>> b/src/acpica/source/compiler/aslload.c
>> index 338cf1c..27bd254 100644
>> --- a/src/acpica/source/compiler/aslload.c
>> +++ b/src/acpica/source/compiler/aslload.c
>> @@ -559,6 +559,10 @@ LdNamespace1Begin (
>>                               ACPI_TYPE_LOCAL_SCOPE,
>>                               ACPI_IMODE_LOAD_PASS1, Flags,
>>                               WalkState, &(Node));
>> +                if (ACPI_FAILURE (Status))
>> +                {
>> +                    return_ACPI_STATUS (Status);
>> +                }
>
> Hi Colin,
>
> Do you think we can remove the curly brackets?

No. This is from ACPICA, we need to keep it in ACPICA formatting style.
>
>>
>>                   /*
>>                    * However, this is an error -- primarily because
>> the MS
>> diff --git a/src/acpica/source/components/namespace/nsaccess.c
>> b/src/acpica/source/components/namespace/nsaccess.c
>> index c09e11b..d0e70f6 100644
>> --- a/src/acpica/source/components/namespace/nsaccess.c
>> +++ b/src/acpica/source/components/namespace/nsaccess.c
>> @@ -520,8 +520,8 @@ AcpiNsLookup (
>>                       /* Current scope has no parent scope */
>>
>>                       ACPI_ERROR ((AE_INFO,
>> -                        "ACPI path has too many parent prefixes (^) "
>> -                        "- reached beyond root node"));
>> +                        "%s: Path has too many parent prefixes (^) "
>> +                        "- reached beyond root node", Pathname));
>>                       return_ACPI_STATUS (AE_NOT_FOUND);
>>                   }
>>               }
>>
>
>
Alex Hung - July 29, 2013, 11:34 a.m.
On 07/29/2013 05:32 PM, Colin Ian King wrote:
> On 29/07/13 09:22, Alex Hung wrote:
>> On 07/28/2013 04:36 AM, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> While doing some regression checks I found a particular DSDT that caused
>>> ACPICA to segfault on compilation when too many ^^^^^ prefixes are
>>> found.
>>> This was fixed on Friday by Robert Moore and I'd like the changes
>>> applied
>>> to fwts.  Original bug: https://bugs.acpica.org/show_bug.cgi?id=1035
>>>
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>>   src/acpica/source/compiler/aslload.c              | 4 ++++
>>>   src/acpica/source/components/namespace/nsaccess.c | 4 ++--
>>>   2 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/acpica/source/compiler/aslload.c
>>> b/src/acpica/source/compiler/aslload.c
>>> index 338cf1c..27bd254 100644
>>> --- a/src/acpica/source/compiler/aslload.c
>>> +++ b/src/acpica/source/compiler/aslload.c
>>> @@ -559,6 +559,10 @@ LdNamespace1Begin (
>>>                               ACPI_TYPE_LOCAL_SCOPE,
>>>                               ACPI_IMODE_LOAD_PASS1, Flags,
>>>                               WalkState, &(Node));
>>> +                if (ACPI_FAILURE (Status))
>>> +                {
>>> +                    return_ACPI_STATUS (Status);
>>> +                }
>>
>> Hi Colin,
>>
>> Do you think we can remove the curly brackets?
>
> No. This is from ACPICA, we need to keep it in ACPICA formatting style.
>>
>>>
>>>                   /*
>>>                    * However, this is an error -- primarily because
>>> the MS
>>> diff --git a/src/acpica/source/components/namespace/nsaccess.c
>>> b/src/acpica/source/components/namespace/nsaccess.c
>>> index c09e11b..d0e70f6 100644
>>> --- a/src/acpica/source/components/namespace/nsaccess.c
>>> +++ b/src/acpica/source/components/namespace/nsaccess.c
>>> @@ -520,8 +520,8 @@ AcpiNsLookup (
>>>                       /* Current scope has no parent scope */
>>>
>>>                       ACPI_ERROR ((AE_INFO,
>>> -                        "ACPI path has too many parent prefixes (^) "
>>> -                        "- reached beyond root node"));
>>> +                        "%s: Path has too many parent prefixes (^) "
>>> +                        "- reached beyond root node", Pathname));
>>>                       return_ACPI_STATUS (AE_NOT_FOUND);
>>>                   }
>>>               }
>>>
>>
>>
>
>
Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu - Aug. 2, 2013, 2:46 a.m.
On 07/28/2013 04:36 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> While doing some regression checks I found a particular DSDT that caused
> ACPICA to segfault on compilation when too many ^^^^^ prefixes are found.
> This was fixed on Friday by Robert Moore and I'd like the changes applied
> to fwts.  Original bug: https://bugs.acpica.org/show_bug.cgi?id=1035
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpica/source/compiler/aslload.c              | 4 ++++
>   src/acpica/source/components/namespace/nsaccess.c | 4 ++--
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/acpica/source/compiler/aslload.c b/src/acpica/source/compiler/aslload.c
> index 338cf1c..27bd254 100644
> --- a/src/acpica/source/compiler/aslload.c
> +++ b/src/acpica/source/compiler/aslload.c
> @@ -559,6 +559,10 @@ LdNamespace1Begin (
>                               ACPI_TYPE_LOCAL_SCOPE,
>                               ACPI_IMODE_LOAD_PASS1, Flags,
>                               WalkState, &(Node));
> +                if (ACPI_FAILURE (Status))
> +                {
> +                    return_ACPI_STATUS (Status);
> +                }
>
>                   /*
>                    * However, this is an error -- primarily because the MS
> diff --git a/src/acpica/source/components/namespace/nsaccess.c b/src/acpica/source/components/namespace/nsaccess.c
> index c09e11b..d0e70f6 100644
> --- a/src/acpica/source/components/namespace/nsaccess.c
> +++ b/src/acpica/source/components/namespace/nsaccess.c
> @@ -520,8 +520,8 @@ AcpiNsLookup (
>                       /* Current scope has no parent scope */
>
>                       ACPI_ERROR ((AE_INFO,
> -                        "ACPI path has too many parent prefixes (^) "
> -                        "- reached beyond root node"));
> +                        "%s: Path has too many parent prefixes (^) "
> +                        "- reached beyond root node", Pathname));
>                       return_ACPI_STATUS (AE_NOT_FOUND);
>                   }
>               }
>

Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch

diff --git a/src/acpica/source/compiler/aslload.c b/src/acpica/source/compiler/aslload.c
index 338cf1c..27bd254 100644
--- a/src/acpica/source/compiler/aslload.c
+++ b/src/acpica/source/compiler/aslload.c
@@ -559,6 +559,10 @@  LdNamespace1Begin (
                             ACPI_TYPE_LOCAL_SCOPE,
                             ACPI_IMODE_LOAD_PASS1, Flags,
                             WalkState, &(Node));
+                if (ACPI_FAILURE (Status))
+                {
+                    return_ACPI_STATUS (Status);
+                }
 
                 /*
                  * However, this is an error -- primarily because the MS
diff --git a/src/acpica/source/components/namespace/nsaccess.c b/src/acpica/source/components/namespace/nsaccess.c
index c09e11b..d0e70f6 100644
--- a/src/acpica/source/components/namespace/nsaccess.c
+++ b/src/acpica/source/components/namespace/nsaccess.c
@@ -520,8 +520,8 @@  AcpiNsLookup (
                     /* Current scope has no parent scope */
 
                     ACPI_ERROR ((AE_INFO,
-                        "ACPI path has too many parent prefixes (^) "
-                        "- reached beyond root node"));
+                        "%s: Path has too many parent prefixes (^) "
+                        "- reached beyond root node", Pathname));
                     return_ACPI_STATUS (AE_NOT_FOUND);
                 }
             }