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

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: "Schmauss, Erik" <erik.schmauss@intel.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>,
	"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: Possible mis-backport of 4abb951b in 4.19.35 ("ACPICA: AML interpreter: add region addresses...")
Date: Wed, 15 May 2019 06:57:11 +0200	[thread overview]
Message-ID: <20190515045711.GA16452@kroah.com> (raw)
In-Reply-To: <CF6A88132359CE47947DB4C6E1709ED53C5CF0B6@ORSMSX121.amr.corp.intel.com>

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