devmem/kmem/kcore fixes, cleanups and hwpoison checks
Subsystems (9)
cover devmem/kmem/kcore fixes, cleanups and hwpoison checks
Andrew,
Here are some patches on mem/kmem/kcore.
Most of them have been individually reviewed in LKML.
bug fixes
[PATCH 1/8] vfs: fix too big f_pos handling [PATCH 2/8] devmem: check vmalloc address on kmem read/write [PATCH 3/8] devmem: fix kmem write bug on memory holes
simplify vread/vwrite
[PATCH 4/8] resources: introduce generic page_is_ram() [PATCH 5/8] vmalloc: simplify vread()/vwrite()
check for corrupted page
[PATCH 6/8] hwpoison: prevent /dev/kmem from accessing hwpoison pages [PATCH 7/8] hwpoison: prevent /dev/mem from accessing hwpoison pages [PATCH 8/8] hwpoison: prevent /dev/kcore from accessing hwpoison pages
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 1/8 vfs: fix too big f_pos handling
From: KAMEZAWA Hiroyuki <[email protected]>
Now, rw_verify_area() checsk f_pos is negative or not. And if negative, returns -EINVAL.
But, some special files as /dev/(k)mem and /proc/<pid>/mem etc.. has negative offsets. And we can't do any access via read/write to the file(device).
This patch introduce a flag S_VERYBIG and allow negative file offsets.
Changelog: v4->v5
- clean up patches dor /dev/mem.
- rebased onto 2.6.32-rc1
Changelog: v3->v4
- make changes in mem.c aligned.
- change __negative_fpos_check() to return int.
- fixed bug in "pos" check.
- added comments.
Changelog: v2->v3
- fixed bug in rw_verify_area (it cannot be compiled)
--- drivers/char/mem.c | 4 ++++ fs/proc/base.c | 2 ++ fs/read_write.c | 22 ++++++++++++++++++++-- include/linux/fs.h | 2 ++ 4 files changed, 28 insertions(+), 2 deletions(-)
fs/read_write.c +20 -2
@@ -205,6 +205,21 @@ bad: }#endif +static int +__negative_fpos_check (struct inode *inode,loff_t pos,size_t count) +{ +/* +* pos or pos+count is negative here, check overflow. +* too big "count" will be caught in rw_verify_area(). +*/ +if ((pos <0 ) && (pos + count < pos)) +return -EOVERFLOW ; +/* If !VERYBIG inode, negative pos(pos+count) is not allowed */ +if (!IS_VERYBIG (inode)) +return -EINVAL ; +return 0 ; +} +/* * rw_verify_area doesn't like huge counts. We limit * them to something that fits in "int" so that others
@@ -222,8 +237,11 @@ int rw_verify_area(int read_write, strucif (unlikely ((ssize_t ) count <0 ))return retval; pos = *ppos; -if (unlikely ((pos <0 ) || (loff_t) (pos + count) <0 )) -return retval; +if (unlikely ((pos <0 ) || (loff_t) (pos + count) <0 )) { + retval =__negative_fpos_check (inode, pos, count); +if (retval) +return retval; + }if (unlikely (inode->i_flock &&mandatory_lock (inode))) { retval = locks_mandatory_area(
include/linux/fs.h +2 -0
@@ -235,6 +235,7 @@ struct inodes_stat_t {#define S_NOCMTIME 128/* Do not update file c/mtime */ #define S_SWAPFILE 256/* Do not truncate: swapon got its bmaps */ #define S_PRIVATE 512/* Inode is fs-internal */ +#define S_VERYBIG 1024/* Inode is huge: treat loff_t as unsigned */ /* * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -269,6 +270,7 @@ struct inodes_stat_t {#define IS_NOCMTIME (inode) ((inode)->i_flags & S_NOCMTIME)#define IS_SWAPFILE (inode) ((inode)->i_flags & S_SWAPFILE)#define IS_PRIVATE (inode) ((inode)->i_flags & S_PRIVATE) +#define IS_VERYBIG (inode) ((inode)->i_flags & S_VERYBIG)/* the read-only stuff doesn't really belong here, but any other place is probably as bad and I don't want to create yet another include file. */
drivers/char/mem.c +4 -0
@@ -861,6 +861,10 @@ static int memory_open(struct inode *inoif (dev->dev_info) filp->f_mapping->backing_dev_info = dev->dev_info; +/* Is /dev/mem or /dev/kmem ? */ +if (dev->dev_info == &directly_mappable_cdev_bdi) + inode->i_flags |=S_VERYBIG ; +if (dev->fops->open)return dev->fops->open (inode, filp);
fs/proc/base.c +2 -0
@@ -861,6 +861,8 @@ static const struct file_operations procstatic int mem_open (struct inode * inode,struct file * file) { file->private_data = (void *)((long )current->self_exec_id); +/* this file is read only and we can catch out-of-range */ + inode->i_flags |=S_VERYBIG ;return 0 ; }
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/8 devmem: check vmalloc address on kmem read/write
In reply to Wu FengguangFrom: KAMEZAWA Hiroyuki <[email protected]>
Otherwise vmalloc_to_page() will BUG().
This also makes the kmem read/write implementation aligned with mem(4):
"References to nonexistent locations cause errors to be returned." Here we return -ENXIO (inspired by Hugh) if no bytes have been transfered to/from user space, otherwise return partial read/write results.
--- drivers/char/mem.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)
drivers/char/mem.c +18 -10
@@ -395,6 +395,7 @@ static ssize_t read_kmem(struct file *fiunsigned long p = *ppos;ssize_t low_count, read, sz;char * kbuf;/* k-addr because vread() takes vmlist_lock rwlock */ +int err =0 ; read =0 ;if (p < (unsigned long ) high_memory) {
@@ -441,12 +442,16 @@ static ssize_t read_kmem(struct file *fireturn -ENOMEM ;while (count >0 ) { sz =size_inside_page (p, count); +if (!is_vmalloc_or_module_addr ((void *)p)) { + err = -ENXIO ; +break ; + } sz =vread (kbuf, (char *)p, sz);if (!sz)break ;if (copy_to_user (buf, kbuf, sz)) { -free_page ((unsigned long )kbuf); -return -EFAULT ; + err = -EFAULT ; +break ; } count -= sz; buf += sz;
@@ -455,8 +460,8 @@ static ssize_t read_kmem(struct file *fi }free_page ((unsigned long )kbuf); } - *ppos = p; -return read; + *ppos = p; +return read ? read : err; }
@@ -520,6 +525,7 @@ static ssize_t write_kmem(struct file *ssize_t wrote =0 ;ssize_t virtr =0 ;char * kbuf;/* k-addr because vwrite() takes vmlist_lock rwlock */ +int err =0 ;if (p < (unsigned long ) high_memory) { unsigned long to_write = min_t(unsigned long, count,
@@ -540,12 +546,14 @@ static ssize_t write_kmem(struct file *unsigned long sz =size_inside_page (p, count);unsigned long n; +if (!is_vmalloc_or_module_addr ((void *)p)) { + err = -ENXIO ; +break ; + } n =copy_from_user (kbuf, buf, sz);if (n) { -if (wrote + virtr) -break ; -free_page ((unsigned long )kbuf); -return -EFAULT ; + err = -EFAULT ; +break ; } sz =vwrite (kbuf, (char *)p, sz); count -= sz;
@@ -556,8 +564,8 @@ static ssize_t write_kmem(struct file * free_page((unsigned long)kbuf); } - *ppos = p; -return virtr + wrote; + *ppos = p; +return virtr + wrote ? : err; }#endif
PATCH 3/8 devmem: fix kmem write bug on memory holes
In reply to Wu Fengguangwrite_kmem() used to assume vwrite() always return the full buffer length. However now vwrite() could return 0 to indicate memory hole. This creates a bug that "buf" is not advanced accordingly.
Fix it to simply ignore the return value, hence the memory hole.
--- drivers/char/mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/char/mem.c +1 -1
@@ -555,7 +555,7 @@ static ssize_t write_kmem(struct file * err = -EFAULT ;break ; } - sz =vwrite (kbuf, (char *)p, sz); +vwrite (kbuf, (char *)p, sz); count -= sz; buf += sz; virtr += sz;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 4/8 resources: introduce generic page_is_ram()
In reply to Wu FengguangIt's based on walk_system_ram_range(), for archs that don't have their own page_is_ram().
The static verions in MIPS and SCORE are also made global.
--- arch/mips/mm/init.c | 2 +- arch/score/mm/init.c | 2 +- include/linux/ioport.h | 2 ++ kernel/resource.c | 10 ++++++++++ 4 files changed, 14 insertions(+), 2 deletions(-)
kernel/resource.c +10 -0
@@ -297,6 +297,16 @@ int walk_system_ram_range(unsigned long#endif +static int __is_ram (unsigned long pfn,unsigned long nr_pages,void *arg) +{ +return 24 ; +} + +int __attribute__((weak))page_is_ram (unsigned long pfn) +{ +return 24 ==walk_system_ram_range (pfn,1 ,NULL , __is_ram); +} +/* * Find empty slot in the resource tree given range and alignment. */
include/linux/ioport.h +2 -0
@@ -188,5 +188,7 @@ extern int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, void *arg, int (*func)(unsigned long, unsigned long, void *)); +extern int page_is_ram (unsigned long pfn); +#endif /* __ASSEMBLY__ */ #endif /* _LINUX_IOPORT_H */
arch/score/mm/init.c +1 -1
@@ -59,7 +59,7 @@ static unsigned long setup_zero_page(voi }#ifndef CONFIG_NEED_MULTIPLE_NODES -static int __init page_is_ram (unsigned long pagenr) +int page_is_ram (unsigned long pagenr) {if (pagenr >= min_low_pfn && pagenr < max_low_pfn)return 1 ;
arch/mips/mm/init.c +1 -1
@@ -298,7 +298,7 @@ void __init fixrange_init(unsigned long }#ifndef CONFIG_NEED_MULTIPLE_NODES -static int __init page_is_ram (unsigned long pagenr) +int page_is_ram (unsigned long pagenr) {int i;
Re: [PATCH 4/8] resources: introduce generic page_is_ram()
Quoted 23 lines · Wu Fengguang
It's based on
walk_system_ram_range(), for archs that don't have their ownpage_is_ram().The static verions in MIPS and SCORE are also made global.
CC Chen Liqin <[email protected]>CC Lennox Wu <[email protected]>CC Ralf Baechle <[email protected]>CC KAMEZAWA Hiroyuki <[email protected]>Signed-off-by Wu Fengguang <[email protected]>---
arch/mips/mm/init.c | 2 +- arch/score/mm/init.c | 2 +- include/linux/ioport.h | 2 ++ kernel/resource.c | 10 ++++++++++ 4 files changed, 14 insertions(+), 2 deletions(-)kernel/resource.c +0
-0@@ -297,6 +297,16 @@ int walk_system_ram_range(unsigned long#endif
kernel/resource.c int walk_system_ram_range(unsigned long · line 308
Quoted 10 lines
kernel/resource.c +9
-0+static int __is_ram (unsigned long pfn,unsigned long nr_pages,void *arg) +{ +return 24 ; +} + +int __attribute__((weak))page_is_ram (unsigned long pfn) +{ +return 24 ==walk_system_ram_range (pfn,1 ,NULL , __is_ram); +}
Why do you choose 24 instead of using a macro expressing its meaning?
Quoted 43 lines
kernel/resource.c +1
-0+/* * Find empty slot in the resource tree given range and alignment. */ include/linux/ioport.h +2
-0@@ -188,5 +188,7 @@ extern int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, void *arg, int (*func)(unsigned long, unsigned long, void *)); +extern int page_is_ram (unsigned long pfn); +#endif /* __ASSEMBLY__ */ #endif /* _LINUX_IOPORT_H */ arch/score/mm/init.c +1
-1@@ -59,7 +59,7 @@ static unsigned long setup_zero_page(voi }#ifndef CONFIG_NEED_MULTIPLE_NODES -static int __init page_is_ram (unsigned long pagenr) +int page_is_ram (unsigned long pagenr) {if (pagenr >= min_low_pfn && pagenr < max_low_pfn)return 1 ;arch/mips/mm/init.c +1
-2@@ -298,7 +298,7 @@ void __init fixrange_init(unsigned long }#ifndef CONFIG_NEED_MULTIPLE_NODES -static int __init page_is_ram (unsigned long pagenr) +int page_is_ram (unsigned long pagenr) {int i; --To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 8 lines
Live like a child, think like the god. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 4/8] resources: introduce generic page_is_ram()
Quoted 25 lines · AmA(C)rico Wang
Quoted 24 lines · Wu Fengguang
It's based on
walk_system_ram_range(), for archs that don't have their ownpage_is_ram().The static verions in MIPS and SCORE are also made global.
CC Chen Liqin <[email protected]>CC Lennox Wu <[email protected]>CC Ralf Baechle <[email protected]>CC KAMEZAWA Hiroyuki <[email protected]>Signed-off-by Wu Fengguang <[email protected]>---
arch/mips/mm/init.c | 2 +- arch/score/mm/init.c | 2 +- include/linux/ioport.h | 2 ++ kernel/resource.c | 10 ++++++++++ 4 files changed, 14 insertions(+), 2 deletions(-)kernel/resource.c +0
-0@@ -297,6 +297,16 @@ int walk_system_ram_range(unsigned long#endif
Quoted 12 lines
kernel/resource.c int walk_system_ram_range(unsigned long · line 308
Quoted 9 lines
kernel/resource.c +9
-0+static int __is_ram (unsigned long pfn,unsigned long nr_pages,void *arg) +{ +return 24 ; +} + +int __attribute__((weak))page_is_ram (unsigned long pfn) +{ +return 24 ==walk_system_ram_range (pfn,1 ,NULL , __is_ram); +}Why do you choose 24 instead of using a macro expressing its meaning?
Hmm, I thought they are close enough to be obvious. Anyway this should look better:
resources: introduce generic page_is_ram()
It's based on walk_system_ram_range(), for archs that don't have their own page_is_ram().
The static verions in MIPS and SCORE are also made global.
--- arch/mips/mm/init.c | 2 +- arch/score/mm/init.c | 2 +- include/linux/ioport.h | 2 ++ kernel/resource.c | 11 +++++++++++ 4 files changed, 15 insertions(+), 2 deletions(-)
kernel/resource.c +11 -0
@@ -297,6 +297,17 @@ int walk_system_ram_range(unsigned long#endif +#define PAGE_IS_RAM 24 +static int __is_ram (unsigned long pfn,unsigned long nr_pages,void *arg) +{ +return PAGE_IS_RAM ; +} +int __attribute__((weak))page_is_ram (unsigned long pfn) +{ +return PAGE_IS_RAM ==walk_system_ram_range (pfn,1 ,NULL , __is_ram); +} +#undef PAGE_IS_RAM +/* * Find empty slot in the resource tree given range and alignment. */
include/linux/ioport.h +2 -0
@@ -188,5 +188,7 @@ extern int walk_system_ram_range(unsigned long start_pfn, unsigned long nr_pages, void *arg, int (*func)(unsigned long, unsigned long, void *)); +extern int page_is_ram (unsigned long pfn); +#endif /* __ASSEMBLY__ */ #endif /* _LINUX_IOPORT_H */
arch/score/mm/init.c +1 -1
@@ -59,7 +59,7 @@ static unsigned long setup_zero_page(voi }#ifndef CONFIG_NEED_MULTIPLE_NODES -static int __init page_is_ram (unsigned long pagenr) +int page_is_ram (unsigned long pagenr) {if (pagenr >= min_low_pfn && pagenr < max_low_pfn)return 1 ;
arch/mips/mm/init.c +1 -1
@@ -298,7 +298,7 @@ void __init fixrange_init(unsigned long }#ifndef CONFIG_NEED_MULTIPLE_NODES -static int __init page_is_ram (unsigned long pagenr) +int page_is_ram (unsigned long pagenr) {int i;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 5/8 vmalloc: simplify vread()/vwrite()
In reply to Wu Fengguangvread()/vwrite() is only called from kcore/kmem to access one page at a time. So the logic can be vastly simplified.
The changes are:
- remove the vmlist walk and rely solely on vmalloc_to_page()
- replace the VM_IOREMAP check with (page && page_is_ram(pfn))
- rename to vread_page()/vwrite_page()
The page_is_ram() check is necessary because kmap_atomic() is not designed to work with non-RAM pages.
Note that even for a RAM page, we don't own the page, and cannot assume it's a _PAGE_CACHE_WB page.
--- drivers/char/mem.c | 8 - fs/proc/kcore.c | 2 include/linux/vmalloc.h | 6 mm/vmalloc.c | 230 ++++++++------------------------------ 4 files changed, 58 insertions(+), 188 deletions(-)
mm/vmalloc.c +50 -180
@@ -1646,232 +1646,102 @@ void *vmalloc_32_user(unsigned long size }EXPORT_SYMBOL (vmalloc_32_user); -/* -* small helper routine , copy contents to buf from addr. -* If the page is not present, fill zero. -*/ - -static int aligned_vread (char *buf,char *addr,unsigned long count) -{ -struct page *p; -int copied =0 ; - -while (count) { -unsigned long offset, length; - - offset = (unsigned long )addr & ~PAGE_MASK ; - length =PAGE_SIZE - offset; -if (length > count) - length = count; - p =vmalloc_to_page (addr); -/* -* To do safe access to this _mapped_ area, we need -* lock. But adding lock here means that we need to add -* overhead of vmalloc()/vfree() calles for this _debug_ -* interface, rarely used. Instead of that, we'll use -* kmap() and get small overhead in this access function. -*/ -if (p) { -/* -* we can expect USER0 is not used (see vread/vwrite's -* function description) -*/ -void *map =kmap_atomic (p,KM_USER0 ); -memcpy (buf, map + offset, length); -kunmap_atomic (map,KM_USER0 ); - }else -memset (buf,0 , length); - - addr += length; - buf += length; - copied += length; - count -= length; - } -return copied; -} - -static int aligned_vwrite (char *buf,char *addr,unsigned long count) -{ -struct page *p; -int copied =0 ; - -while (count) { -unsigned long offset, length; - - offset = (unsigned long )addr & ~PAGE_MASK ; - length =PAGE_SIZE - offset; -if (length > count) - length = count; - p =vmalloc_to_page (addr); -/* -* To do safe access to this _mapped_ area, we need -* lock. But adding lock here means that we need to add -* overhead of vmalloc()/vfree() calles for this _debug_ -* interface, rarely used. Instead of that, we'll use -* kmap() and get small overhead in this access function. -*/ -if (p) { -/* -* we can expect USER0 is not used (see vread/vwrite's -* function description) -*/ -void *map =kmap_atomic (p,KM_USER0 ); -memcpy (map + offset, buf, length); -kunmap_atomic (map,KM_USER0 ); - } - addr += length; - buf += length; - copied += length; - count -= length; - } -return copied; -} -/** -* vread() - read vmalloc area in a safe way. +* vread_page() - read vmalloc area in a safe way. * @buf: buffer for reading data * @addr: vm address. -* @count: number of bytes to be read. +* @count: number of bytes to read inside the page. * -* Returns # of bytes which addr and buf should be increased. -* (same number to @count). Returns 0 if [addr...addr+count) doesn't -* includes any intersect with alive vmalloc area. +* Returns # of bytes copied on success. +* Returns 0 if @addr is not vmalloc'ed, or is mapped to non-RAM. * * This function checks that addr is a valid vmalloc'ed area, and * copy data from that area to a given buffer. If the given memory range * of [addr...addr+count) includes some valid address, data is copied to * proper area of @buf. If there are memory holes, they'll be zero-filled. -* IOREMAP area is treated as memory hole and no copy is done. * -* If [addr...addr+count) doesn't includes any intersects with alive -* vm_struct area, returns 0. * @buf should be kernel's buffer. Because this function uses KM_USER0, * the caller should guarantee KM_USER0 is not used. * -* Note: In usual ops, vread() is never necessary because the caller +* Note: In usual ops, vread_page() is never necessary because the caller * should know vmalloc() area is valid and can use memcpy(). * This is for routines which have to access vmalloc area without -* any informaion, as /dev/kmem. +* any informaion, as /dev/kmem and /dev/kcore. * */ -long vread (char *buf,char *addr,unsigned long count) +int vread_page (char *buf,char *addr,unsigned int count) { -struct vm_struct *tmp; -char *vaddr, *buf_start = buf; -unsigned long buflen = count; -unsigned long n; - -/* Don't allow overflow */ -if ((unsigned long ) addr + count < count) - count = -(unsigned long ) addr; +struct page *p; +void *map; +int offset = (unsigned long )addr & (PAGE_SIZE -1 ); -read_lock (&vmlist_lock); -for (tmp = vmlist; count && tmp; tmp = tmp->next) { - vaddr = (char *) tmp->addr; -if (addr >= vaddr + tmp->size -PAGE_SIZE ) -continue ; -while (addr < vaddr) { -if (count ==0 ) - goto finished; - *buf ='\0' ; - buf++; - addr++; - count--; - } - n = vaddr + tmp->size -PAGE_SIZE - addr; -if (n > count) - n = count; -if (!(tmp->flags &VM_IOREMAP )) -aligned_vread (buf, addr, n); -else /* IOREMAP area is treated as memory hole */ -memset (buf,0 , n); - buf += n; - addr += n; - count -= n; - } -finished: -read_unlock (&vmlist_lock); +/* Assume subpage access */ +BUG_ON (count >PAGE_SIZE - offset); -if (buf == buf_start) + p =vmalloc_to_page (addr); +if (!p || !page_is_ram (page_to_pfn (p))) { +memset (buf,0 , count);return 0 ; -/* zero-fill memory holes */ -if (buf != buf_start + buflen) -memset (buf,0 , buflen - (buf - buf_start)); + } -return buflen; +/* +* To do safe access to this _mapped_ area, we need +* lock. But adding lock here means that we need to add +* overhead of vmalloc()/vfree() calles for this _debug_ +* interface, rarely used. Instead of that, we'll use +* kmap() and get small overhead in this access function. +*/ + map =kmap_atomic (p,KM_USER0 ); +memcpy (buf, map + offset, count); +kunmap_atomic (map,KM_USER0 ); + +return count; }/** -* vwrite() - write vmalloc area in a safe way. +* vwrite_page() - write vmalloc area in a safe way. * @buf: buffer for source data * @addr: vm address. -* @count: number of bytes to be read. +* @count: number of bytes to write inside the page. * -* Returns # of bytes which addr and buf should be incresed. -* (same number to @count). -* If [addr...addr+count) doesn't includes any intersect with valid -* vmalloc area, returns 0. +* Returns # of bytes copied on success. +* Returns 0 if @addr is not vmalloc'ed, or is mapped to non-RAM. * * This function checks that addr is a valid vmalloc'ed area, and * copy data from a buffer to the given addr. If specified range of * [addr...addr+count) includes some valid address, data is copied from * proper area of @buf. If there are memory holes, no copy to hole. -* IOREMAP area is treated as memory hole and no copy is done. * -* If [addr...addr+count) doesn't includes any intersects with alive -* vm_struct area, returns 0. * @buf should be kernel's buffer. Because this function uses KM_USER0, * the caller should guarantee KM_USER0 is not used. * -* Note: In usual ops, vwrite() is never necessary because the caller +* Note: In usual ops, vwrite_page() is never necessary because the caller * should know vmalloc() area is valid and can use memcpy(). * This is for routines which have to access vmalloc area without * any informaion, as /dev/kmem. -* -* The caller should guarantee KM_USER1 is not used. */ -long vwrite (char *buf,char *addr,unsigned long count) +int vwrite_page (char *buf,char *addr,unsigned int count) { -struct vm_struct *tmp; -char *vaddr; -unsigned long n, buflen; -int copied =0 ; - -/* Don't allow overflow */ -if ((unsigned long ) addr + count < count) - count = -(unsigned long ) addr; - buflen = count; +struct page *p; +void *map; +int offset = (unsigned long )addr & (PAGE_SIZE -1 ); -read_lock (&vmlist_lock); -for (tmp = vmlist; count && tmp; tmp = tmp->next) { - vaddr = (char *) tmp->addr; -if (addr >= vaddr + tmp->size -PAGE_SIZE ) -continue ; -while (addr < vaddr) { -if (count ==0 ) - goto finished; - buf++; - addr++; - count--; - } - n = vaddr + tmp->size -PAGE_SIZE - addr; -if (n > count) - n = count; -if (!(tmp->flags &VM_IOREMAP )) { -aligned_vwrite (buf, addr, n); - copied++; - } - buf += n; - addr += n; - count -= n; - } -finished: -read_unlock (&vmlist_lock); -if (!copied) +/* Assume subpage access */ +BUG_ON (count >PAGE_SIZE - offset); + + p =vmalloc_to_page (addr); +if (!p) +return 0 ; +if (!page_is_ram (page_to_pfn (p)))return 0 ; -return buflen; + + map =kmap_atomic (p,KM_USER0 ); +memcpy (map + offset, buf, count); +kunmap_atomic (map,KM_USER0 ); + +return count; }/**
drivers/char/mem.c +4 -4
@@ -394,7 +394,7 @@ static ssize_t read_kmem(struct file *fi {unsigned long p = *ppos;ssize_t low_count, read, sz; -char * kbuf;/* k-addr because vread() takes vmlist_lock rwlock */ +char *kbuf;/* k-addr because vread_page() does kmap_atomic */ int err =0 ; read =0 ;
@@ -446,7 +446,7 @@ static ssize_t read_kmem(struct file *fi err = -ENXIO ;break ; } - sz =vread (kbuf, (char *)p, sz); + sz =vread_page (kbuf, (char *)p, sz);if (!sz)break ;if (copy_to_user (buf, kbuf, sz)) {
@@ -524,7 +524,7 @@ static ssize_t write_kmem(struct file *unsigned long p = *ppos;ssize_t wrote =0 ;ssize_t virtr =0 ; -char * kbuf;/* k-addr because vwrite() takes vmlist_lock rwlock */ +char *kbuf;/* k-addr because vwrite_page() does kmap_atomic */ int err =0 ;if (p < (unsigned long ) high_memory) {
@@ -555,7 +555,7 @@ static ssize_t write_kmem(struct file * err = -EFAULT ;break ; } -vwrite (kbuf, (char *)p, sz); +vwrite_page (kbuf, (char *)p, sz); count -= sz; buf += sz; virtr += sz;
fs/proc/kcore.c +1 -1
@@ -499,7 +499,7 @@ read_kcore(struct file *file, char __use elf_buf =kzalloc (tsz,GFP_KERNEL );if (!elf_buf)return -ENOMEM ; -vread (elf_buf, (char *)start, tsz); +vread_page (elf_buf, (char *)start, tsz);/* we have to zero-fill user buffer even if no read */ if (copy_to_user (buffer, elf_buf, tsz)) {kfree (elf_buf);
include/linux/vmalloc.h +3 -3
@@ -104,9 +104,9 @@ extern void unmap_kernel_range(unsignedextern struct vm_struct *alloc_vm_area (size_t size);extern void free_vm_area (struct vm_struct *area); -/* for /dev/kmem */ -extern long vread (char *buf,char *addr,unsigned long count); -extern long vwrite (char *buf,char *addr,unsigned long count); +/* for /dev/kmem and /proc/kcore */ +extern int vread_page (char *buf,char *addr,unsigned int count); +extern int vwrite_page (char *buf,char *addr,unsigned int count);/* * Internals. Dont't use..
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 10 lines · Wu Fengguang
vread()/vwrite() is only called from kcore/kmem to access one page at a time. So the logic can be vastly simplified.
The changes are:
- remove the vmlist walk and rely solely onvmalloc_to_page()
- replace theVM_IOREMAPcheck with (page &&page_is_ram(pfn))
- rename to vread_page()/vwrite_page()The
page_is_ram()check is necessary becausekmap_atomic()is not designed to work with non-RAM pages.
I don't know if you can really do this. Previously vmlist_lock would be taken, which will prevent these vm areas from being freed.
Quoted 2 lines
Note that even for a RAM page, we don't own the page, and cannot assume it's a
_PAGE_CACHE_WBpage.
So why is this not a problem for your patch? I don't see how you handle it.
What's the problem with the current code, exactly? I would prefer that you continue using the same vmlist locking and checking for validating addresses.
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 12 lines · Nick Piggin
Quoted 10 lines · Wu Fengguang
vread()/vwrite() is only called from kcore/kmem to access one page at a time. So the logic can be vastly simplified.
The changes are:
- remove the vmlist walk and rely solely onvmalloc_to_page()
- replace theVM_IOREMAPcheck with (page &&page_is_ram(pfn))
- rename to vread_page()/vwrite_page()The
page_is_ram()check is necessary becausekmap_atomic()is not designed to work with non-RAM pages.
Quoted 8 lines
I don't know if you can really do this. Previously
vmlist_lockwould be taken, which will prevent these vm areas from being freed.Quoted 2 lines
Note that even for a RAM page, we don't own the page, and cannot assume it's a
_PAGE_CACHE_WBpage.So why is this not a problem for your patch? I don't see how you handle it.
Sorry I didn't handle it. Just hope to catch attentions from someone (ie. you :).
It's not a problem for x86_64 at all. For others I wonder if any driver will vmalloc HIGHMEM pages with !_PAGE_CACHE_WB attribute..
So I noted the possible problem and leave it alone.
Quoted 1 line
What's the problem with the current code, exactly? I would prefer that
- unnecessary complexity to handle multi-page case, since it's always called to access one single page;
- the kmap_atomic() cache consistency problem, which I expressed some concern (without further action)
Quoted 2 lines
you continue using the same vmlist locking and checking for validating addresses.
It's a reasonable suggestion. Kame, would you agree on killing the kmap_atomic() and revert to the vmlist walk?
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 22 lines · Wu Fengguang
Quoted 12 lines · Nick Piggin
Quoted 10 lines · Wu Fengguang
vread()/vwrite() is only called from kcore/kmem to access one page at a time. So the logic can be vastly simplified.
The changes are:
- remove the vmlist walk and rely solely onvmalloc_to_page()
- replace theVM_IOREMAPcheck with (page &&page_is_ram(pfn))
- rename to vread_page()/vwrite_page()The
page_is_ram()check is necessary becausekmap_atomic()is not designed to work with non-RAM pages.Quoted 8 lines
I don't know if you can really do this. Previously
vmlist_lockwould be taken, which will prevent these vm areas from being freed.Quoted 2 lines
Note that even for a RAM page, we don't own the page, and cannot assume it's a
_PAGE_CACHE_WBpage.So why is this not a problem for your patch? I don't see how you handle it.
Quoted 7 lines
Sorry I didn't handle it. Just hope to catch attentions from someone (ie. you :).
It's not a problem for
x86_64at all. For others I wonder if any driver will vmalloc HIGHMEM pages with !_PAGE_CACHE_WB attribute..So I noted the possible problem and leave it alone.
Well it doesn't need to be vmalloc. Any kind of vmap like ioremap. And these can be accompanied by changing the caching attribute. Like agp code, for an example. But I don't know if that ever becomes a problem in practice.
Quoted 4 lines
Quoted 1 line
What's the problem with the current code, exactly? I would prefer that
- unnecessary complexity to handle multi-page case, since it's always called to access one single page;
Fair point there. It just wasn't clear what exactly is your rationale because this was in a set of other patches.
Quoted 2 lines
- the
kmap_atomic()cache consistency problem, which I expressed some concern (without further action)
Which kmap_atomic problem? Can you explain again? Virtual cache aliasing problem you mean? Or caching attribute conflicts?
The whole thing looks stupid though, apparently kmap is used to avoid "the lock". But the lock is already held. We should just use the vmap address.
Quoted 5 lines
Quoted 2 lines
you continue using the same vmlist locking and checking for validating addresses.
It's a reasonable suggestion. Kame, would you agree on killing the
kmap_atomic()and revert to the vmlist walk?
Yes, vmlist locking is always required to have a pin on the pages, and IMO it should be quite easy to check for IOREMAP, so we should leave that check there to avoid the possibility of regressions.
Thanks,
Nick
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 29 lines · Nick Piggin
Quoted 28 lines · Wu Fengguang
Quoted 12 lines · Nick Piggin
Quoted 10 lines · Wu Fengguang
vread()/vwrite() is only called from kcore/kmem to access one page at a time. So the logic can be vastly simplified.
The changes are:
- remove the vmlist walk and rely solely onvmalloc_to_page()
- replace theVM_IOREMAPcheck with (page &&page_is_ram(pfn))
- rename to vread_page()/vwrite_page()The
page_is_ram()check is necessary becausekmap_atomic()is not designed to work with non-RAM pages.Quoted 8 lines
I don't know if you can really do this. Previously
vmlist_lockwould be taken, which will prevent these vm areas from being freed.Quoted 2 lines
Note that even for a RAM page, we don't own the page, and cannot assume it's a
_PAGE_CACHE_WBpage.So why is this not a problem for your patch? I don't see how you handle it.
Sorry I didn't handle it. Just hope to catch attentions from someone (ie. you :).
It's not a problem for
x86_64at all. For others I wonder if any driver will vmalloc HIGHMEM pages with !_PAGE_CACHE_WB attribute..
Quoted 6 lines
Quoted 1 line
So I noted the possible problem and leave it alone.
Well it doesn't need to be vmalloc. Any kind of vmap like ioremap. And these can be accompanied by changing the caching attribute. Like agp code, for an example. But I don't know if that ever becomes a problem in practice.
Yes vmap in general can change caching attribute. However I only care about vmap that maps RAM pages, since my patch treats non-RAM pages as hole and won't access them.
Quoted 5 lines
Quoted 4 lines
Quoted 1 line
What's the problem with the current code, exactly? I would prefer that
- unnecessary complexity to handle multi-page case, since it's always called to access one single page;
Quoted 8 lines
Fair point there. It just wasn't clear what exactly is your rationale because this was in a set of other patches.
Quoted 2 lines
- the
kmap_atomic()cache consistency problem, which I expressed some concern (without further action)Which
kmap_atomicproblem? Can you explain again? Virtual cache aliasing problem you mean? Or caching attribute conflicts?
kmap_atomic() assumes you own the page and always use _PAGE_CACHE_WB. So there may be conflicts if the page was !_PAGE_CACHE_WB.
Quoted 3 lines
The whole thing looks stupid though, apparently kmap is used to avoid "the lock". But the lock is already held. We should just use the vmap address.
Yes. I wonder why Kame introduced kmap_atomic() in d0107eb07 -- given that he at the same time fixed the order of removing vm_struct and vmap in dd32c279983b.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
you continue using the same vmlist locking and checking for validating addresses.
It's a reasonable suggestion. Kame, would you agree on killing the
kmap_atomic()and revert to the vmlist walk?Yes, vmlist locking is always required to have a pin on the pages, and IMO it should be quite easy to check for IOREMAP, so we should leave that check there to avoid the possibility of regressions.
I have no problem if Kame could dismiss my question :)
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 8 lines · On Tue, 19 Jan 2010 09:33:03 +0800 Wu Fengguang <[email protected]> wrote:
Quoted 3 lines
The whole thing looks stupid though, apparently kmap is used to avoid "the lock". But the lock is already held. We should just use the vmap address.
Yes. I wonder why Kame introduced
kmap_atomic()ind0107eb07-- given that he at the same time fixed the order of removingvm_structand vmap indd32c279983b.
Hmm...I must check my thinking again before answering..
vmalloc/vmap is constructed by 2 layer.
- vmalloc layer....guarded by vmlist_lock.
- vmap layer ....gurderd by purge_lock. etc.
Now, let's see how vmalloc() works. It does job in 2 steps. vmalloc():
- allocate vmalloc area to the list under vmlist_lock.
- map pages.vfree()
- free vmalloc area from the list under vmlist_lock.
- unmap pages under purge_lock.
Now. vread(), vwrite() just take vmlist_lock, doesn't take purge_lock(). It walks page table and find pte entry, page, kmap and access it.
Oh, yes. It seems it's safe without kmap. But My concern is percpu allocator.
It uses get_vm_area() and controls mapped pages by themselves and map/unmap pages by with their own logic. vmalloc.c is just used for alloc/free virtual address.
Now, vread()/vwrite() just holds vmlist_lock() and walk page table without no guarantee that the found page is stably mapped. So, I used kmap.
If I miss something, I'm very sorry to add such kmap.
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 29 lines · KAMEZAWA Hiroyuki
Quoted 8 lines · On Tue, 19 Jan 2010 09:33:03 +0800 Wu Fengguang <[email protected]> wrote:
Quoted 3 lines
The whole thing looks stupid though, apparently kmap is used to avoid "the lock". But the lock is already held. We should just use the vmap address.
Yes. I wonder why Kame introduced
kmap_atomic()ind0107eb07-- given that he at the same time fixed the order of removingvm_structand vmap indd32c279983b.Hmm...I must check my thinking again before answering..
vmalloc/vmap is constructed by 2 layer.
- vmalloc layer....guarded byvmlist_lock.
- vmap layer ....gurderd bypurge_lock. etc.Now, let's see how
vmalloc()works. It does job in 2 steps.vmalloc():
- allocate vmalloc area to the list undervmlist_lock.
- map pages.vfree()
- free vmalloc area from the list undervmlist_lock.
- unmap pages underpurge_lock.Now.
vread(),vwrite()just takevmlist_lock, doesn't takepurge_lock(). It walks page table and find pte entry, page, kmap and access it.Oh, yes. It seems it's safe without kmap. But My concern is percpu allocator.
Quoted 8 lines
It uses
get_vm_area()and controls mapped pages by themselves and map/unmap pages by with their own logic.vmalloc.cis just used for alloc/free virtual address.Now, vread()/vwrite() just holds
vmlist_lock()and walk page table without no guarantee that the found page is stably mapped. So, I used kmap.If I miss something, I'm very sorry to add such kmap.
Ah Thanks for explanation!
I did some audit and find that
- set_memory_uc(), set_memory_array_uc(), set_pages_uc(), set_pages_array_uc() are called EFI code and various video drivers, all of them don't touch HIGHMEM RAM
- Kame: ioremap() won't allow remap of physical RAM
So kmap_atomic() is safe. Let's just settle on this patch?
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 43 lines · On Thu, 21 Jan 2010 13:05:21 +0800 Wu Fengguang <[email protected]> wrote:
Quoted 29 lines · KAMEZAWA Hiroyuki
Quoted 8 lines · On Tue, 19 Jan 2010 09:33:03 +0800 Wu Fengguang <[email protected]> wrote:
Quoted 3 lines
The whole thing looks stupid though, apparently kmap is used to avoid "the lock". But the lock is already held. We should just use the vmap address.
Yes. I wonder why Kame introduced
kmap_atomic()ind0107eb07-- given that he at the same time fixed the order of removingvm_structand vmap indd32c279983b.Hmm...I must check my thinking again before answering..
vmalloc/vmap is constructed by 2 layer.
- vmalloc layer....guarded byvmlist_lock.
- vmap layer ....gurderd bypurge_lock. etc.Now, let's see how
vmalloc()works. It does job in 2 steps.vmalloc():
- allocate vmalloc area to the list undervmlist_lock.
- map pages.vfree()
- free vmalloc area from the list undervmlist_lock.
- unmap pages underpurge_lock.Now.
vread(),vwrite()just takevmlist_lock, doesn't takepurge_lock(). It walks page table and find pte entry, page, kmap and access it.Oh, yes. It seems it's safe without kmap. But My concern is percpu allocator.
Quoted 8 lines
It uses
get_vm_area()and controls mapped pages by themselves and map/unmap pages by with their own logic.vmalloc.cis just used for alloc/free virtual address.Now, vread()/vwrite() just holds
vmlist_lock()and walk page table without no guarantee that the found page is stably mapped. So, I used kmap.If I miss something, I'm very sorry to add such kmap.
Ah Thanks for explanation!
I did some audit and find that
Quoted 8 lines
-
set_memory_uc(),set_memory_array_uc(),set_pages_uc(),set_pages_array_uc()are called EFI code and various video drivers, all of them don't touch HIGHMEM RAM- Kame:
ioremap()won't allow remap of physical RAMSo
kmap_atomic()is safe. Let's just settle on this patch?
I recommend you to keep check on VM_IOREMAP. That was checked far before I started to see Linux. Some _unknown_ driver can call get_vm_area() and map arbitrary pages there.
I'm sorry I coundn't track discussion correctly.
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 5/8] vmalloc: simplify vread()/vwrite()
Quoted 11 lines · KAMEZAWA Hiroyuki
Quoted 8 lines · On Thu, 21 Jan 2010 13:05:21 +0800 Wu Fengguang <[email protected]> wrote:
On Mon, Jan 18, 2010 at 07:23:43PM -0700, KAMEZAWA Hiroyuki wrote:
I did some audit and find that
-
set_memory_uc(),set_memory_array_uc(),set_pages_uc(),set_pages_array_uc()are called EFI code and various video drivers, all of them don't touch HIGHMEM RAM
Quoted 7 lines
Quoted 4 lines
- Kame:
ioremap()won't allow remap of physical RAMSo
kmap_atomic()is safe. Let's just settle on this patch?I recommend you to keep check on
VM_IOREMAP. That was checked far before I started to see Linux. Some _unknown_ driver can callget_vm_area()and map arbitrary pages there.
OK, I'll turn this patch into a less radical one.
Thanks,
Fengguang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 6/8 hwpoison: prevent /dev/kmem from accessing hwpoison pages
In reply to Wu FengguangWhen /dev/kmem read()/write() encounters hwpoison page, stop it and return the amount of work done till now, or return -EIO if nothing have been copied.
--- drivers/char/mem.c | 26 ++++++++++++++++++++------ mm/vmalloc.c | 8 ++++++++ 2 files changed, 28 insertions(+), 6 deletions(-)
drivers/char/mem.c +20 -6
@@ -426,6 +426,9 @@ static ssize_t read_kmem(struct file *fi*/ kbuf =xlate_dev_kmem_ptr ((char *)p); +if (unlikely (virt_addr_valid (kbuf) && +PageHWPoison (virt_to_page (kbuf)))) +return -EIO ;if (copy_to_user (buf, kbuf, sz))return -EFAULT ; buf += sz;
@@ -447,8 +450,10 @@ static ssize_t read_kmem(struct file *fibreak ; } sz =vread_page (kbuf, (char *)p, sz); -if (!sz) +if (sz <=0 ) { + err = sz;break ; + }if (copy_to_user (buf, kbuf, sz)) { err = -EFAULT ;break ;
@@ -471,6 +476,7 @@ do_write_kmem(unsigned long p, const cha {ssize_t written, sz;unsigned long copied; +int err =0 ; written =0 ;#ifdef __ARCH_HAS_NO_PAGE_ZERO_MAPPED
@@ -497,13 +503,19 @@ do_write_kmem(unsigned long p, const cha*/ ptr =xlate_dev_kmem_ptr ((char *)p); +if (unlikely (virt_addr_valid (ptr) && +PageHWPoison (virt_to_page (ptr)))) { + err = -EIO ; +break ; + } + copied =copy_from_user (ptr, buf, sz);if (copied) { written += sz - copied; -if (written) -break ; -return -EFAULT ; + err = -EFAULT ; +break ; } + buf += sz; p += sz; count -= sz;
@@ -511,7 +523,7 @@ do_write_kmem(unsigned long p, const cha } *ppos += written; -return written; +return written ? written : err; }
@@ -555,7 +567,9 @@ static ssize_t write_kmem(struct file * err = -EFAULT ;break ; } -vwrite_page (kbuf, (char *)p, sz); + err =vwrite_page (kbuf, (char *)p, sz); +if (err <0 ) +break ; count -= sz; buf += sz; virtr += sz;
mm/vmalloc.c +8 -0
@@ -1654,6 +1654,7 @@ EXPORT_SYMBOL(vmalloc_32_user);* * Returns # of bytes copied on success. * Returns 0 if @addr is not vmalloc'ed, or is mapped to non-RAM. +* Returns -EIO if the mapped page is corrupted. * * This function checks that addr is a valid vmalloc'ed area, and * copy data from that area to a given buffer. If the given memory range
@@ -1684,6 +1685,10 @@ int vread_page(char *buf, char *addr, unmemset (buf,0 , count);return 0 ; } +if (PageHWPoison (p)) { +memset (buf,0 , count); +return -EIO ; + }/* * To do safe access to this _mapped_ area, we need
@@ -1707,6 +1712,7 @@ int vread_page(char *buf, char *addr, un* * Returns # of bytes copied on success. * Returns 0 if @addr is not vmalloc'ed, or is mapped to non-RAM. +* Returns -EIO if the mapped page is corrupted. * * This function checks that addr is a valid vmalloc'ed area, and * copy data from a buffer to the given addr. If specified range of
@@ -1736,6 +1742,8 @@ int vwrite_page(char *buf, char *addr, ureturn 0 ;if (!page_is_ram (page_to_pfn (p)))return 0 ; +if (PageHWPoison (p)) +return -EIO ; map =kmap_atomic (p,KM_USER0 );memcpy (map + offset, buf, count);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 7/8 hwpoison: prevent /dev/mem from accessing hwpoison pages
In reply to Wu FengguangReturn EIO when user space tries to read/write/mmap hwpoison pages via the /dev/mem interface.
The approach: rename range_is_allowed() to devmem_check_pfn_range(), and add PageHWPoison() test in it. This function will be called for the whole mmap() range, or page by page for read()/write(). So it would fail the mmap() request as a whole, and return partial results for read()/write().
--- drivers/char/mem.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-)
drivers/char/mem.c +21 -18
@@ -89,31 +89,28 @@ static inline int valid_mmap_phys_addr_r }#endif -#ifdef CONFIG_STRICT_DEVMEM -static inline int range_is_allowed (unsigned long pfn,unsigned long size) +static int devmem_check_pfn_range (unsigned long pfn,unsigned long bytes) {u64 from = ((u64 )pfn) <<PAGE_SHIFT ; -u64 to = from + size; +u64 to = from + bytes;u64 cursor = from;while (cursor < to) { +#ifdef CONFIG_STRICT_DEVMEM if (!devmem_is_allowed (pfn)) {printk (KERN_INFO "Program %s tried to access /dev/mem between %Lx->%Lx.\n" , current->comm, from, to); -return 0 ; +return -EPERM ; } +#endif +if (pfn_valid (pfn) &&PageHWPoison (pfn_to_page (pfn))) +return -EIO ; cursor +=PAGE_SIZE ; pfn++; } -return 1 ; -} -#else -static inline int range_is_allowed (unsigned long pfn,unsigned long size) -{ -return 1 ; +return 0 ; } -#endif void __attribute__((weak))unxlate_dev_mem_ptr (unsigned long phys,void *addr) {
@@ -150,11 +147,13 @@ static ssize_t read_mem(struct file * fiwhile (count >0 ) {unsigned long remaining; +int err; sz =size_inside_page (p, count); -if (!range_is_allowed (p >>PAGE_SHIFT , count)) -return -EPERM ; + err =devmem_check_pfn_range (p >>PAGE_SHIFT , count); +if (err) +return err;/* * On ia64 if a page has been mapped somewhere as
@@ -184,9 +183,10 @@ static ssize_t write_mem(struct file * f size_t count, loff_t *ppos) { unsigned long p = *ppos; -ssize_t written, sz;unsigned long copied; +ssize_t written, sz;void *ptr; +int err;if (!valid_phys_addr_range (p, count))return -EFAULT ;
@@ -208,8 +208,9 @@ static ssize_t write_mem(struct file * fwhile (count >0 ) { sz =size_inside_page (p, count); -if (!range_is_allowed (p >>PAGE_SHIFT , sz)) -return -EPERM ; + err =devmem_check_pfn_range (p >>PAGE_SHIFT , sz); +if (err) +return err;/* * On ia64 if a page has been mapped somewhere as
@@ -297,6 +298,7 @@ static const struct vm_operations_structstatic int mmap_mem (struct file * file,struct vm_area_struct * vma) {size_t size = vma->vm_end - vma->vm_start; +int err;if (!valid_mmap_phys_addr_range (vma->vm_pgoff, size))return -EINVAL ;
@@ -304,8 +306,9 @@ static int mmap_mem(struct file * file,if (!private_mapping_ok (vma))return -ENOSYS ; -if (!range_is_allowed (vma->vm_pgoff, size)) -return -EPERM ; + err =devmem_check_pfn_range (vma->vm_pgoff, size); +if (err) +return err;if (!phys_mem_access_prot_allowed (file, vma->vm_pgoff, size, &vma->vm_page_prot))
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 8/8 hwpoison: prevent /dev/kcore from accessing hwpoison pages
In reply to Wu FengguangSilently fill buffer with zeros when encounter hwpoison pages (accessing the hwpoison page content is deadly).
This patch does not cover X86_32 - which has a dumb kern_addr_valid(). It is unlikely anyone run a 32bit kernel will care about the hwpoison feature - its usable memory is limited.
--- arch/x86/mm/init_64.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
arch/x86/mm/init_64.c +13 -3
@@ -825,6 +825,7 @@ int __init reserve_bootmem_generic(unsigint kern_addr_valid (unsigned long addr) {unsigned long above = ((long )addr) >> __VIRTUAL_MASK_SHIFT; +unsigned long pfn;pgd_t *pgd;pud_t *pud;pmd_t *pmd;
@@ -845,14 +846,23 @@ int kern_addr_valid(unsigned long addr)if (pmd_none (*pmd))return 0 ; -if (pmd_large (*pmd)) -return pfn_valid (pmd_pfn (*pmd)); +if (pmd_large (*pmd)) { + pfn =pmd_pfn (*pmd); + pfn +=pte_index (addr); + goto check_pfn; + } pte =pte_offset_kernel (pmd, addr);if (pte_none (*pte))return 0 ; -return pfn_valid (pte_pfn (*pte)); + pfn =pte_pfn (*pte); +check_pfn: +if (!pfn_valid (pfn)) +return 0 ; +if (PageHWPoison (pfn_to_page (pfn))) +return 0 ; +return 1 ; }/*
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 8/8] hwpoison: prevent /dev/kcore from accessing hwpoison pages
Your $subject, I think you mean /proc/kcore...
Quoted 11 lines · Wu Fengguang
Silently fill buffer with zeros when encounter hwpoison pages (accessing the hwpoison page content is deadly).
This patch does not cover
X86_32- which has a dumbkern_addr_valid(). It is unlikely anyone run a 32bit kernel will care about the hwpoison feature - its usable memory is limited.CC Ingo Molnar <[email protected]>CC Andi Kleen <[email protected]>CC Pekka Enberg <[email protected]>Signed-off-by Wu Fengguang <[email protected]>
This patch looks fine for me.
Quoted 48 lines
---
arch/x86/mm/init_64.c| 16 +++++++++++++---1 file changed, 13 insertions(+), 3 deletions(-)arch/x86/mm/init_64.c +13
-4@@ -825,6 +825,7 @@ int __init reserve_bootmem_generic(unsigint kern_addr_valid (unsigned long addr) {unsigned long above = ((long )addr) >> __VIRTUAL_MASK_SHIFT; +unsigned long pfn;pgd_t *pgd;pud_t *pud;pmd_t *pmd;@@ -845,14 +846,23 @@ int kern_addr_valid(unsigned long addr)if (pmd_none (*pmd))return 0 ; -if (pmd_large (*pmd)) -return pfn_valid (pmd_pfn (*pmd)); +if (pmd_large (*pmd)) { + pfn =pmd_pfn (*pmd); + pfn +=pte_index (addr); + goto check_pfn; + } pte =pte_offset_kernel (pmd, addr);if (pte_none (*pte))return 0 ; -return pfn_valid (pte_pfn (*pte)); + pfn =pte_pfn (*pte); +check_pfn: +if (!pfn_valid (pfn)) +return 0 ; +if (PageHWPoison (pfn_to_page (pfn))) +return 0 ; +return 1 ; }/* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 1 line
Live like a child, think like the god.