Linux-EDAC Archive mirror
 help / color / mirror / Atom feed
From: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
To: Borislav Petkov <bp@alien8.de>, Tony Luck <tony.luck@intel.com>
Cc: Qiuxu Zhuo <qiuxu.zhuo@intel.com>, Lili Li <lili.li@intel.com>,
	James Morse <james.morse@arm.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Robert Richter <rric@kernel.org>,
	linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/3] EDAC/igen6: Fix slab-use-after-free in igen6_unregister_mci()
Date: Sun,  8 Oct 2023 16:02:29 +0800	[thread overview]
Message-ID: <20231008080231.51917-1-qiuxu.zhuo@intel.com> (raw)

When unloading the igen6_edac driver, the EDAC core wrongly kfreed
'pvt_info,' which was private data and managed by the igen6_edac
driver. This resulted in a slab-use-after-free bug. Fix it by adding
a flag to indicate whether 'pvt_info' is managed by the EDAC core.
The EDAC core will only kfree 'pvt_info' when the flag is set to true.

 BUG: KASAN: slab-use-after-free in igen6_unregister_mcis+0x74/0x1f0 [igen6_edac]
 Read of size 8 at addr ffff88810c10a350 by task modprobe/5137

 Call Trace:
  <TASK>
  dump_stack_lvl+0x4c/0x70
  print_report+0xcf/0x620
  ? __virt_addr_valid+0xfc/0x180
  ? kasan_complete_mode_report_info+0x80/0x210
  ? igen6_unregister_mcis+0x74/0x1f0 [igen6_edac]
  kasan_report+0xbf/0x100
  ? igen6_unregister_mcis+0x74/0x1f0 [igen6_edac]
  __asan_load8+0x82/0xb0
  igen6_unregister_mcis+0x74/0x1f0 [igen6_edac]
  igen6_remove+0x97/0xc0 [igen6_edac]
...
 Allocated by task 578:
  kasan_save_stack+0x2a/0x50
  kasan_set_track+0x29/0x40
  kasan_save_alloc_info+0x1f/0x30
  __kasan_kmalloc+0x88/0xa0
  kmalloc_trace+0x4e/0xb0
  igen6_probe+0xe5/0x1400 [igen6_edac]
  local_pci_probe+0x89/0xf0
  pci_device_probe+0x18e/0x450
...
 Freed by task 5137:
  kasan_save_stack+0x2a/0x50
  kasan_set_track+0x29/0x40
  kasan_save_free_info+0x32/0x50
  __kasan_slab_free+0x113/0x1b0
  slab_free_freelist_hook+0xb1/0x190
  __kmem_cache_free+0x15d/0x280
  kfree+0x7d/0x120
  mci_release+0x24a/0x280
  device_release+0x64/0x110
  kobject_put+0xfd/0x260
  put_device+0x17/0x30
  edac_mc_free+0x43/0x50
  igen6_unregister_mcis+0x18f/0x1f0 [igen6_edac]
  igen6_remove+0x97/0xc0 [igen6_edac]
  pci_device_remove+0x6a/0x100
  device_remove+0x6f/0xb0

Fixes: 0bbb265f7089 ("EDAC/mc: Get rid of silly one-shot struct allocation in edac_mc_alloc()")
Co-developed-by: Lili Li <lili.li@intel.com>
Signed-off-by: Lili Li <lili.li@intel.com>
Tested-by: Lili Li <lili.li@intel.com>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---
 drivers/edac/edac_mc.c | 19 +++++++++++++++----
 include/linux/edac.h   |  5 +++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 6faeb2ab3960..6a68b0225130 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -201,7 +201,14 @@ static void mci_release(struct device *dev)
 		}
 		kfree(mci->csrows);
 	}
-	kfree(mci->pvt_info);
+
+	/*
+	 * if !pvt_managed_by_edac_core, the resource, e.g. memory or MMIO-mapped
+	 * registers pointed by pvt_info is managed by the EDAC	driver. The EDAC
+	 * core shouldn't kfree it.
+	 */
+	if (mci->pvt_managed_by_edac_core)
+		kfree(mci->pvt_info);
 	kfree(mci->layers);
 	kfree(mci);
 }
@@ -369,9 +376,13 @@ struct mem_ctl_info *edac_mc_alloc(unsigned int mc_num,
 	if (!mci->layers)
 		goto error;
 
-	mci->pvt_info = kzalloc(sz_pvt, GFP_KERNEL);
-	if (!mci->pvt_info)
-		goto error;
+	if (sz_pvt) {
+		mci->pvt_info = kzalloc(sz_pvt, GFP_KERNEL);
+		if (!mci->pvt_info)
+			goto error;
+
+		mci->pvt_managed_by_edac_core = true;
+	}
 
 	mci->dev.release = mci_release;
 	device_initialize(&mci->dev);
diff --git a/include/linux/edac.h b/include/linux/edac.h
index fa4bda2a70f6..6f9f4893ba77 100644
--- a/include/linux/edac.h
+++ b/include/linux/edac.h
@@ -567,6 +567,11 @@ struct mem_ctl_info {
 	const char *ctl_name;
 	const char *dev_name;
 	void *pvt_info;
+	/*
+	 * Indicate whether the resource pointed by pvt_info is managed by
+	 * the EDAC core.
+	 */
+	bool pvt_managed_by_edac_core;
 	unsigned long start_time;	/* mci load start time (in jiffies) */
 
 	/*
-- 
2.17.1


             reply	other threads:[~2023-10-08  8:03 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-08  8:02 Qiuxu Zhuo [this message]
2023-10-08  8:02 ` [PATCH 2/3] EDAC/device: Fix potential slab-use-after-free Qiuxu Zhuo
2023-10-08  8:02 ` [PATCH 3/3] EDAC/pci: Fix a potential memory leak Qiuxu Zhuo
2023-10-08 10:57 ` [PATCH 1/3] EDAC/igen6: Fix slab-use-after-free in igen6_unregister_mci() Borislav Petkov
2023-10-09  2:39   ` Zhuo, Qiuxu
2023-10-09  8:50     ` Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231008080231.51917-1-qiuxu.zhuo@intel.com \
    --to=qiuxu.zhuo@intel.com \
    --cc=bp@alien8.de \
    --cc=james.morse@arm.com \
    --cc=lili.li@intel.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rric@kernel.org \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).