From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9FCDAC433E0 for ; Thu, 18 Mar 2021 15:43:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7648A64F2B for ; Thu, 18 Mar 2021 15:43:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230367AbhCRPnG (ORCPT ); Thu, 18 Mar 2021 11:43:06 -0400 Received: from mail-ot1-f46.google.com ([209.85.210.46]:34424 "EHLO mail-ot1-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231841AbhCRPmd (ORCPT ); Thu, 18 Mar 2021 11:42:33 -0400 Received: by mail-ot1-f46.google.com with SMTP id k14-20020a9d7dce0000b02901b866632f29so5613276otn.1; Thu, 18 Mar 2021 08:42:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=MSzbL7q7UOHy8XukE1pAVuaN95+QmKI6ybgop91aXbk=; b=ANlYDidrioCB7tNaBCxDGmIfMCsD0wenZXnpauJoKeC9QqLP+C7GNMjwfc4Yk/pgjG eH1D8f5dlZNdlJOpGkQQrUcPBV2Eidf15Fdy+PFTDyZ+Bzl+ohO3hfNgpciC999dq420 qhd9ITxX8+vE7cO0irjs6Jg3uNiAs5m45dQiDKTkgxEetekpO8qp1PN6t903AjASggcw LG2A3RXLH4QOdNFbdhtkoSeNzMXLq3sFI6hJhbnKyBMg2lX+nMyLoG12BTmVipY2yIUH 1jy7cS/TzPNhsepEfHf5A2XEBE2TYqIqsEB6BWaX+qpw9MCumF5hVBdv+fdJ0zKDNSAG bPrw== X-Gm-Message-State: AOAM532SxSD3enol9/kMd2aGZgIopvLi36M+m2M9YFfdkIBjyDmjiFXE tE8cxCwBMixKvnHq3GZ6Fa0rhwnBkic6ar0QhGM= X-Google-Smtp-Source: ABdhPJw9CbqKs+8q9CJPeZk/eqMne0374o6AJM6grGChTntRlJqy7ruxZhdfrwTKhrfgAYU8cT2aiydfv+IKHBoN0VQ= X-Received: by 2002:a05:6830:20d2:: with SMTP id z18mr5393994otq.260.1616082153103; Thu, 18 Mar 2021 08:42:33 -0700 (PDT) MIME-Version: 1.0 References: <3236337.DtqTXxM43S@kreacher> In-Reply-To: From: "Rafael J. Wysocki" Date: Thu, 18 Mar 2021 16:42:21 +0100 Message-ID: Subject: Re: [PATCH 1/1] ACPI: fix acpi table use after free To: George Kennedy Cc: "Rafael J. Wysocki" , Mike Rapoport , Erik Kaneda , "Rafael J. Wysocki" , David Hildenbrand , Robert Moore , Rafael Wysocki , Len Brown , ACPI Devel Maling List , "open list:ACPI COMPONENT ARCHITECTURE (ACPICA)" , Linux Kernel Mailing List , Konrad Rzeszutek Wilk , Dan Carpenter , Dhaval Giani , Andrew Morton , Vlastimil Babka , Oscar Salvador , Wei Yang , Pankaj Gupta , Michal Hocko Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On Wed, Mar 17, 2021 at 11:28 PM George Kennedy wrote: > > > > On 3/17/2021 4:14 PM, Rafael J. Wysocki wrote: > > On Monday, March 15, 2021 5:19:29 PM CET Rafael J. Wysocki wrote: > >> On Sun, Mar 14, 2021 at 8:00 PM Mike Rapoport wrote: > >>> On Thu, Mar 11, 2021 at 04:36:31PM +0100, Rafael J. Wysocki wrote: > >>>> On Wed, Mar 10, 2021 at 8:47 PM David Hildenbrand wrote: > >>>>>> There is some care that should be taken to make sure we get the order > >>>>>> right, but I don't see a fundamental issue here. > >>>> Me neither. > >>>> > >>>>>> If I understand correctly, Rafael's concern is about changing the parts of > >>>>>> ACPICA that should be OS agnostic, so I think we just need another place to > >>>>>> call memblock_reserve() rather than acpi_tb_install_table_with_override(). > >>>> Something like this. > >>>> > >>>> There is also the problem that memblock_reserve() needs to be called > >>>> for all of the tables early enough, which will require some reordering > >>>> of the early init code. > >>>> > >>>>>> Since the reservation should be done early in x86::setup_arch() (and > >>>>>> probably in arm64::setup_arch()) we might just have a function that parses > >>>>>> table headers and reserves them, similarly to how we parse the tables > >>>>>> during KASLR setup. > >>>> Right. > >>> I've looked at it a bit more and we do something like the patch below that > >>> nearly duplicates acpi_tb_parse_root_table() which is not very nice. > >> It looks to me that the code need not be duplicated (see below). > >> > >>> Besides, reserving ACPI tables early and then calling acpi_table_init() > >>> (and acpi_tb_parse_root_table() again would mean doing the dance with > >>> early_memremap() twice for no good reason. > >> That'd be simply inefficient which is kind of acceptable to me to start with. > >> > >> And I changing the ACPICA code can be avoided at least initially, it > >> by itself would be a good enough reason. > >> > >>> I believe the most effective way to deal with this would be to have a > >>> function that does parsing, reservation and installs the tables supplied by > >>> the firmware which can be called really early and then another function > >>> that overrides tables if needed a some later point. > >> I agree that this should be the direction to go into. > > So maybe something like the patch below? > > Do you want me to try it out in the failing setup? Yes, please! From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============8874661952942508003==" MIME-Version: 1.0 From: Rafael J. Wysocki Subject: [Devel] Re: [PATCH 1/1] ACPI: fix acpi table use after free Date: Thu, 18 Mar 2021 16:42:21 +0100 Message-ID: In-Reply-To: bad1ea33-1a6e-bd4d-b795-fbc51ac6ecca@oracle.com List-ID: To: devel@acpica.org --===============8874661952942508003== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Wed, Mar 17, 2021 at 11:28 PM George Kennedy wrote: > > > > On 3/17/2021 4:14 PM, Rafael J. Wysocki wrote: > > On Monday, March 15, 2021 5:19:29 PM CET Rafael J. Wysocki wrote: > >> On Sun, Mar 14, 2021 at 8:00 PM Mike Rapoport w= rote: > >>> On Thu, Mar 11, 2021 at 04:36:31PM +0100, Rafael J. Wysocki wrote: > >>>> On Wed, Mar 10, 2021 at 8:47 PM David Hildenbrand wrote: > >>>>>> There is some care that should be taken to make sure we get the or= der > >>>>>> right, but I don't see a fundamental issue here. > >>>> Me neither. > >>>> > >>>>>> If I understand correctly, Rafael's concern is about changing the = parts of > >>>>>> ACPICA that should be OS agnostic, so I think we just need another= place to > >>>>>> call memblock_reserve() rather than acpi_tb_install_table_with_ove= rride(). > >>>> Something like this. > >>>> > >>>> There is also the problem that memblock_reserve() needs to be called > >>>> for all of the tables early enough, which will require some reorderi= ng > >>>> of the early init code. > >>>> > >>>>>> Since the reservation should be done early in x86::setup_arch() (a= nd > >>>>>> probably in arm64::setup_arch()) we might just have a function tha= t parses > >>>>>> table headers and reserves them, similarly to how we parse the tab= les > >>>>>> during KASLR setup. > >>>> Right. > >>> I've looked at it a bit more and we do something like the patch below= that > >>> nearly duplicates acpi_tb_parse_root_table() which is not very nice. > >> It looks to me that the code need not be duplicated (see below). > >> > >>> Besides, reserving ACPI tables early and then calling acpi_table_init= () > >>> (and acpi_tb_parse_root_table() again would mean doing the dance with > >>> early_memremap() twice for no good reason. > >> That'd be simply inefficient which is kind of acceptable to me to star= t with. > >> > >> And I changing the ACPICA code can be avoided at least initially, it > >> by itself would be a good enough reason. > >> > >>> I believe the most effective way to deal with this would be to have a > >>> function that does parsing, reservation and installs the tables suppl= ied by > >>> the firmware which can be called really early and then another functi= on > >>> that overrides tables if needed a some later point. > >> I agree that this should be the direction to go into. > > So maybe something like the patch below? > > Do you want me to try it out in the failing setup? Yes, please! --===============8874661952942508003==--