Re: [PATCH 1/4] Prep for paravirt: Be careful about touching BIOS address space
flat view
From: Rusty Russell <hidden>
Date: 2006-10-30 02:35:07
Also in:
lkml
On Fri, 2006-10-27 at 21:50 -0700, Andrew Morton wrote:
On Fri, 27 Oct 2006 21:33:08 -0700 Jeremy Fitzhardinge [off-list ref] wrote:quoted
Andrew Morton wrote:quoted
It'd be better to use include/linux/uaccess.h:probe_kernel_address() for this operation.Ah, yes, that was the precedent I was thinking of,We've done open-coded __get_user() in various places in the past. The difference with probe_kernel_address() is that it doesn't get deadlocked on mmap_sem().quoted
but I guess it would be better to just use it directly. It's a relatively new interface, isn't it?Yeah. New enough that nobody's tried using it on non-x86 ;) It needs to do set_fs(KERNEL_DS).
And the function name is misleading: it really does get a value, not merely probe an address. And the arguments are reversed from __get_user, just to add fun. Andrew, please replace prep-for-paravirt-be-careful-about-touching-bios-warning-fix.patch Subject: Be careful about touching BIOS address space BIOS ROM areas may not be mapped into the guest address space, so be careful when touching those addresses to make sure they appear to be mapped. At Andrew's request, fix probe_kernel_address for non-x86. Signed-off-by: Jeremy Fitzhardinge <redacted> Signed-off-by: Rusty Russell <redacted> (modified) diff -r 9a6c8ceba677 arch/i386/kernel/setup.c
--- a/arch/i386/kernel/setup.c Mon Oct 30 11:34:30 2006 +1100
+++ b/arch/i386/kernel/setup.c Mon Oct 30 13:15:33 2006 +1100@@ -47,6 +47,7 @@ #include <linux/crash_dump.h> #include <linux/dmi.h> #include <linux/pfn.h> +#include <linux/uaccess.h> #include <video/edid.h>
@@ -270,7 +271,14 @@ static struct resource standard_io_resou .flags = IORESOURCE_BUSY | IORESOURCE_IO } }; -#define romsignature(x) (*(unsigned short *)(x) == 0xaa55) +static inline int romsignature(const unsigned char *x) +{ + unsigned short sig; + int ret = 0; + if (probe_kernel_address((const unsigned short *)x, sig) == 0) + ret = (sig == 0xaa55); + return ret; +} static int __init romchecksum(unsigned char *rom, unsigned long length) {
diff -r 9a6c8ceba677 arch/i386/pci/pcbios.c
--- a/arch/i386/pci/pcbios.c Mon Oct 30 11:34:30 2006 +1100
+++ b/arch/i386/pci/pcbios.c Mon Oct 30 13:15:02 2006 +1100@@ -5,6 +5,7 @@ #include <linux/pci.h> #include <linux/init.h> #include <linux/module.h> +#include <linux/uaccess.h> #include "pci.h" #include "pci-functions.h"
@@ -314,6 +315,10 @@ static struct pci_raw_ops * __devinit pc for (check = (union bios32 *) __va(0xe0000); check <= (union bios32 *) __va(0xffff0); ++check) { + long sig; + if (probe_kernel_address(&check->fields.signature, sig)) + continue; + if (check->fields.signature != BIOS32_SIGNATURE) continue; length = check->fields.length * 16;
diff -r 9a6c8ceba677 include/linux/uaccess.h
--- a/include/linux/uaccess.h Mon Oct 30 11:34:30 2006 +1100
+++ b/include/linux/uaccess.h Mon Oct 30 13:10:39 2006 +1100@@ -34,10 +34,13 @@ static inline unsigned long __copy_from_ #define probe_kernel_address(addr, retval) \ ({ \ long ret; \ + mm_segment_t old_fs = get_fs(); \ \ + set_fs(KERNEL_DS); \ inc_preempt_count(); \ ret = __get_user(retval, addr); \ dec_preempt_count(); \ + set_fs(old_fs); \ ret; \ })