From patchwork Mon Oct 14 17:19:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Morse X-Patchwork-Id: 11189217 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id C26D814DB for ; Mon, 14 Oct 2019 17:19:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A43C221925 for ; Mon, 14 Oct 2019 17:19:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388111AbfJNRTb (ORCPT ); Mon, 14 Oct 2019 13:19:31 -0400 Received: from foss.arm.com ([217.140.110.172]:49590 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387802AbfJNRTb (ORCPT ); Mon, 14 Oct 2019 13:19:31 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D2C231000; Mon, 14 Oct 2019 10:19:30 -0700 (PDT) Received: from eglon.cambridge.arm.com (eglon.cambridge.arm.com [10.1.196.105]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EB9F03F6C4; Mon, 14 Oct 2019 10:19:29 -0700 (PDT) From: James Morse To: linux-edac@vger.kernel.org Cc: Mauro Carvalho Chehab , Borislav Petkov , Tony Luck , Robert Richter , John Garry Subject: [PATCH 1/2] EDAC, ghes: Fix Use after free in ghes_edac remove path Date: Mon, 14 Oct 2019 18:19:18 +0100 Message-Id: <20191014171919.85044-2-james.morse@arm.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191014171919.85044-1-james.morse@arm.com> References: <20191014171919.85044-1-james.morse@arm.com> MIME-Version: 1.0 Sender: linux-edac-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-edac@vger.kernel.org ghes_edac models a single logical memory controller, and uses a global ghes_init variable to ensure only the first ghes_edac_register() will do anything. ghes_edac is registered the first time a GHES entry in the HEST is probed. There may be multiple entries, so subsequent attempts to register ghes_edac are silently ignored as the work has already been done. When a GHES entry is unregistered, it calls ghes_edac_unregister(), which free()s the memory behind the global variables in ghes_edac. ... but there may be multiple GHES entries, the next call to ghes_edac_unregister() will dereference the free()d memory, and attempt to free it a second time. This may also be triggered on a platform with one GHES entry, if the driver is unbound/re-bound and unbound. The re-bind step will do nothing because of ghes_init, the second unbind will then do the same work as the first. This was detected by KASAN and DEBUG_TEST_DRIVER_REMOVE. Reported-by: John Garry Link: lore.kernel.org/r/304df85b-8b56-b77e-1a11-aa23769f2e7c@huawei.com Signed-off-by: James Morse Fixes: 0fe5f281f749 ("EDAC, ghes: Model a single, logical memory controller") --- drivers/edac/ghes_edac.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c index d413a0bdc9ad..955b59b6aade 100644 --- a/drivers/edac/ghes_edac.c +++ b/drivers/edac/ghes_edac.c @@ -554,6 +554,7 @@ void ghes_edac_unregister(struct ghes *ghes) return; mci = ghes_pvt->mci; + ghes_pvt = NULL; edac_mc_del_mc(mci->pdev); edac_mc_free(mci); } From patchwork Mon Oct 14 17:19:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Morse X-Patchwork-Id: 11189219 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3C85A17D4 for ; Mon, 14 Oct 2019 17:19:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 27FB521848 for ; Mon, 14 Oct 2019 17:19:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388277AbfJNRTd (ORCPT ); Mon, 14 Oct 2019 13:19:33 -0400 Received: from foss.arm.com ([217.140.110.172]:49600 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387802AbfJNRTd (ORCPT ); Mon, 14 Oct 2019 13:19:33 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DBE4A1576; Mon, 14 Oct 2019 10:19:32 -0700 (PDT) Received: from eglon.cambridge.arm.com (eglon.cambridge.arm.com [10.1.196.105]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id F3DDD3F6C4; Mon, 14 Oct 2019 10:19:31 -0700 (PDT) From: James Morse To: linux-edac@vger.kernel.org Cc: Mauro Carvalho Chehab , Borislav Petkov , Tony Luck , Robert Richter , John Garry Subject: [PATCH 2/2] EDAC, ghes: Reference count GHES users of ghes_edac Date: Mon, 14 Oct 2019 18:19:19 +0100 Message-Id: <20191014171919.85044-3-james.morse@arm.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191014171919.85044-1-james.morse@arm.com> References: <20191014171919.85044-1-james.morse@arm.com> MIME-Version: 1.0 Sender: linux-edac-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-edac@vger.kernel.org ghes_edac only does work for the first GHES entry that registers it. The same is true for unregister: first come first served. This lack of symmetry is problematic if we have more than one GHES entry, as ghes_edac_register() can only be called once, nothing decrements ghes_init. Doing the unregister work on the first call is unsafe, as another CPU may be processing a notification in ghes_edac_report_mem_error(), using the memory we are about to free. ghes_init is already half of the reference counting. We only need to do the register work for the first call, and the unregister work for the last. Add the unregister check. This means we no longer free ghes_edac's memory while there are GHES entries that may receive a notification. Signed-off-by: James Morse Fixes: 0fe5f281f749 ("EDAC, ghes: Model a single, logical memory controller") --- drivers/edac/ghes_edac.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c index 955b59b6aade..0bb62857ffb2 100644 --- a/drivers/edac/ghes_edac.c +++ b/drivers/edac/ghes_edac.c @@ -553,6 +553,9 @@ void ghes_edac_unregister(struct ghes *ghes) if (!ghes_pvt) return; + if (atomic_dec_return(&ghes_init)) + return; + mci = ghes_pvt->mci; ghes_pvt = NULL; edac_mc_del_mc(mci->pdev);