All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Garry <john.garry@huawei.com>
To: <jdelvare@suse.com>, <linux@roeck-us.net>, <bhelgaas@google.com>,
	<rafael@kernel.org>, <arnd@arndb.de>, <lorenzo.pieralisi@arm.com>,
	<bp@suse.de>
Cc: <linux-hwmon@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-pci@vger.kernel.org>, <wangkefeng.wang@huawei.com>,
	<linuxarm@huawei.com>, John Garry <john.garry@huawei.com>
Subject: [RFC PATCH 1/2] resource: Request IO port regions from children of ioport_resource
Date: Fri, 15 Mar 2019 00:55:15 +0800	[thread overview]
Message-ID: <1552582516-70855-2-git-send-email-john.garry@huawei.com> (raw)
In-Reply-To: <1552582516-70855-1-git-send-email-john.garry@huawei.com>

Currently when we request an IO port region, the request is made directly
to the top resource, ioport_resource.

There is an issue here, in that drivers may request an IO port region even
if the IO port region has not even been mapped in (in pci_remap_iospace()).

This may lead to crashes when the PCI host has not enumerated prior to
accessing the IO port region, as below:

root@(none)$ insmod f71805f.ko
[   32.264016] Unable to handle kernel paging request at virtual address ffff7dfffee0002e
[   32.279936] Mem abort info:
[   32.285536]   ESR = 0x96000046
[   32.291657]   Exception class = DABT (current EL), IL = 32 bits
[   32.303542]   SET = 0, FnV = 0
[   32.309661]   EA = 0, S1PTW = 0
[   32.315957] Data abort info:
[   32.321728]   ISV = 0, ISS = 0x00000046
[   32.329420]   CM = 0, WnR = 1
[   32.335367] swapper pgtable: 4k pages, 48-bit VAs, pgdp = (____ptrval____)
[   32.349174] [ffff7dfffee0002e] pgd=0000000001392003, pud=0000000001393003, pmd=0000000000000000
[   32.366652] Internal error: Oops: 96000046 [#1] PREEMPT SMP
[   32.377835] Modules linked in: f71805f(+)
[   32.385876] CPU: 4 PID: 2681 Comm: insmod Not tainted 5.0.0-00003-ga6247cc0f8f2 #52
[   32.401252] Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018
[   32.419597] pstate: 80000005 (Nzcv daif -PAN -UAO)
[   32.429213] pc : logic_outb+0x54/0xb8
[   32.436556] lr : f71805f_find+0x2c/0x1b8 [f71805f]
[   32.446167] sp : ffff0000256bba90
[   32.452808] x29: ffff0000256bba90 x28: ffff000008b544d0
[   32.463468] x27: ffff0000256bbdf0 x26: 0000000000000100
[   32.474127] x25: 000000000000002c x24: ffff000011396000
[   32.484787] x23: ffff0000256bbb3e x22: ffff0000256bbb40
[   32.495446] x21: ffff000008b591b8 x20: 0000000000000087
[   32.506106] x19: 000000000000002e x18: ffffffffffffffff
[   32.516765] x17: 0000000000000000 x16: 0000000000000000
[   32.527424] x15: 0000000000000400 x14: 0000000000000400
[   32.538084] x13: 0000000000001832 x12: 0000000000000000
[   32.548743] x11: ffff801ffbf08ac0 x10: 0000801fead02000
[   32.559403] x9 : ffff801ffbffe840 x8 : 0000000000000001
[   32.570062] x7 : 0000000000210d00 x6 : 0000000000000000
[   32.580721] x5 : ffff801fb3eac380 x4 : ffff801ffbef4b20
[   32.591381] x3 : 0000000000ffbffe x2 : ffff0000256bbb40
[   32.602040] x1 : ffff7dfffee0002e x0 : ffff7dfffee00000
[   32.612701] Process insmod (pid: 2681, stack limit = 0x(____ptrval____))
[   32.626155] Call trace:
[   32.631050]  logic_outb+0x54/0xb8
[   32.637693]  f71805f_find+0x2c/0x1b8 [f71805f]
[   32.646607]  f71805f_init+0x38/0xe48 [f71805f]
[   32.655521]  do_one_initcall+0x5c/0x178
[   32.663212]  do_init_module+0x58/0x1b0
[   32.670728]  load_module+0x1dc8/0x2178
[   32.678243]  __se_sys_init_module+0x14c/0x1e8
[   32.686981]  __arm64_sys_init_module+0x18/0x20
[   32.695894]  el0_svc_common+0x60/0x100
[   32.703409]  el0_svc_handler+0x2c/0x80
[   32.710924]  el0_svc+0x8/0xc
[   32.716693] Code: d2bfdc00 f2cfbfe0 f2ffffe0 8b000021 (39000034)
[   32.728925] ---[ end trace ddb5e493ee686685 ]---
Segmentation fault
root@(none)$

This issue was originally reported in [1].

This patch changes the functionality of request_region() to request a
region of the direct children of the top ioport_resource.

In this, if the IO port region has not been mapped for a particular IO
region, a suitable child region will not exist, and, as such,
request_region() calls will fail.

A side note: there are many drivers in the kernel which fail to even call
request_region() prior to IO port accesses, and they also need to be fixed
(to call request_region() as appropriate).

[1] https://www.spinics.net/lists/linux-pci/msg49821.html

Signed-off-by: John Garry <john.garry@huawei.com>
---
 include/linux/ioport.h |  6 +++++-
 kernel/resource.c      | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index da0ebaec25f0..cf40e1ed8211 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -217,7 +217,7 @@ static inline bool resource_contains(struct resource *r1, struct resource *r2)
 
 
 /* Convenience shorthand with allocation */
-#define request_region(start,n,name)		__request_region(&ioport_resource, (start), (n), (name), 0)
+#define request_region(start,n,name)		__request_region_from_children(&ioport_resource, (start), (n), (name), 0)
 #define request_muxed_region(start,n,name)	__request_region(&ioport_resource, (start), (n), (name), IORESOURCE_MUXED)
 #define __request_mem_region(start,n,name, excl) __request_region(&iomem_resource, (start), (n), (name), excl)
 #define request_mem_region(start,n,name) __request_region(&iomem_resource, (start), (n), (name), 0)
@@ -230,6 +230,10 @@ extern struct resource * __request_region(struct resource *,
 					resource_size_t n,
 					const char *name, int flags);
 
+extern struct resource * __request_region_from_children(struct resource *,
+							resource_size_t start,
+							resource_size_t n,
+							const char *name, int flags);
 /* Compatibility cruft */
 #define release_region(start,n)	__release_region(&ioport_resource, (start), (n))
 #define release_mem_region(start,n)	__release_region(&iomem_resource, (start), (n))
diff --git a/kernel/resource.c b/kernel/resource.c
index 915c02e8e5dd..4a4656bd6051 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1098,6 +1098,25 @@ resource_size_t resource_alignment(struct resource *res)
 
 static DECLARE_WAIT_QUEUE_HEAD(muxed_resource_wait);
 
+struct resource * __request_region_from_children(struct resource *parent,
+						 resource_size_t start,
+						 resource_size_t n,
+						 const char *name, int flags)
+{
+	struct resource *child;
+
+	/* TODO - add locking */
+	for (child = parent->child; child; child = child->sibling) {
+		struct resource *res = __request_region(child, start, n, name, flags);
+
+		if (res)
+			return res;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL(__request_region_from_children);
+
 /**
  * __request_region - create a new busy resource region
  * @parent: parent resource descriptor
-- 
2.17.1


  reply	other threads:[~2019-03-14 16:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-14 16:55 [RFC PATCH 0/2] Fix system crash for accessing unmapped IO port regions John Garry
2019-03-14 16:55 ` John Garry [this message]
2019-03-14 17:12   ` [RFC PATCH 1/2] resource: Request IO port regions from children of ioport_resource Guenter Roeck
2019-03-14 17:39     ` John Garry
2019-03-14 16:55 ` [RFC PATCH 2/2] hwmon: (f71805f): Use request_region() in f71805f_init() John Garry
2019-03-14 17:05   ` Guenter Roeck
2019-03-14 17:21     ` John Garry
2019-03-14 17:44       ` Guenter Roeck
2019-03-15 11:20         ` John Garry
2019-03-15 12:58           ` Guenter Roeck

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=1552582516-70855-2-git-send-email-john.garry@huawei.com \
    --to=john.garry@huawei.com \
    --cc=arnd@arndb.de \
    --cc=bhelgaas@google.com \
    --cc=bp@suse.de \
    --cc=jdelvare@suse.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=linuxarm@huawei.com \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=rafael@kernel.org \
    --cc=wangkefeng.wang@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.