| From eec24848fca678e4f359e39207fe827b96e2778e Mon Sep 17 00:00:00 2001 |
| From: James Morse <james.morse@arm.com> |
| Date: Mon, 14 Oct 2019 18:19:18 +0100 |
| Subject: [PATCH] EDAC/ghes: Fix Use after free in ghes_edac remove path |
| |
| commit 1e72e673b9d102ff2e8333e74b3308d012ddf75b upstream. |
| |
| 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. |
| |
| 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. |
| |
| This was detected by KASAN and DEBUG_TEST_DRIVER_REMOVE. |
| |
| [ bp: merge into a single patch. ] |
| |
| Fixes: 0fe5f281f749 ("EDAC, ghes: Model a single, logical memory controller") |
| Reported-by: John Garry <john.garry@huawei.com> |
| Signed-off-by: James Morse <james.morse@arm.com> |
| Signed-off-by: Borislav Petkov <bp@suse.de> |
| Cc: linux-edac <linux-edac@vger.kernel.org> |
| Cc: Mauro Carvalho Chehab <mchehab@kernel.org> |
| Cc: Robert Richter <rrichter@marvell.com> |
| Cc: Tony Luck <tony.luck@intel.com> |
| Cc: <stable@vger.kernel.org> |
| Link: https://lkml.kernel.org/r/20191014171919.85044-2-james.morse@arm.com |
| Link: https://lkml.kernel.org/r/304df85b-8b56-b77e-1a11-aa23769f2e7c@huawei.com |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c |
| index 7f19f1c672c3..2059e43ccc01 100644 |
| --- a/drivers/edac/ghes_edac.c |
| +++ b/drivers/edac/ghes_edac.c |
| @@ -553,7 +553,11 @@ 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); |
| edac_mc_free(mci); |
| } |
| -- |
| 2.7.4 |
| |