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: Thursday, May 16, 2019 8:36 AM
> To: Schmauss, Erik <erik.schmauss@xxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: 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/16/2019 2:45 AM, Schmauss, Erik wrote:
> >
> >> -----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-addres
> >>>>>> se
> >>>>>> 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
> 
> Thanks Erik, LGTM.
> 

Thanks for your patience with these patches!

Erik





[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