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

[Sasha Levin <sashal@kernel.org>]

On Thu, May 16, 2019 at 12:45:54AM +0000, Schmauss, Erik wrote:
>
>
>> -----Original Message-----
>> From: Wysocki, Rafael J
>> Sent: Wednesday, May 15, 2019 1:57 PM
>> To: Schmauss, Erik <erik.schmauss@intel.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Paul Gortmaker
>> <paul.gortmaker@windriver.com>; stable@vger.kernel.org
>> 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@linuxfoundation.org]
>> >>> Sent: Monday, May 6, 2019 1:42 AM
>> >>> To: Paul Gortmaker <paul.gortmaker@windriver.com>; Wysocki, Rafael J
>> >>> <rafael.j.wysocki@intel.com>
>> >>> Cc: stable@vger.kernel.org; Schmauss, Erik <erik.schmauss@intel.com>
>> >>> 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@intel.com>
>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.

So to clarify, we're now just going to revert it? It's not needed in
-stable?

--
Thanks,
Sasha