This patch series enhances /dev/mem, so that read and write is possible
at any address. The patchset includes actual implementation for x86.
Petr Tesarik (10):
Return EOF on out-of-bounds read from /dev/mem
(un)xlate_dev_mem_ptr: use phys_addr_t for the @phys parameter
x86: translate highmem /dev/mem pointers
ia64: change xlate_dev_mem_ptr's argument to phys_addr_t
valid_phys_addr_range: use phys_addr_t for the @addr parameter
sh: change valid_phys_addr_range's @addr param to phys_addr_t
arm: change valid_phys_addr_range's @addr param to phys_addr_t
ia64: change valid_phys_addr_range's @addr param to phys_addr_t
x86: provide arch-specific valid_phys_addr_range()
Allow reading/writing all memory through /dev/mem
arch/arm/include/asm/io.h | 2 +-
arch/arm/mm/mmap.c | 2 +-
arch/ia64/include/asm/io.h | 2 +-
arch/ia64/include/asm/uaccess.h | 2 +-
arch/ia64/kernel/efi.c | 2 +-
arch/sh/include/asm/io.h | 2 +-
arch/sh/mm/mmap.c | 2 +-
arch/x86/include/asm/io.h | 15 +++++++++++++--
arch/x86/mm/ioremap.c | 24 ++++++++++++++++++------
drivers/char/mem.c | 14 ++++++++++----
10 files changed, 48 insertions(+), 19 deletions(-)
--
1.7.3.4
The off parameter (type loff_t) may specify an offset that cannot
be represented by a long. Currently, /dev/mem wraps around, which
may to cause applications to read/write incorrect regions of memory
by accident.
Follow the usual file semantics here and return 0 when reading or
-EFBIG when writing beyond the accessible range.
Signed-off-by: Petr Tesarik <[email protected]>
---
drivers/char/mem.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 8fc04b4..f5cbd4e 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -97,6 +97,9 @@ static ssize_t read_mem(struct file *file, char __user *buf,
ssize_t read, sz;
char *ptr;
+ if (p != *ppos)
+ return 0;
+
if (!valid_phys_addr_range(p, count))
return -EFAULT;
read = 0;
@@ -155,6 +158,9 @@ static ssize_t write_mem(struct file *file, const char __user *buf,
unsigned long copied;
void *ptr;
+ if (p != *ppos)
+ return -EFBIG;
+
if (!valid_phys_addr_range(p, count))
return -EFAULT;
--
1.7.3.4
To read/write 64-bit physical addresses on 32-bit architectures,
the type must be changed to phys_addr_t. It is also semantically
more exact.
Note that the default xlate_dev_mem_ptr() is defined as a macro,
so no adjustment is needed there.
Signed-off-by: Petr Tesarik <[email protected]>
---
drivers/char/mem.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index f5cbd4e..7d86bda 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -82,7 +82,7 @@ static inline int range_is_allowed(unsigned long pfn, unsigned long size)
}
#endif
-void __weak unxlate_dev_mem_ptr(unsigned long phys, void *addr)
+void __weak unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
{
}
--
1.7.3.4
On 32-bit x86, it is necessary to kmap/kunmap /dev/mem pointers in
the high memory area.
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/x86/include/asm/io.h | 4 ++--
arch/x86/mm/ioremap.c | 24 ++++++++++++++++++------
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index d02804d..9b994dd 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -311,8 +311,8 @@ BUILDIO(b, b, char)
BUILDIO(w, w, short)
BUILDIO(l, , int)
-extern void *xlate_dev_mem_ptr(unsigned long phys);
-extern void unxlate_dev_mem_ptr(unsigned long phys, void *addr);
+extern void *xlate_dev_mem_ptr(phys_addr_t phys);
+extern void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr);
extern int ioremap_change_attr(unsigned long vaddr, unsigned long size,
unsigned long prot_val);
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index be1ef57..fb566d7 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -308,26 +308,38 @@ EXPORT_SYMBOL(iounmap);
* Convert a physical pointer to a virtual kernel pointer for /dev/mem
* access
*/
-void *xlate_dev_mem_ptr(unsigned long phys)
+void *xlate_dev_mem_ptr(phys_addr_t phys)
{
void *addr;
- unsigned long start = phys & PAGE_MASK;
+ unsigned long pfn = phys >> PAGE_SHIFT;
+ phys_addr_t start = phys & ~((phys_addr_t)PAGE_SIZE-1);
/* If page is RAM, we can use __va. Otherwise ioremap and unmap. */
- if (page_is_ram(start >> PAGE_SHIFT))
+ if (page_is_ram(pfn)) {
+ if (phys >= __pa(high_memory))
+ return pfn_valid(pfn)
+ ? kmap(pfn_to_page(pfn))
+ : NULL;
return __va(phys);
+ }
addr = (void __force *)ioremap_cache(start, PAGE_SIZE);
if (addr)
- addr = (void *)((unsigned long)addr | (phys & ~PAGE_MASK));
+ addr = (void *)((unsigned long)addr |
+ ((unsigned long)phys & ~PAGE_MASK));
return addr;
}
-void unxlate_dev_mem_ptr(unsigned long phys, void *addr)
+void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
{
- if (page_is_ram(phys >> PAGE_SHIFT))
+ unsigned long pfn = phys >> PAGE_SHIFT;
+
+ if (page_is_ram(pfn)) {
+ if (phys >= __pa(high_memory))
+ kunmap(pfn_to_page(pfn));
return;
+ }
iounmap((void __iomem *)((unsigned long)addr & PAGE_MASK));
return;
--
1.7.3.4
No further changes are needed on ia64, because phys_addr_t is always
64-bit here, hence the same size as an unsigned long.
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/ia64/include/asm/uaccess.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/ia64/include/asm/uaccess.h b/arch/ia64/include/asm/uaccess.h
index 449c8c0..930717e 100644
--- a/arch/ia64/include/asm/uaccess.h
+++ b/arch/ia64/include/asm/uaccess.h
@@ -366,7 +366,7 @@ ia64_done_with_exception (struct pt_regs *regs)
#define ARCH_HAS_TRANSLATE_MEM_PTR 1
static __inline__ char *
-xlate_dev_mem_ptr (unsigned long p)
+xlate_dev_mem_ptr (phys_addr_t p)
{
struct page *page;
char * ptr;
--
1.7.3.4
The correct type for physical addresses is phys_addr_t. This is in
fact needed to check 64-bit physical addresses on 32-bit architectures
(such as i386 PAE).
Signed-off-by: Petr Tesarik <[email protected]>
---
drivers/char/mem.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 7d86bda..cbbaf36 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -45,7 +45,7 @@ static inline unsigned long size_inside_page(unsigned long start,
}
#ifndef ARCH_HAS_VALID_PHYS_ADDR_RANGE
-static inline int valid_phys_addr_range(unsigned long addr, size_t count)
+static inline int valid_phys_addr_range(phys_addr_t addr, size_t count)
{
return addr + count <= __pa(high_memory);
}
--
1.7.3.4
No other changes are needed, the function will work just fine with
a potentially larger data type.
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/sh/include/asm/io.h | 2 +-
arch/sh/mm/mmap.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sh/include/asm/io.h b/arch/sh/include/asm/io.h
index 28c5aa5..e6d5add 100644
--- a/arch/sh/include/asm/io.h
+++ b/arch/sh/include/asm/io.h
@@ -383,7 +383,7 @@ static inline int iounmap_fixed(void __iomem *addr) { return -EINVAL; }
#define xlate_dev_kmem_ptr(p) p
#define ARCH_HAS_VALID_PHYS_ADDR_RANGE
-int valid_phys_addr_range(unsigned long addr, size_t size);
+int valid_phys_addr_range(phys_addr_t addr, size_t size);
int valid_mmap_phys_addr_range(unsigned long pfn, size_t size);
#endif /* __KERNEL__ */
diff --git a/arch/sh/mm/mmap.c b/arch/sh/mm/mmap.c
index afeb710..80bf494 100644
--- a/arch/sh/mm/mmap.c
+++ b/arch/sh/mm/mmap.c
@@ -238,7 +238,7 @@ bottomup:
* You really shouldn't be using read() or write() on /dev/mem. This
* might go away in the future.
*/
-int valid_phys_addr_range(unsigned long addr, size_t count)
+int valid_phys_addr_range(phys_addr_t addr, size_t count)
{
if (addr < __MEMORY_START)
return 0;
--
1.7.3.4
No other changes are needed, the function will work just fine with
a potentially larger data type.
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/arm/include/asm/io.h | 2 +-
arch/arm/mm/mmap.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index d66605d..91e4ff2 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -290,7 +290,7 @@ extern void pci_iounmap(struct pci_dev *dev, void __iomem *addr);
#ifdef CONFIG_MMU
#define ARCH_HAS_VALID_PHYS_ADDR_RANGE
-extern int valid_phys_addr_range(unsigned long addr, size_t size);
+extern int valid_phys_addr_range(phys_addr_t addr, size_t size);
extern int valid_mmap_phys_addr_range(unsigned long pfn, size_t size);
extern int devmem_is_allowed(unsigned long pfn);
#endif
diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index 74be05f..3bf1d88 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -127,7 +127,7 @@ full_search:
* You really shouldn't be using read() or write() on /dev/mem. This
* might go away in the future.
*/
-int valid_phys_addr_range(unsigned long addr, size_t size)
+int valid_phys_addr_range(phys_addr_t addr, size_t size)
{
if (addr < PHYS_OFFSET)
return 0;
--
1.7.3.4
On ia64, this doesn't change anything, because the size of phys_addr_t
is always the same as the size of unsigned long.
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/ia64/include/asm/io.h | 2 +-
arch/ia64/kernel/efi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h
index e5a6c35..d9d3848 100644
--- a/arch/ia64/include/asm/io.h
+++ b/arch/ia64/include/asm/io.h
@@ -91,7 +91,7 @@ phys_to_virt (unsigned long address)
#define ARCH_HAS_VALID_PHYS_ADDR_RANGE
extern u64 kern_mem_attribute (unsigned long phys_addr, unsigned long size);
-extern int valid_phys_addr_range (unsigned long addr, size_t count); /* efi.c */
+extern int valid_phys_addr_range (phys_addr_t addr, size_t count); /* efi.c */
extern int valid_mmap_phys_addr_range (unsigned long pfn, size_t count);
/*
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index 6fc03af..b12d2b4 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -869,7 +869,7 @@ kern_mem_attribute (unsigned long phys_addr, unsigned long size)
EXPORT_SYMBOL(kern_mem_attribute);
int
-valid_phys_addr_range (unsigned long phys_addr, unsigned long size)
+valid_phys_addr_range (phys_addr_t phys_addr, unsigned long size)
{
u64 attr;
--
1.7.3.4
The default implementation in drivers/char/mem.c refuses to map high
memory, but since xlate_dev_mem_ptr() can now handle high memory on
x86, all physical addresses are valid.
We needn't check the available physical memory (or memory holes) here,
because these cases are handled by xlate_dev_mem_ptr (using pfn_valid).
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/x86/include/asm/io.h | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 9b994dd..e3150fe 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -311,6 +311,17 @@ BUILDIO(b, b, char)
BUILDIO(w, w, short)
BUILDIO(l, , int)
+#define ARCH_HAS_VALID_PHYS_ADDR_RANGE
+static inline int valid_phys_addr_range (phys_addr_t addr, size_t size)
+{
+ return 1;
+}
+
+static inline int valid_mmap_phys_addr_range (unsigned long pfn, size_t size)
+{
+ return 1;
+}
+
extern void *xlate_dev_mem_ptr(phys_addr_t phys);
extern void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr);
--
1.7.3.4
With all pieces in place, I can now change the type of the physical
address pointer and get access to all memory.
Signed-off-by: Petr Tesarik <[email protected]>
---
drivers/char/mem.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index cbbaf36..bc16ab7 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -93,7 +93,7 @@ void __weak unxlate_dev_mem_ptr(phys_addr_t phys, void *addr)
static ssize_t read_mem(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
- unsigned long p = *ppos;
+ phys_addr_t p = *ppos;
ssize_t read, sz;
char *ptr;
@@ -153,7 +153,7 @@ static ssize_t read_mem(struct file *file, char __user *buf,
static ssize_t write_mem(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
- unsigned long p = *ppos;
+ phys_addr_t p = *ppos;
ssize_t written, sz;
unsigned long copied;
void *ptr;
--
1.7.3.4
* Petr Tesarik <[email protected]> wrote:
> This patch series enhances /dev/mem, so that read and write is
> possible at any address. The patchset includes actual
> implementation for x86.
This series lacks a description of why this is desired.
My strong opinion is that it's not desired at all: /dev/mem never
worked beyond 4G addresses so by today it has become largely obsolete
and is on the way out really.
I'm aware of these current /dev/mem uses:
- Xorg maps below 4G non-RAM addresses and the video BIOS
- It used to have some debugging role but these days kexec and kgdb
has largely taken over that role - partly due to the 4G limit.
- there's some really horrible out-of-tree drivers that do mmap()s
via /dev/mem, those should be fixed if they want to move beyond
4G: their char device should be mmap()able.
- all distro kernel's i'm aware of use CONFIG_STRICT_DEVMEM=y, which
restricts /dev/mem to non-RAM pages of physical memory.
[ With the sad inclusion of the first 1MB, which Xorg needs. ]
Are you aware of any legitimate usecases?
Frankly, i dont think we ever *want* to 'fix' /dev/mem to support
addresses beyond 4G and grow messy userspace (and kernelspace) that
somehow relies on that. Thank goodness that we never supported it ...
Thanks,
Ingo
On Fri, Jun 17, 2011 at 11:30:32AM +0200, Ingo Molnar wrote:
> I'm aware of these current /dev/mem uses:
>
> - Xorg maps below 4G non-RAM addresses and the video BIOS
>
> - It used to have some debugging role but these days kexec and kgdb
> has largely taken over that role - partly due to the 4G limit.
>
> - there's some really horrible out-of-tree drivers that do mmap()s
> via /dev/mem, those should be fixed if they want to move beyond
> 4G: their char device should be mmap()able.
>
> - all distro kernel's i'm aware of use CONFIG_STRICT_DEVMEM=y, which
> restricts /dev/mem to non-RAM pages of physical memory.
> [ With the sad inclusion of the first 1MB, which Xorg needs. ]
There's another use case for /dev/mem:
- debugging via devmem2 on embedded platforms, where you want to be able
to boot a kernel, and then peek and poke at MMIO registers either to
verify register values or test things out.
Dne Pá 17. června 2011 11:30:32 Ingo Molnar napsal(a):
> * Petr Tesarik <[email protected]> wrote:
> > This patch series enhances /dev/mem, so that read and write is
> > possible at any address. The patchset includes actual
> > implementation for x86.
>
> This series lacks a description of why this is desired.
Hi Ingo,
> My strong opinion is that it's not desired at all: /dev/mem never
> worked beyond 4G addresses so by today it has become largely obsolete
> and is on the way out really.
>
> I'm aware of these current /dev/mem uses:
>
> - Xorg maps below 4G non-RAM addresses and the video BIOS
>
> - It used to have some debugging role but these days kexec and kgdb
> has largely taken over that role - partly due to the 4G limit.
It is still used as a "memory source" by Dave Anderson's crash utility for
live examination of a running system. Redhat has "overcome" the /dev/mem
deficiencies by writing an out-of-tree re-implementation of /dev/mem, which
uses /dev/crash instead. As it is an "unnecessary duplication of an existing
driver", this method was rejected by the project manager here at SUSE.
The suggested alternative was to enhance (or fix) the existing driver. Without
this patch series there is no way to access high memory. In conjunction with
CONFIG_HIGHPTE, it makes the crash utility near to useless on anything with
high memory, because crash can no longer translate virtual to physical
addresses.
> - there's some really horrible out-of-tree drivers that do mmap()s
> via /dev/mem, those should be fixed if they want to move beyond
> 4G: their char device should be mmap()able.
>
> - all distro kernel's i'm aware of use CONFIG_STRICT_DEVMEM=y, which
> restricts /dev/mem to non-RAM pages of physical memory.
> [ With the sad inclusion of the first 1MB, which Xorg needs. ]
Well, there's one (quite unimportant) distro that doesn't compile with
CONFIG_STRICT_DEVMEM: openSUSE (and SLES, consequently).
> Are you aware of any legitimate usecases?
See above - live crash sessions on i586 (or any other arch that has high
memory).
> Frankly, i dont think we ever *want* to 'fix' /dev/mem to support
> addresses beyond 4G and grow messy userspace (and kernelspace) that
> somehow relies on that. Thank goodness that we never supported it ...
Let me put it differently. Do you think that an out-of-tree duplicated effort
(the crash.ko module) offered by Redhat is somehow superior to enhancing the
in-tree /dev/mem driver?
Regards,
Petr Tesarik
On 17/06/11 19:30, Ingo Molnar wrote:
> * Petr Tesarik<[email protected]> wrote:
>
>> This patch series enhances /dev/mem, so that read and write is
>> possible at any address. The patchset includes actual
>> implementation for x86.
> This series lacks a description of why this is desired.
>
> My strong opinion is that it's not desired at all: /dev/mem never
> worked beyond 4G addresses so by today it has become largely obsolete
> and is on the way out really.
>
> I'm aware of these current /dev/mem uses:
>
> - Xorg maps below 4G non-RAM addresses and the video BIOS
>
> - It used to have some debugging role but these days kexec and kgdb
> has largely taken over that role - partly due to the 4G limit.
>
> - there's some really horrible out-of-tree drivers that do mmap()s
> via /dev/mem, those should be fixed if they want to move beyond
> 4G: their char device should be mmap()able.
There are drivers where this makes sense. For example an FPGA device
with a proprietary register layout on the memory bus can be done this
way. The FPGA can simply be mapped in user-space via /dev/mem and
handled there. If the device requires no access other than memory bus
reads and writes then writing a custom char device driver just to get an
mmap function seems a bit overkill.
~Ryan
On 06/19/2011 04:02 PM, Ryan Mallon wrote:
> On 17/06/11 19:30, Ingo Molnar wrote:
>> * Petr Tesarik<[email protected]> wrote:
>>
>>> This patch series enhances /dev/mem, so that read and write is
>>> possible at any address. The patchset includes actual
>>> implementation for x86.
>> This series lacks a description of why this is desired.
>>
>> My strong opinion is that it's not desired at all: /dev/mem never
>> worked beyond 4G addresses so by today it has become largely obsolete
>> and is on the way out really.
>>
>> I'm aware of these current /dev/mem uses:
>>
>> - Xorg maps below 4G non-RAM addresses and the video BIOS
>>
>> - It used to have some debugging role but these days kexec and kgdb
>> has largely taken over that role - partly due to the 4G limit.
>>
>> - there's some really horrible out-of-tree drivers that do mmap()s
>> via /dev/mem, those should be fixed if they want to move beyond
>> 4G: their char device should be mmap()able.
>
> There are drivers where this makes sense. For example an FPGA device
> with a proprietary register layout on the memory bus can be done this
> way. The FPGA can simply be mapped in user-space via /dev/mem and
> handled there. If the device requires no access other than memory bus
> reads and writes then writing a custom char device driver just to get an
> mmap function seems a bit overkill.
>
There are some test drivers which really want /dev/mem to work.
FPGA devices like that really should be exported as resources from a
platform driver or device tree driver, at which point those resources
can be memory-mapped. That being said, using /dev/mem for fixed
resources is semicommon.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Mon, Jun 20, 2011 at 09:02:17AM +1000, Ryan Mallon wrote:
> There are drivers where this makes sense. For example an FPGA device
> with a proprietary register layout on the memory bus can be done this
> way. The FPGA can simply be mapped in user-space via /dev/mem and
> handled there. If the device requires no access other than memory bus
> reads and writes then writing a custom char device driver just to get an
> mmap function seems a bit overkill.
Calling a 30 line device driver "overkill" might in itself be overkill?
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On 20/06/11 10:42, Matthew Wilcox wrote:
> On Mon, Jun 20, 2011 at 09:02:17AM +1000, Ryan Mallon wrote:
>> There are drivers where this makes sense. For example an FPGA device
>> with a proprietary register layout on the memory bus can be done this
>> way. The FPGA can simply be mapped in user-space via /dev/mem and
>> handled there. If the device requires no access other than memory bus
>> reads and writes then writing a custom char device driver just to get an
>> mmap function seems a bit overkill.
> Calling a 30 line device driver "overkill" might in itself be overkill?
>
I mean overkill in the sense of having to write the driver at all. Why
write a 30 line driver just to re-implement some functionality of /dev/mem?
~Ryan
On Mon, Jun 20, 2011 at 10:46:08AM +1000, Ryan Mallon wrote:
> On 20/06/11 10:42, Matthew Wilcox wrote:
>> On Mon, Jun 20, 2011 at 09:02:17AM +1000, Ryan Mallon wrote:
>>> There are drivers where this makes sense. For example an FPGA device
>>> with a proprietary register layout on the memory bus can be done this
>>> way. The FPGA can simply be mapped in user-space via /dev/mem and
>>> handled there. If the device requires no access other than memory bus
>>> reads and writes then writing a custom char device driver just to get an
>>> mmap function seems a bit overkill.
>> Calling a 30 line device driver "overkill" might in itself be overkill?
>>
> I mean overkill in the sense of having to write the driver at all. Why
> write a 30 line driver just to re-implement some functionality of
> /dev/mem?
Because it pushes the tradeoff in the right direction. Somebody wants
to do something weird is a little inconvenienced vs protecting the vast
majority of users from some security escalation problems.
Besides, if you have a real bus with discoverable regions
(like PCI BARs), the bus should have sysfs entries like
/sys/bus/pci/devices/0000\:06\:06.0/resource0 that can be mmaped.
Then there's no need for a device driver at all, *and* the privilege
escalation isn't achievable.
Of course, most embedded architectures have crap discoverability.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
On 20/06/11 10:52, Matthew Wilcox wrote:
> On Mon, Jun 20, 2011 at 10:46:08AM +1000, Ryan Mallon wrote:
>> On 20/06/11 10:42, Matthew Wilcox wrote:
>>> On Mon, Jun 20, 2011 at 09:02:17AM +1000, Ryan Mallon wrote:
>>>> There are drivers where this makes sense. For example an FPGA device
>>>> with a proprietary register layout on the memory bus can be done this
>>>> way. The FPGA can simply be mapped in user-space via /dev/mem and
>>>> handled there. If the device requires no access other than memory bus
>>>> reads and writes then writing a custom char device driver just to get an
>>>> mmap function seems a bit overkill.
>>> Calling a 30 line device driver "overkill" might in itself be overkill?
>>>
>> I mean overkill in the sense of having to write the driver at all. Why
>> write a 30 line driver just to re-implement some functionality of
>> /dev/mem?
> Because it pushes the tradeoff in the right direction. Somebody wants
> to do something weird is a little inconvenienced vs protecting the vast
> majority of users from some security escalation problems.
How does it protect against security escalation? A process mapping a
region either from /dev/mem or from some custom char device can't escape
that region right? In either case you need root privileges to make the
mapping in the first place.
> Besides, if you have a real bus with discoverable regions
> (like PCI BARs), the bus should have sysfs entries like
> /sys/bus/pci/devices/0000\:06\:06.0/resource0 that can be mmaped.
> Then there's no need for a device driver at all, *and* the privilege
> escalation isn't achievable.
>
> Of course, most embedded architectures have crap discoverability.
Which is also where devices like FPGAs tend to exist :-).
~Ryan
On Fri, Jun 17, 2011 at 5:55 PM, Petr Tesarik <[email protected]> wrote:
> Dne Pá 17. června 2011 11:30:32 Ingo Molnar napsal(a):
>> * Petr Tesarik <[email protected]> wrote:
>> > This patch series enhances /dev/mem, so that read and write is
>> > possible at any address. The patchset includes actual
>> > implementation for x86.
>>
>> This series lacks a description of why this is desired.
>
> Hi Ingo,
>
>> My strong opinion is that it's not desired at all: /dev/mem never
>> worked beyond 4G addresses so by today it has become largely obsolete
>> and is on the way out really.
>>
>> I'm aware of these current /dev/mem uses:
>>
>> - Xorg maps below 4G non-RAM addresses and the video BIOS
>>
>> - It used to have some debugging role but these days kexec and kgdb
>> has largely taken over that role - partly due to the 4G limit.
>
> It is still used as a "memory source" by Dave Anderson's crash utility for
> live examination of a running system. Redhat has "overcome" the /dev/mem
> deficiencies by writing an out-of-tree re-implementation of /dev/mem, which
> uses /dev/crash instead. As it is an "unnecessary duplication of an existing
> driver", this method was rejected by the project manager here at SUSE.
>
> The suggested alternative was to enhance (or fix) the existing driver. Without
> this patch series there is no way to access high memory. In conjunction with
> CONFIG_HIGHPTE, it makes the crash utility near to useless on anything with
> high memory, because crash can no longer translate virtual to physical
> addresses.
>
How about /proc/kcore? AFAIK, it can access highmem, but Dave didn't consider
it for some reason.
* Ryan Mallon <[email protected]> wrote:
> On 17/06/11 19:30, Ingo Molnar wrote:
> >* Petr Tesarik<[email protected]> wrote:
> >
> >>This patch series enhances /dev/mem, so that read and write is
> >>possible at any address. The patchset includes actual
> >>implementation for x86.
> >This series lacks a description of why this is desired.
> >
> >My strong opinion is that it's not desired at all: /dev/mem never
> >worked beyond 4G addresses so by today it has become largely obsolete
> >and is on the way out really.
> >
> >I'm aware of these current /dev/mem uses:
> >
> > - Xorg maps below 4G non-RAM addresses and the video BIOS
> >
> > - It used to have some debugging role but these days kexec and kgdb
> > has largely taken over that role - partly due to the 4G limit.
> >
> > - there's some really horrible out-of-tree drivers that do mmap()s
> > via /dev/mem, those should be fixed if they want to move beyond
> > 4G: their char device should be mmap()able.
>
> There are drivers where this makes sense. For example an FPGA
> device with a proprietary register layout on the memory bus can be
> done this way. [...]
So you want us to help vendors screw users with insane, proprietary,
user-space drivers with sekrit binary blobs?
Wow.
Thanks,
Ingo
* H. Peter Anvin <[email protected]> wrote:
> There are some test drivers which really want /dev/mem to work.
Test drivers and test hardware with zero enumeration can use below 4G
addresses just fine.
Also, debugging is not an issue and some non-default kernel or boot
option can enable any sort of device i guess, so my main worry isnt
really security but that we should stand in the way of the apparent
practice of *shipping* user-space drivers that use /dev/mem ...
Thanks,
Ingo
On 20/06/11 17:31, Ingo Molnar wrote:
> * Ryan Mallon <[email protected]> wrote:
>
>> On 17/06/11 19:30, Ingo Molnar wrote:
>>> * Petr Tesarik<[email protected]> wrote:
>>>
>>>> This patch series enhances /dev/mem, so that read and write is
>>>> possible at any address. The patchset includes actual
>>>> implementation for x86.
>>> This series lacks a description of why this is desired.
>>>
>>> My strong opinion is that it's not desired at all: /dev/mem never
>>> worked beyond 4G addresses so by today it has become largely obsolete
>>> and is on the way out really.
>>>
>>> I'm aware of these current /dev/mem uses:
>>>
>>> - Xorg maps below 4G non-RAM addresses and the video BIOS
>>>
>>> - It used to have some debugging role but these days kexec and kgdb
>>> has largely taken over that role - partly due to the 4G limit.
>>>
>>> - there's some really horrible out-of-tree drivers that do mmap()s
>>> via /dev/mem, those should be fixed if they want to move beyond
>>> 4G: their char device should be mmap()able.
>> There are drivers where this makes sense. For example an FPGA
>> device with a proprietary register layout on the memory bus can be
>> done this way. [...]
> So you want us to help vendors screw users with insane, proprietary,
> user-space drivers with sekrit binary blobs?
>
> Wow.
It's not about that. I mean proprietary in the sense that the register
layout is not based on some open spec and is customised to some
particular usage, not that it is evil, anti-GPL and "sekrit". I have
worked on embedded products which have custom FPGAs for _very_ dedicated
usage. The FPGA firmware is company IP and therefore not open (which is
nothing to do with Linux). The types of products I'm talking about are
often very niche market and dedicate use and therefore not a case of
vendors screwing over the general public. Writing the drivers in
user-space makes development easier; it's not an attempt to hide code
from the public. There is nothing to stop a /dev/mem userspace driver
from being open, just as there is nothing to stop a kernel driver for a
closed platform under Linux being "closed".
~Ryan
> On Mon, Jun 20, 2011 at 10:42:47, Amerigo Wang <[email protected]> wrote:
>> On Fri, Jun 17, 2011 at 5:55 PM, Petr Tesarik <[email protected]> wrote:
>> Dne Pá 17. Äervna 2011 11:30:32 Ingo Molnar napsal(a):
>>> * Petr Tesarik <[email protected]> wrote:
>>> > This patch series enhances /dev/mem, so that read and write is
>>> > possible at any address. The patchset includes actual
>>> > implementation for x86.
>>>
>>> This series lacks a description of why this is desired.
>>
>> Hi Ingo,
>>
>>> My strong opinion is that it's not desired at all: /dev/mem never
>>> worked beyond 4G addresses so by today it has become largely obsolete
>>> and is on the way out really.
>>>
>>> I'm aware of these current /dev/mem uses:
>>>
>>> - Xorg maps below 4G non-RAM addresses and the video BIOS
>>>
>>> - It used to have some debugging role but these days kexec and kgdb
>>> has largely taken over that role - partly due to the 4G limit.
>>
>> It is still used as a "memory source" by Dave Anderson's crash utility for
>> live examination of a running system. Redhat has "overcome" the /dev/mem
>> deficiencies by writing an out-of-tree re-implementation of /dev/mem, which
>> uses /dev/crash instead. As it is an "unnecessary duplication of an existing
>> driver", this method was rejected by the project manager here at SUSE.
>>
>> The suggested alternative was to enhance (or fix) the existing driver. Without
>> this patch series there is no way to access high memory. In conjunction with
>> CONFIG_HIGHPTE, it makes the crash utility near to useless on anything with
>> high memory, because crash can no longer translate virtual to physical
>> addresses.
>>
>
> How about /proc/kcore? AFAIK, it can access highmem, but Dave didn't consider
> it for some reason.
I don't know what you mean by I "didn't consider it", because
the crash utility does support using /proc/kcore as an alternative
live memory source.
The problem is that /proc/kcore can only access highmem if it
is mapped as kernel virtual address. So it cannot read 32-bit
highmem PTE's, user-space memory, etc.
Dave
On 06/20/2011 12:41 AM, Ingo Molnar wrote:
>
> * H. Peter Anvin <[email protected]> wrote:
>
>> There are some test drivers which really want /dev/mem to work.
>
> Test drivers and test hardware with zero enumeration can use below 4G
> addresses just fine.
>
> Also, debugging is not an issue and some non-default kernel or boot
> option can enable any sort of device i guess, so my main worry isnt
> really security but that we should stand in the way of the apparent
> practice of *shipping* user-space drivers that use /dev/mem ...
>
We should either fix /dev/mem to work according to its specification or
rip it out. The issue with test drivers is not spurious... I ran into
this one myself a few weeks ago while trying to do a memory test (by
limiting the amount of memory available to the kernel). This is
something that is typically done on factory floors, and it would be nice
to be able to get those environments over to using Linux.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
* H. Peter Anvin <[email protected]> wrote:
> On 06/20/2011 12:41 AM, Ingo Molnar wrote:
> >
> > * H. Peter Anvin <[email protected]> wrote:
> >
> >> There are some test drivers which really want /dev/mem to work.
> >
> > Test drivers and test hardware with zero enumeration can use below 4G
> > addresses just fine.
> >
> > Also, debugging is not an issue and some non-default kernel or boot
> > option can enable any sort of device i guess, so my main worry isnt
> > really security but that we should stand in the way of the apparent
> > practice of *shipping* user-space drivers that use /dev/mem ...
> >
>
> We should either fix /dev/mem to work according to its
> specification or rip it out. The issue with test drivers is not
> spurious... I ran into this one myself a few weeks ago while trying
> to do a memory test (by limiting the amount of memory available to
> the kernel). This is something that is typically done on factory
> floors, and it would be nice to be able to get those environments
> over to using Linux.
There is no reason why they couldnt use a .config option or a boot
option to get their weird stuff going, which weird stuff also happens
to be useful.
What i'm somewhat against is having this enabled by default for weird
stuff that also happens to be harmful - and the fact that it never
worked over 4G physical gives us the perfect opportunity to do just
that.
Thanks,
Ingo
On 06/20/2011 09:40 AM, Ingo Molnar wrote:
>
> There is no reason why they couldnt use a .config option or a boot
> option to get their weird stuff going, which weird stuff also happens
> to be useful.
>
> What i'm somewhat against is having this enabled by default for weird
> stuff that also happens to be harmful - and the fact that it never
> worked over 4G physical gives us the perfect opportunity to do just
> that.
>
I'm concerned about that notion. I think it's fine to modularize
/dev/mem, but what we're seeing is that Red Hat and all kinds of other
entities are putting in ad hoc versions of /dev/mem, but of course doing
so incorrectly.
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
On Sun, Jun 19, 2011 at 4:02 PM, Ryan Mallon <[email protected]> wrote:
> On 17/06/11 19:30, Ingo Molnar wrote:
>> - there's some really horrible out-of-tree drivers that do mmap()s
>> via /dev/mem, those should be fixed if they want to move beyond
>> 4G: their char device should be mmap()able.
>
> There are drivers where this makes sense. For example an FPGA device with a
> proprietary register layout on the memory bus can be done this way. The FPGA
> can simply be mapped in user-space via /dev/mem and handled there. If the
> device requires no access other than memory bus reads and writes then
> writing a custom char device driver just to get an mmap function seems a bit
> overkill.
While VFIO is still out-of-tree, it handles this use-case, as well as
interrupts and transparent DMA remapping via the IOMMU. I'm using it
on a current project and it works as advertised. UIO is in-tree and
also handles this (as I understand it), but without DMA or interrupt
support.
https://github.com/pugs/vfio-linux-2.6/blob/vfio/Documentation/vfio.txt
It's perhaps arguable whether any human with taste should ship a
driver based on UIO/VFIO, but for code that exists solely to exercise
and test a hardware design, it works great.
~r.
于 2011年06月20日 21:24, Dave Anderson 写道:
>> On Mon, Jun 20, 2011 at 10:42:47, Amerigo Wang<[email protected]> wrote:
>>> On Fri, Jun 17, 2011 at 5:55 PM, Petr Tesarik<[email protected]> wrote:
>>> Dne Pá 17. Äervna 2011 11:30:32 Ingo Molnar napsal(a):
>>>> * Petr Tesarik<[email protected]> wrote:
>>>>> This patch series enhances /dev/mem, so that read and write is
>>>>> possible at any address. The patchset includes actual
>>>>> implementation for x86.
>>>>
>>>> This series lacks a description of why this is desired.
>>>
>>> Hi Ingo,
>>>
>>>> My strong opinion is that it's not desired at all: /dev/mem never
>>>> worked beyond 4G addresses so by today it has become largely obsolete
>>>> and is on the way out really.
>>>>
>>>> I'm aware of these current /dev/mem uses:
>>>>
>>>> - Xorg maps below 4G non-RAM addresses and the video BIOS
>>>>
>>>> - It used to have some debugging role but these days kexec and kgdb
>>>> has largely taken over that role - partly due to the 4G limit.
>>>
>>> It is still used as a "memory source" by Dave Anderson's crash utility for
>>> live examination of a running system. Redhat has "overcome" the /dev/mem
>>> deficiencies by writing an out-of-tree re-implementation of /dev/mem, which
>>> uses /dev/crash instead. As it is an "unnecessary duplication of an existing
>>> driver", this method was rejected by the project manager here at SUSE.
>>>
>>> The suggested alternative was to enhance (or fix) the existing driver. Without
>>> this patch series there is no way to access high memory. In conjunction with
>>> CONFIG_HIGHPTE, it makes the crash utility near to useless on anything with
>>> high memory, because crash can no longer translate virtual to physical
>>> addresses.
>>>
>>
>> How about /proc/kcore? AFAIK, it can access highmem, but Dave didn't consider
>> it for some reason.
>
> I don't know what you mean by I "didn't consider it", because
> the crash utility does support using /proc/kcore as an alternative
> live memory source.
Sorry, I recall that in our last discussion you didn't
mention this. But it is good to know that crash supports this!
>
> The problem is that /proc/kcore can only access highmem if it
> is mapped as kernel virtual address. So it cannot read 32-bit
> highmem PTE's, user-space memory, etc.
>
Hmm, looking at the code, you are right, it only has
low memory in kernel space.
But what's the problem of adding highmem into kcore? :-/
Thanks.
On Mon, Jun 20, 2011 at 08:59:20AM -0700, H. Peter Anvin wrote:
> We should either fix /dev/mem to work according to its specification or
> rip it out. The issue with test drivers is not spurious... I ran into
> this one myself a few weeks ago while trying to do a memory test (by
> limiting the amount of memory available to the kernel). This is
> something that is typically done on factory floors, and it would be nice
> to be able to get those environments over to using Linux.
We came across a similar request from our manufacturing folks. They have a
memory stress tool that needs to test how much bandwidth is possible between
various cpus and memory modules. Essentially they want the tool to run on some
chosen CPU and want it to drive traffic to some chosen memory module.
/dev/mem would be handy for such test programs ..
- vatsa
----- Original Message -----
> 于 2011年06月20日 21:24, Dave Anderson 写道:
> >> On Mon, Jun 20, 2011 at 10:42:47, Amerigo Wang<[email protected]>
> >
> > I don't know what you mean by I "didn't consider it", because
> > the crash utility does support using /proc/kcore as an alternative
> > live memory source.
>
> Sorry, I recall that in our last discussion you didn't
> mention this. But it is good to know that crash supports this!
>
> >
> > The problem is that /proc/kcore can only access highmem if it
> > is mapped as kernel virtual address. So it cannot read 32-bit
> > highmem PTE's, user-space memory, etc.
> >
>
> Hmm, looking at the code, you are right, it only has
> low memory in kernel space.
>
> But what's the problem of adding highmem into kcore? :-/
>
> Thanks.
/proc/kcore was presumably designed for use by gdb with a vmlinux
namelist, and like user-space core dumps, the PT_LOAD segments
reference virtual address regions with p_vaddr/p_paddr pairs.
Dave
Américo Wang píše v Po 20. 06. 2011 v 10:42 +0800:
> On Fri, Jun 17, 2011 at 5:55 PM, Petr Tesarik <[email protected]> wrote:
> > Dne Pá 17. června 2011 11:30:32 Ingo Molnar napsal(a):
> >> * Petr Tesarik <[email protected]> wrote:
> >> > This patch series enhances /dev/mem, so that read and write is
> >> > possible at any address. The patchset includes actual
> >> > implementation for x86.
> >>
> >> This series lacks a description of why this is desired.
> >
> > Hi Ingo,
> >
> >> My strong opinion is that it's not desired at all: /dev/mem never
> >> worked beyond 4G addresses so by today it has become largely obsolete
> >> and is on the way out really.
> >>
> >> I'm aware of these current /dev/mem uses:
> >>
> >> - Xorg maps below 4G non-RAM addresses and the video BIOS
> >>
> >> - It used to have some debugging role but these days kexec and kgdb
> >> has largely taken over that role - partly due to the 4G limit.
> >
> > It is still used as a "memory source" by Dave Anderson's crash utility for
> > live examination of a running system. Redhat has "overcome" the /dev/mem
> > deficiencies by writing an out-of-tree re-implementation of /dev/mem, which
> > uses /dev/crash instead. As it is an "unnecessary duplication of an existing
> > driver", this method was rejected by the project manager here at SUSE.
> >
> > The suggested alternative was to enhance (or fix) the existing driver. Without
> > this patch series there is no way to access high memory. In conjunction with
> > CONFIG_HIGHPTE, it makes the crash utility near to useless on anything with
> > high memory, because crash can no longer translate virtual to physical
> > addresses.
> >
>
> How about /proc/kcore? AFAIK, it can access highmem, but Dave didn't consider
> it for some reason.
(Sorry for the dealy, I was on vacation.)
/proc/kcore is in fact supported by the crash utility and it does work
with highmem to some extent. Unfortunately, it cannot handle all of it,
because it is ELF32 format, so there's no way to describe regions beyond
4GB.
Petr Tesarik
Dne Pá 17. června 2011 11:30:32 Ingo Molnar napsal(a):
> * Petr Tesarik <[email protected]> wrote:
> > This patch series enhances /dev/mem, so that read and write is
> > possible at any address. The patchset includes actual
> > implementation for x86.
>
> This series lacks a description of why this is desired.
>[...]
>
> Are you aware of any legitimate usecases?
Looking back at the mail tread, I'd say there are people who have legitimate
usecases. However, this may not be the most important question. At the moment,
the /dev/mem interface is broken (it doesn't implement the specification
correctly), and my patchset fixes it.
If there are no technical objections, I'd like to ask for an Acked-by from all
involved people.
@Ingo: you can also send a patchset that rips off the /dev/mem driver
completely if you believe that would get through. I'm completely fine with
that, because then the /dev/crash driver will no longer be a semi-broken re-
implementation of an existing in-kernel driver, so I'll be able to post it as
a new driver.
But please make a decision either way.
Petr Tesarik