linux-kernel - [PATCH 1/8] ACPI: Fix acpi_os_read_memory() and acpi_os_write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 25 Jan 2011 00:26:47 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Len Brown <lenb@...nel.org>
Cc:	Jeff Chua <jeff.chua.linux@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	"Linux-pm mailing list" <linux-pm@...ts.linux-foundation.org>,
	Matthew Garrett <mjg59@...f.ucam.org>
Subject: [PATCH 1/8] ACPI: Fix acpi_os_read_memory() and acpi_os_write_memory() (v2)

From: Rafael J. Wysocki <rjw@...k.pl>

The functions acpi_os_read_memory() and acpi_os_write_memory() do
two wrong things.  First, they shouldn't call rcu_read_unlock()
before the looked up address is actually used for I/O, because in
that case the iomap it belongs to may be removed before the I/O
is done.  Second, if they have to create a new mapping, they should
check the returned virtual address and tell the caller that the
operation failed if it is NULL (in fact, I think they even should not
attempt to map an address that's not present in one of the existing
ACPI iomaps, because that may cause problems to happen when they are
called from nonpreemptible context and their callers ought to know
what they are doing and map the requisite memory regions beforehand).

Make these functions call rcu_read_unlock() when the I/O is complete
(or if it's necessary to map the given address "on the fly") and
return an error code if the requested physical address is not present
in the existing ACPI iomaps and cannot be mapped.

Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
---
 drivers/acpi/osl.c |   25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/acpi/osl.c
===================================================================
--- linux-2.6.orig/drivers/acpi/osl.c
+++ linux-2.6/drivers/acpi/osl.c
@@ -636,17 +636,21 @@ EXPORT_SYMBOL(acpi_os_write_port);
 acpi_status
 acpi_os_read_memory(acpi_physical_address phys_addr, u32 * value, u32 width)
 {
-	u32 dummy;
 	void __iomem *virt_addr;
-	int size = width / 8, unmap = 0;
+	unsigned int size = width / 8;
+	bool unmap = false;
+	u32 dummy;
 
 	rcu_read_lock();
 	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
-	rcu_read_unlock();
 	if (!virt_addr) {
+		rcu_read_unlock();
 		virt_addr = acpi_os_ioremap(phys_addr, size);
-		unmap = 1;
+		if (!virt_addr)
+			return AE_BAD_ADDRESS;
+		unmap = true;
 	}
+
 	if (!value)
 		value = &dummy;
 
@@ -666,6 +670,8 @@ acpi_os_read_memory(acpi_physical_addres
 
 	if (unmap)
 		iounmap(virt_addr);
+	else
+		rcu_read_unlock();
 
 	return AE_OK;
 }
@@ -674,14 +680,17 @@ acpi_status
 acpi_os_write_memory(acpi_physical_address phys_addr, u32 value, u32 width)
 {
 	void __iomem *virt_addr;
-	int size = width / 8, unmap = 0;
+	unsigned int size = width / 8;
+	bool unmap = false;
 
 	rcu_read_lock();
 	virt_addr = acpi_map_vaddr_lookup(phys_addr, size);
-	rcu_read_unlock();
 	if (!virt_addr) {
+		rcu_read_unlock();
 		virt_addr = acpi_os_ioremap(phys_addr, size);
-		unmap = 1;
+		if (!virt_addr)
+			return AE_BAD_ADDRESS;
+		unmap = true;
 	}
 
 	switch (width) {
@@ -700,6 +709,8 @@ acpi_os_write_memory(acpi_physical_addre
 
 	if (unmap)
 		iounmap(virt_addr);
+	else
+		rcu_read_unlock();
 
 	return AE_OK;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/