mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Masayoshi Mizuma <msys.mizuma@gmail.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Borislav Petkov <bp@alien8.de>,
	linux-edac@vger.kernel.org
Cc: Masayoshi Mizuma <msys.mizuma@gmail.com>,
	linux-kernel@vger.kernel.org,
	Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
Subject: [PATCH] EDAC, sb_edac: Fix out of bound in PCI multi segment env
Date: Thu, 19 Jul 2018 10:07:45 -0400	[thread overview]
Message-ID: <20180719140745.4503-1-msys.mizuma@gmail.com> (raw)

From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

KASAN reported the following slab-out-of-bounds when sb_edac
module was loaded on Broadwell machine which has two PCI segments.

==================================================================
BUG: KASAN: slab-out-of-bounds in sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac]
Read of size 8 at addr ffff8c0d44dfe850 by task modprobe/4221

CPU: 19 PID: 4221 Comm: modprobe Not tainted 4.18.0-rc5 #2
Call Trace:
 dump_stack+0xc2/0x16b
 ? show_regs_print_info+0x5/0x5
 ? kmsg_dump_rewind_nolock+0xd9/0xd9
 ? pci_get_dev_by_id+0x57/0x70
 ? pci_get_device+0x155/0x210
 print_address_description+0x6a/0x270
 kasan_report+0x258/0x380
 ? sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac]
 sbridge_get_all_devices.constprop.14+0x75f/0x96a [sb_edac]
...
Allocated by task 4221:
 kasan_kmalloc+0xa0/0xd0
 __kmalloc+0x112/0x230
 sbridge_get_all_devices.constprop.14+0x555/0x96a [sb_edac]
 sbridge_init+0x1ba/0x4000 [sb_edac]
 do_one_initcall+0xaa/0x401
 do_init_module+0x20b/0x676
 load_module+0x443f/0x6dc0
 __do_sys_finit_module+0x25f/0x2c0
 do_syscall_64+0x14e/0x4b0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
...
==================================================================

The out bounds happened because the index of sbridge_dev->pdev[] is over
than the allocated size.
---
static int sbridge_get_onedevice(struct pci_dev **prev,
...
        if (sbridge_dev->pdev[sbridge_dev->i_devs]) { <= !! out bounds HERE !!
                sbridge_printk(KERN_ERR,
                        "Duplicated device for %04x:%04x\n",
                        PCI_VENDOR_ID_INTEL, dev_descr->dev_id);
---

If a sbridge_dev which has the same bus as the new device is already exists,
the sbridge_dev is assigned to the new one.

---
static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bus,
                                           struct sbridge_dev *prev)
...
        list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) {
                if (sbridge_dev->bus == bus && (dom == SOCK || dom == sbridge_dev->dom))
                        return sbridge_dev;
        }
---

If the system has multi PCI segment and the each bus number is same,
the same sbridge_dev is assigned. So, the index of sbridge_dev->pdev[]
is incremented by each segments.
As the result, the index is over than the allocated size, then the
out-of-bounds happens.

This patch introduces a segment member to sbridge_dev to separate
it each PCI segments.

Fixes: e2f747b1f42a ("EDAC, sb_edac: Assign EDAC memory controller per h/w controller")

Signed-off-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
---
 drivers/edac/sb_edac.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 4a89c80..07726fb 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -352,6 +352,7 @@ struct pci_id_table {
 
 struct sbridge_dev {
 	struct list_head	list;
+	int			seg;
 	u8			bus, mc;
 	u8			node_id, source_id;
 	struct pci_dev		**pdev;
@@ -729,7 +730,8 @@ static inline int numcol(u32 mtr)
 	return 1 << cols;
 }
 
-static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bus,
+static struct sbridge_dev *get_sbridge_dev(int seg, u8 bus, enum domain dom,
+					   int multi_bus,
 					   struct sbridge_dev *prev)
 {
 	struct sbridge_dev *sbridge_dev;
@@ -747,14 +749,15 @@ static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int multi_bu
 				      : sbridge_edac_list.next, struct sbridge_dev, list);
 
 	list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) {
-		if (sbridge_dev->bus == bus && (dom == SOCK || dom == sbridge_dev->dom))
+		if ((sbridge_dev->seg == seg) && (sbridge_dev->bus == bus) &&
+				(dom == SOCK || dom == sbridge_dev->dom))
 			return sbridge_dev;
 	}
 
 	return NULL;
 }
 
-static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
+static struct sbridge_dev *alloc_sbridge_dev(int seg, u8 bus, enum domain dom,
 					     const struct pci_id_table *table)
 {
 	struct sbridge_dev *sbridge_dev;
@@ -771,6 +774,7 @@ static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom,
 		return NULL;
 	}
 
+	sbridge_dev->seg = seg;
 	sbridge_dev->bus = bus;
 	sbridge_dev->dom = dom;
 	sbridge_dev->n_devs = table->n_devs_per_imc;
@@ -2246,6 +2250,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 	struct sbridge_dev *sbridge_dev = NULL;
 	const struct pci_id_descr *dev_descr = &table->descr[devno];
 	struct pci_dev *pdev = NULL;
+	int seg = 0;
 	u8 bus = 0;
 	int i = 0;
 
@@ -2276,10 +2281,12 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 		/* End of list, leave */
 		return -ENODEV;
 	}
+	seg = pci_domain_nr(pdev->bus);
 	bus = pdev->bus->number;
 
 next_imc:
-	sbridge_dev = get_sbridge_dev(bus, dev_descr->dom, multi_bus, sbridge_dev);
+	sbridge_dev = get_sbridge_dev(seg, bus, dev_descr->dom,
+				      multi_bus, sbridge_dev);
 	if (!sbridge_dev) {
 		/* If the HA1 wasn't found, don't create EDAC second memory controller */
 		if (dev_descr->dom == IMC1 && devno != 1) {
@@ -2292,7 +2299,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev,
 		if (dev_descr->dom == SOCK)
 			goto out_imc;
 
-		sbridge_dev = alloc_sbridge_dev(bus, dev_descr->dom, table);
+		sbridge_dev = alloc_sbridge_dev(seg, bus, dev_descr->dom, table);
 		if (!sbridge_dev) {
 			pci_dev_put(pdev);
 			return -ENOMEM;
-- 
2.18.0


             reply	other threads:[~2018-07-19 14:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-19 14:07 Masayoshi Mizuma [this message]
2018-07-24 17:01 ` Tony Luck
2018-07-24 18:41   ` Masayoshi Mizuma

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=20180719140745.4503-1-msys.mizuma@gmail.com \
    --to=msys.mizuma@gmail.com \
    --cc=bp@alien8.de \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.mizuma@jp.fujitsu.com \
    --cc=mchehab@kernel.org \
    /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

all inboxes | Powered by JetHome®