RE: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...")

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Wysocki, Rafael J
> Sent: Wednesday, May 15, 2019 1:57 PM
> To: Schmauss, Erik <erik.schmauss@xxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Paul Gortmaker
> <paul.gortmaker@xxxxxxxxxxxxx>; stable@xxxxxxxxxxxxxxx
> Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML
> interpreter: add region addresses...")
> 
> On 5/15/2019 6:57 AM, Greg Kroah-Hartman wrote:
> > On Wed, May 15, 2019 at 01:17:28AM +0000, Schmauss, Erik wrote:
> >>
> >>> -----Original Message-----
> >>> From: Greg Kroah-Hartman [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> >>> Sent: Monday, May 6, 2019 1:42 AM
> >>> To: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>; Wysocki, Rafael J
> >>> <rafael.j.wysocki@xxxxxxxxx>
> >>> Cc: stable@xxxxxxxxxxxxxxx; Schmauss, Erik <erik.schmauss@xxxxxxxxx>
> >>> Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA:
> >>> AML
> >>> interpreter: add region addresses...")
> >>>
> >>> On Sun, May 05, 2019 at 03:44:48PM -0400, Paul Gortmaker wrote:
> >>>> I noticed 4.19.35 got a backport of mainline 4abb951b, but it
> >>>> appears to be a duplicate backport that landed in the wrong
> >>>> function.  We can see this in the stable-queue repo:
> >>>>
> >>>> stable-queue$ find . -name '*acpica-aml-interpreter-add-region-addr*'
> >>>> |grep 4.19
> >>>> ./releases/4.19.6/acpica-aml-interpreter-add-region-addresses-in-gl
> >>>> oba
> >>>> l-list-during-initialization.patch
> >>>> ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresse
> >>>> s-i
> >>>> n.patch
> >>>> ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-g
> >>>> lob al-list-during-initialization.patch
> >>>> ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-gl
> >>>> oba
> >>>> l-list-during-initialization.patch
> >>>>
> >>>> So it was added to 4.19.2, reverted in .3, re-added in .6, and then
> >>>> finally patched into a similar looking but wrong function in .35
> >>>>
> >>>> If we diff the .6 and .35 versions, we see the function difference:
> >>>>
> >>>> -@@ -417,6 +417,10 @@ acpi_ds_eval_region_operands(struct acpi
> >>>> +@@ -523,6 +523,10 @@ acpi_ds_eval_table_region_operands(struc
> >>>>
> >>>> I don't know what the history is/was around the 2/3/6 churn, but
> >>>> the re-addition in 4.19.35 to a different function sure looks wrong.
> >>>>
> >>>> The commit adds a call "status = acpi_ut_add_address_range(..." and
> >>>> if we check mainline, there is only one in that file, but in
> >>>> 4.19.35+ there now are two calls - since the two functions had
> >>>> similar context and comments, it isn't hard to see how patch
> >>>> could/would apply it a 2nd time in the wrong place.
> >>>>
> >>>> I didn't check if any of the other currently maintained
> >>>> linux-stable versions also had this possible issue.
> >>>>
> >> Hi Greg,
> >>
> >>> Ugh, Rafael, did I mess this up again?  Can you check to see if I
> >>> need to fix this somehow?
> >> It should be called in acpi_ds_eval_region_operands rather than
> acpi_ds_eval_table_region_operands.
> >> Please remove the call from acpi_ds_eval_table_region_operands.
> > Great, can someone please send me a patch for this so that I don't get
> > it wrong myself?
> 
> Erik, can you please cut a patch for that against 4.19.35 and send it to Greg?
> 

I'm not sure what the process is for this case but here's the patch...
Let me know if you need me to send it some other way...

>From a738f1c452c0762d3c0a1b1a9a12c78bd97b0a23 Mon Sep 17 00:00:00 2001
From: Erik Schmauss <erik.schmauss@xxxxxxxxx>
Date: Wed, 15 May 2019 17:25:31 -0700
Subject: [PATCH] Revert "ACPICA: AML interpreter: add region addresses in
 global list during initialization"

This reverts commit f8053df634d40c733f26ca49c2c3835002e61b77 that was
unintentionally included as a part of the stable branch.

Reported-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
Signed-off-by: Erik Schmauss <erik.schmauss@xxxxxxxxx>
---
 drivers/acpi/acpica/dsopcode.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/acpi/acpica/dsopcode.c b/drivers/acpi/acpica/dsopcode.c
index 2f4641e5ecde..78f9de260d5f 100644
--- a/drivers/acpi/acpica/dsopcode.c
+++ b/drivers/acpi/acpica/dsopcode.c
@@ -523,10 +523,6 @@ acpi_ds_eval_table_region_operands(struct acpi_walk_state *walk_state,
                          ACPI_FORMAT_UINT64(obj_desc->region.address),
                          obj_desc->region.length));

-       status = acpi_ut_add_address_range(obj_desc->region.space_id,
-                                          obj_desc->region.address,
-                                          obj_desc->region.length, node);
-
        /* Now the address and length are valid for this opregion */

        obj_desc->region.flags |= AOPOBJ_DATA_VALID;
--
2.17.2




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux