RE: [PATCH 20/27] ACPICA: Tables: Fix invalid pointer accesses in acpi_tb_parse_root_table().

[Zheng, Lv]

Hi,

> From: Rafael J. Wysocki [mailto:rjw@xxxxxxxxxxxxx]
> Sent: Tuesday, May 06, 2014 8:44 AM
> 
> On Monday, May 05, 2014 08:42:46 AM Josh Boyer wrote:
> > On Mon, May 5, 2014 at 12:23 AM, Zheng, Lv <lv.zheng@xxxxxxxxx> wrote:
> > > Hi, Rafael
> > >
> > >> From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] On Behalf Of Rafael J. Wysocki
> > >> Sent: Monday, May 05, 2014 8:43 AM
> > >>
> > >> On Saturday, May 03, 2014 08:59:14 AM Josh Boyer wrote:
> > >> > On Tue, Apr 29, 2014 at 10:05 PM, Lv Zheng <lv.zheng@xxxxxxxxx> wrote:
> > >> > > The commit of back porting Linux XSDT validation mechanism has introduced
> > >> > > a regreession:
> > >> > >   Commit: 671cc68dc61f029d44b43a681356078e02d8dab8
> > >> > >   Subject: ACPICA: Back port and refine validation of the XSDT root table.
> > >> > > There is a pointer still accessed after unmapping.
> > >> > >
> > >> > > This patch fixes this issue.  Lv Zheng.
> > >> > >
> > >> > > Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=73911
> > >> > > Buglink: https://bugs.archlinux.org/task/39811
> > >> > > Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx>
> > >> > > Reported-and-tested-by: Bruce Chiarelli <mano155@xxxxxxxxx>
> > >> > > Reported-and-tested-by: Spyros Stathopoulos <spystath@xxxxxxxxx>
> > >> > > Signed-off-by: Bob Moore <robert.moore@xxxxxxxxx>
> > >> > > Cc: <stable@xxxxxxxxxxxxxxx> # 3.14.x: 671cc68: ACPICA: Back port and refine validation of the XSDT root table.
> > >> >
> > >> > This patch should get into 3.15-rcX soon.  It fixes a known regression
> > >> > that prevents booting on machines with AMI firmware, and that is
> > >> > present in 3.14 so we need it for stable as well.  Rafael?
> > >>
> > >> Lv, is it safe to take this patch alone into 3.15-rc?
> > >
> > > Yes, it's safe to only take this patch to be a regression fix.
> >
> > FWIW, Fedora is carrying this on top of 3.14.2 already, and people
> > with the impacted machines say it's working for them.  So I agree it
> > should be safe.
> >
> > Thanks to the both of you.
> 
> OK, queued up for 3.15, thanks!

Actually there are 2 fixes in the same patch set for this issue:
One is a regression fix for commit 671cc68 - let me call it "Solution 1 - ill formed XSDT skipping".
 https://patchwork.kernel.org/patch/4090591/
The other is a different approach to solve the old issue that is fixed by solution 1 - let me call it "Solution 2 - R/XSDT NULL entry skipping".
 https://patchwork.kernel.org/patch/4090511/
 https://patchwork.kernel.org/patch/4090501/

And if you want to know the whole story before making further decisions.

The commit 671cc68 is introduced to reduce the ACPICA divergences.
Lacking in platforms having such ill formed XSDT shipped, I only unit tested the commit in ACPICA development environment where AcpiOsUnmapMemory() is a no-op.
Thus the buggy commit is leaked to the Linux kernel during the ACPICA release cycle.

When fixing the regression here: https://bugzilla.kernel.org/show_bug.cgi?id=73911
At first, the root cause is not addressed due to the same test incapability, thus it comes the solution 2 to solve the old issue in a different way.
As the solution 1 prevents higher versioned tables to be used while solution 2 enables higher versioned tables,
I believe solution 2 is more correct than the old approach.
The solution 2 is based on ACPICA 201403 release, for kernel v3.14, a completely different stable material is generated.

In the meanwhile, the regression is root caused.
Though the regression fix may not be useful if the solution 2 is proven to be more correct,
ACPICA also merged this regression fix in order to generate a stable material for Linux.

I'm not sure if patches for solution 2 also need to be merged using the short-cut path.
Possibly not as the solution 2 changed old behavior of Linux, so they are not urgent stable materials.

Thanks and best regards
-Lv

> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.