From patchwork Mon Apr 3 19:00:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ross Zwisler X-Patchwork-Id: 9660365 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 915A16032D for ; Mon, 3 Apr 2017 19:00:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 858D2284C2 for ; Mon, 3 Apr 2017 19:00:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7A792284F5; Mon, 3 Apr 2017 19:00:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 21F73284C2 for ; Mon, 3 Apr 2017 19:00:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753138AbdDCTAm (ORCPT ); Mon, 3 Apr 2017 15:00:42 -0400 Received: from mga14.intel.com ([192.55.52.115]:11715 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752159AbdDCTAj (ORCPT ); Mon, 3 Apr 2017 15:00:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1491246039; x=1522782039; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=Ji0rnFEHTuEqr3AvTSDt03IJ9NkXM+xAcaYIA8qElxs=; b=CKKQ7A//5l0sCYcmuvx1JsVOmwAd8rswcMB9AJKyNBW5Jsuqc2KbbLvC 1rZ7SUeFvLGbFQ1hTYuuxL0FIUmFNg==; Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Apr 2017 12:00:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,271,1486454400"; d="scan'208";a="841350080" Received: from theros.lm.intel.com (HELO linux.intel.com) ([10.232.112.77]) by FMSMGA003.fm.intel.com with ESMTP; 03 Apr 2017 12:00:14 -0700 Date: Mon, 3 Apr 2017 13:00:14 -0600 From: Ross Zwisler To: Dan Williams Cc: linux-nvdimm@lists.01.org, linux-acpi@vger.kernel.org, Ross Zwisler , Lv Zheng , stable@vger.kernel.org Subject: Re: [PATCH] acpi, nfit: fix acpi_get_table leak Message-ID: <20170403190014.GA20892@linux.intel.com> References: <149107472042.16205.8935477753191459036.stgit@dwillia2-desk3.amr.corp.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <149107472042.16205.8935477753191459036.stgit@dwillia2-desk3.amr.corp.intel.com> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Apr 01, 2017 at 12:25:20PM -0700, Dan Williams wrote: > Calls to acpi_get_table() must be paired with acpi_put_table() to undo > the mapping established by acpi_tb_acquire_table(). > > Cc: > Fixes: 6b11d1d67713 ("ACPI / osl: Remove acpi_get_table_with_size()/early_acpi_os_unmap_memory() users") > Cc: Lv Zheng > Reported-by: Ross Zwisler > Signed-off-by: Dan Williams > --- > drivers/acpi/nfit/core.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index c8ea9d698cd0..6acfea69f061 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -2818,6 +2818,11 @@ void acpi_nfit_desc_init(struct acpi_nfit_desc *acpi_desc, struct device *dev) > } > EXPORT_SYMBOL_GPL(acpi_nfit_desc_init); > > +static void acpi_nfit_put_table(void *table) > +{ > + acpi_put_table(table); > +} > + > static int acpi_nfit_add(struct acpi_device *adev) > { > struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL }; > @@ -2834,6 +2839,10 @@ static int acpi_nfit_add(struct acpi_device *adev) > dev_dbg(dev, "failed to find NFIT at startup\n"); > return 0; > } > + > + rc = devm_add_action_or_reset(dev, acpi_nfit_put_table, tbl); > + if (rc) > + return rc; > sz = tbl->length; > > acpi_desc = devm_kzalloc(dev, sizeof(*acpi_desc), GFP_KERNEL); I've been looking at this this as well, and I think it might be better to just drop the reference immediately in acpi_nfit_add() after we're done processing the tables? Anyway, here's the patch I was working on, but I think either works. ------ 8< ------ From 4819cd889b08c3c17f8f9fcf895a083f21fa0898 Mon Sep 17 00:00:00 2001 From: Ross Zwisler Date: Mon, 3 Apr 2017 11:53:32 -0600 Subject: [PATCH] nfit: release reference on ACPI nfit table Currently acpi_nfit_add() grabs a reference to the ACPI NFIT table structures via acpi_get_table(), but never releases it with acpi_put_table(). This means that the refcount protecting the ioremap for the NFIT table never decrements, so the ioremap can never be undone. The ioremap happens via this path: acpi_get_table() acpi_tb_get_table() acpi_tb_validate_table() acpi_tb_acquire_table() acpi_os_map_memory() In practice this fix is correct but won't have a usable visible impact because the ACPI sysfs code (acpi_table_attr_init() et al.), which populates /sys/firmware/acpi/tables/NFIT, also keeps a table reference, so the NFIT table memory will always remain mapped. We drop the refcount at the end of acpi_nfit_add() instead of waiting till driver unload and doing it via acpi_nfit_remove() or something akin to acpi_nfit_destruct() called via the devm_ interface. This is because during acpi_nfit_add() we never actually keep any references to the original ACPI tables. We either copy individual elements out, or we make whole copies of tables in functions like add_spa(), add memdev(), etc. Signed-off-by: Ross Zwisler --- drivers/acpi/nfit/core.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 662036b..ad0dfd6 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -2833,8 +2833,10 @@ static int acpi_nfit_add(struct acpi_device *adev) sz = tbl->length; acpi_desc = devm_kzalloc(dev, sizeof(*acpi_desc), GFP_KERNEL); - if (!acpi_desc) - return -ENOMEM; + if (!acpi_desc) { + rc = -ENOMEM; + goto out; + } acpi_nfit_desc_init(acpi_desc, &adev->dev); /* Save the acpi header for exporting the revision via sysfs */ @@ -2857,6 +2859,8 @@ static int acpi_nfit_add(struct acpi_device *adev) rc = acpi_nfit_init(acpi_desc, (void *) tbl + sizeof(struct acpi_table_nfit), sz - sizeof(struct acpi_table_nfit)); +out: + acpi_put_table(tbl); return rc; }