* Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") @ 2019-05-05 19:44 Paul Gortmaker 2019-05-06 8:41 ` Greg Kroah-Hartman 0 siblings, 1 reply; 10+ messages in thread From: Paul Gortmaker @ 2019-05-05 19:44 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: stable, Rafael J. Wysocki, Erik Schmauss 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-global-list-during-initialization.patch ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-in.patch ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-global-list-during-initialization.patch ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-global-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. Paul. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-05 19:44 Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") Paul Gortmaker @ 2019-05-06 8:41 ` Greg Kroah-Hartman 2019-05-15 1:17 ` Schmauss, Erik 0 siblings, 1 reply; 10+ messages in thread From: Greg Kroah-Hartman @ 2019-05-06 8:41 UTC (permalink / raw) To: Paul Gortmaker, Rafael J. Wysocki; +Cc: stable, Erik Schmauss 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-global-list-during-initialization.patch > ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-in.patch > ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-global-list-during-initialization.patch > ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-global-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. > Ugh, Rafael, did I mess this up again? Can you check to see if I need to fix this somehow? thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-06 8:41 ` Greg Kroah-Hartman @ 2019-05-15 1:17 ` Schmauss, Erik 2019-05-15 4:57 ` Greg Kroah-Hartman 0 siblings, 1 reply; 10+ messages in thread From: Schmauss, Erik @ 2019-05-15 1:17 UTC (permalink / raw) To: Greg Kroah-Hartman, Paul Gortmaker, Wysocki, Rafael J; +Cc: stable > -----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-globa > > l-list-during-initialization.patch > > ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-i > > n.patch > > ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-glob > > al-list-during-initialization.patch > > ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-globa > > 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. Thanks, Erik > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-15 1:17 ` Schmauss, Erik @ 2019-05-15 4:57 ` Greg Kroah-Hartman 2019-05-15 20:56 ` Rafael J. Wysocki 0 siblings, 1 reply; 10+ messages in thread From: Greg Kroah-Hartman @ 2019-05-15 4:57 UTC (permalink / raw) To: Schmauss, Erik; +Cc: Paul Gortmaker, Wysocki, Rafael J, stable 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-globa > > > l-list-during-initialization.patch > > > ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-i > > > n.patch > > > ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-glob > > > al-list-during-initialization.patch > > > ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-globa > > > 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? thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-15 4:57 ` Greg Kroah-Hartman @ 2019-05-15 20:56 ` Rafael J. Wysocki 2019-05-16 0:45 ` Schmauss, Erik 0 siblings, 1 reply; 10+ messages in thread From: Rafael J. Wysocki @ 2019-05-15 20:56 UTC (permalink / raw) To: Schmauss, Erik; +Cc: Greg Kroah-Hartman, Paul Gortmaker, stable 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-globa >>>> l-list-during-initialization.patch >>>> ./releases/4.19.3/revert-acpica-aml-interpreter-add-region-addresses-i >>>> n.patch >>>> ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-in-glob >>>> al-list-during-initialization.patch >>>> ./releases/4.19.2/acpica-aml-interpreter-add-region-addresses-in-globa >>>> 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? ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-15 20:56 ` Rafael J. Wysocki @ 2019-05-16 0:45 ` Schmauss, Erik 2019-05-16 15:36 ` Rafael J. Wysocki 2019-05-17 0:52 ` Sasha Levin 0 siblings, 2 replies; 10+ messages in thread From: Schmauss, Erik @ 2019-05-16 0:45 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Paul Gortmaker, stable, Wysocki, Rafael J > -----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. Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com> Signed-off-by: Erik Schmauss <erik.schmauss@intel.com> --- 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-16 0:45 ` Schmauss, Erik @ 2019-05-16 15:36 ` Rafael J. Wysocki 2019-05-16 17:21 ` Schmauss, Erik 2019-05-17 0:52 ` Sasha Levin 1 sibling, 1 reply; 10+ messages in thread From: Rafael J. Wysocki @ 2019-05-16 15:36 UTC (permalink / raw) To: Schmauss, Erik, Greg Kroah-Hartman; +Cc: Paul Gortmaker, stable 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@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. > > Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com> > Signed-off-by: Erik Schmauss <erik.schmauss@intel.com> > --- > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-16 15:36 ` Rafael J. Wysocki @ 2019-05-16 17:21 ` Schmauss, Erik 0 siblings, 0 replies; 10+ messages in thread From: Schmauss, Erik @ 2019-05-16 17:21 UTC (permalink / raw) To: Wysocki, Rafael J, Greg Kroah-Hartman; +Cc: Paul Gortmaker, stable > -----Original Message----- > From: Wysocki, Rafael J > Sent: Thursday, May 16, 2019 8:36 AM > To: Schmauss, Erik <erik.schmauss@intel.com>; Greg Kroah-Hartman > <gregkh@linuxfoundation.org> > Cc: 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/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@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-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@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. > > > > Reported-by: Paul Gortmaker <paul.gortmaker@windriver.com> > > Signed-off-by: Erik Schmauss <erik.schmauss@intel.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-16 0:45 ` Schmauss, Erik 2019-05-16 15:36 ` Rafael J. Wysocki @ 2019-05-17 0:52 ` Sasha Levin 2019-05-17 21:15 ` Schmauss, Erik 1 sibling, 1 reply; 10+ messages in thread From: Sasha Levin @ 2019-05-17 0:52 UTC (permalink / raw) To: Schmauss, Erik Cc: Greg Kroah-Hartman, Paul Gortmaker, stable, Wysocki, Rafael J 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") 2019-05-17 0:52 ` Sasha Levin @ 2019-05-17 21:15 ` Schmauss, Erik 0 siblings, 0 replies; 10+ messages in thread From: Schmauss, Erik @ 2019-05-17 21:15 UTC (permalink / raw) To: Sasha Levin; +Cc: Greg Kroah-Hartman, Paul Gortmaker, stable, Wysocki, Rafael J > -----Original Message----- > From: Sasha Levin [mailto:sashal@kernel.org] > Sent: Thursday, May 16, 2019 5:52 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; Wysocki, Rafael > J <rafael.j.wysocki@intel.com> > Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML > interpreter: add region addresses...") > > 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-addre > >> >>>> sse > >> >>>> s-i > >> >>>> n.patch > >> >>>> ./releases/4.19.35/acpica-aml-interpreter-add-region-addresses-i > >> >>>> n-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? Yes, we're going to revert a commit that got added by accident to the 4.19.y branch. It may not apply to all stable branches because the reverted commit might not exist in all stable branches. I hope this helps, Erik > > -- > Thanks, > Sasha ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-17 21:15 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-05 19:44 Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...") Paul Gortmaker 2019-05-06 8:41 ` Greg Kroah-Hartman 2019-05-15 1:17 ` Schmauss, Erik 2019-05-15 4:57 ` Greg Kroah-Hartman 2019-05-15 20:56 ` Rafael J. Wysocki 2019-05-16 0:45 ` Schmauss, Erik 2019-05-16 15:36 ` Rafael J. Wysocki 2019-05-16 17:21 ` Schmauss, Erik 2019-05-17 0:52 ` Sasha Levin 2019-05-17 21:15 ` Schmauss, Erik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).