Skip to contentSkip to conversation

enable to read region 5 from /dev/kmem

PATCH In review KAMEZAWA Hiroyuki 4 participants 12 messages Jul 1, 2005 – Jul 13, 2005 Char and misc drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 enable to read region 5 from /dev/kmem

This patch allows users to read region 5 from /dev/kmem and enhances range check of it.

Regards,
-- Kamezawa Hiroyuki.

enables reading region 5 from /dev/kmem

This patch enables to read region 5 from /dev/kmem. And 'cat /dev/kmem' never cause panic. "Don't do that" is good advise :).

---

 linux-2.6.13-rc1-kamezawa/drivers/char/mem.c         |    3 +-
 linux-2.6.13-rc1-kamezawa/include/asm-ia64/uaccess.h |   22 +++++++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)
include/asm-ia64/uaccess.h renamed from include/asm-ia64/uaccess.h~ia64_region5_kmem +20 -2
diff -puN include/asm-ia64/uaccess.h~ia64_region5_kmem include/asm-ia64/uaccess.h
@@ -395,8 +395,26 @@ xlate_dev_kmem_ptr (char * p)
 {
 	struct page *page;
 	char * ptr;
-
-	page = virt_to_page((unsigned long)p >> PAGE_SHIFT);
+	unsigned long pfn;
+	int region;
+	region = REGION_NUMBER(p);
+	switch(region) {
+		case 5:
+			return p;
+		case 6:
+			pfn = ((unsigned long)p - __IA64_UNCACHED_OFFSET) >> PAGE_SHIFT;
+			break;
+		case 7:
+			pfn = __pa(p) >> PAGE_SHIFT;
+			break;
+		default:
+			return NULL;
+	}
+	if (!pfn_valid(pfn))
+		return NULL;
+	if (region == 6)
+		return p;
+	page = pfn_to_page(pfn);
 	if (PageUncached(page))
 		ptr = (char *)__pa(p) + __IA64_UNCACHED_OFFSET;
 	else
drivers/char/mem.c renamed from drivers/char/mem.c~ia64_region5_kmem +2 -1
diff -puN drivers/char/mem.c~ia64_region5_kmem drivers/char/mem.c
@@ -155,7 +155,8 @@ static ssize_t read_mem(struct file * fi
 		 * by the kernel or data corruption may occur
 		 */
 		ptr = xlate_dev_mem_ptr(p);
-
+		if (!ptr)
+			return -EFAULT;
 		if (copy_to_user(buf, ptr, sz))
 			return -EFAULT;
 		buf += sz;

_

#1 lore raw
Quoted 3 lines

This patch allows users to read region 5 from /dev/kmem and enhances range check of it.

I'm uneasy about this change to drivers/char/mem.c. Won't it stop those architectures that have memory at physical 0x0 from accessing it?

ptr = xlate_dev_mem_ptr(p);
-
+ if (!ptr)
+ return -EFAULT

-Tony

#2 lore raw
In reply to KAMEZAWA Hiroyuki
Quoted 7 lines · Tony
Quoted 2 lines

This patch allows users to read region 5 from /dev/kmem and enhances range check of it.

I'm uneasy about this change to drivers/char/mem.c. Won't it stop those architectures that have memory at physical 0x0 from accessing it?

Sorry, I sent wrong patch.
This !ptr check should be done in read_kmem and kmem doesn't have address 0x0.

Another way is
=
#define KMEM_ERR_PTR (void *)-1

if (ptr = KMEM_ERR_PTR)
return -EFAULT
=
or no check.

By the way, can ia64's copy_to_user(from, buf, size) catch bad "from" ?

Regards,
-- Kamezawa

Quoted 12 lines
(quoted diff) +2 -3
 		ptr = xlate_dev_mem_ptr(p);
-
+		if (!ptr)
+			return -EFAULT
 
-Tony
-

To unsubscribe from this list: send the line "unsubscribe linux-ia64" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
In reply to KAMEZAWA Hiroyuki

KAMEZAWA Hiroyuki wrote on Monday, July 11, 2005 5:14 PM

Quoted 1 line

By the way, can ia64's copy_to_user(from, buf, size) catch bad "from" ?

Yes, it has exception handler for both source and destination address.

#4 lore raw
In reply to KAMEZAWA Hiroyuki
Quoted 7 lines · Kenneth W

KAMEZAWA Hiroyuki wrote on Monday, July 11, 2005 5:14 PM

Quoted 1 line

By the way, can ia64's copy_to_user(from, buf, size) catch bad "from" ?

Yes, it has exception handler for both source and destination address.

Thank you.

One more question:
Is a user allowd to access region 0-4 from /dev/kmem ? (I thnk it's not allowed) This patch disables that.

Regards,
Kamezawa Hiroyuki

#5 lore raw
In reply to KAMEZAWA Hiroyuki

drivers/char/mem.c static ssize_t read_mem(struct file * fi

Quoted 13 lines · Tony
Quoted 2 lines

This patch allows users to read region 5 from /dev/kmem and enhances range check of it.

I'm uneasy about this change to drivers/char/mem.c. Won't it stop those architectures that have memory at physical 0x0 from accessing it?

drivers/char/mem.c renamed from drivers/char/mem.c~ia64_region5_kmem +2 -1
 		ptr = xlate_dev_mem_ptr(p);
-
+		if (!ptr)
+			return -EFAULT

I confirmed this is not necessary. I send fixed one.

Note:
linux-2.6.13-rc1 panic when "cat /dev/kmem > /dev/null".
I'ts because xlate_dev_kmem_ptr() doesn't uses pfn_valid() to access page struct.

Thanks,
Kamezawa Hiroyuki.
=

This patch allows to read region 5 of the kernel from /dev/kmem.

Index: linux-2.6.13-rc1/include/asm-ia64/uaccess.h
=================================--- linux-2.6.13-rc1.orig/include/asm-ia64/uaccess.h 2005-07-12 14:40:37.000000000 +0900
+++ linux-2.6.13-rc1/include/asm-ia64/uaccess.h 2005-07-12 15:47:56.000000000 +0900
@@ -395,8 +395,27 @@
{
struct page *page;
char * ptr;
+ unsigned long pfn;
+ int region;
+
+ region = REGION_NUMBER(p);
+
+ switch (region) {
+ case 6:
+ pfn = ((unsigned long)p - __IA64_UNCACHED_OFFSET) >> PAGE_SHIFT;
+ break;
+ case 7:
+ pfn = __pa(p) >> PAGE_SHIFT;
+ break;
+ default:
+ return p;
+ }
+ if (!pfn_valid(pfn))
+ return NULL; /* cause access error in copy_user */
+ if (region = 6)
+ return p;
+ page = pfn_to_page(pfn);

- page = virt_to_page((unsigned long)p >> PAGE_SHIFT);
if (PageUncached(page))
ptr = (char *)__pa(p) + __IA64_UNCACHED_OFFSET;
else

#6 lore raw
In reply to KAMEZAWA Hiroyuki

This is better. But allowing access to region 6 through /dev/kmem will open up a whole new set of problems. If the user does an uncacheable read through region 6 from a memory address that is mapped cacheably in region 7, then the processor may machine check [SDM 4.4.1, page 2:64, last
paragraph: "It is recommended that the processor report a Machine Check abort if the following memory attribute aliasing is detected: * cache-hit on an uncacheable page"]

So I think we should disallow access to region 6.

-Tony

#7 lore raw
In reply to KAMEZAWA Hiroyuki
Quoted 10 lines · Tony

This is better. But allowing access to region 6 through /dev/kmem will open up a whole new set of problems. If the user does an uncacheable read through region 6 from a memory address that is mapped cacheably in region 7, then the processor may machine check [SDM 4.4.1, page 2:64, last
paragraph: "It is recommended that the processor report a Machine Check abort if the following memory attribute aliasing is detected: * cache-hit on an uncacheable page"]

So I think we should disallow access to region 6.

/dev/[k]mem should use the EFI memory attributes to determine accessibility. This is being done to some degree already (well, I didn't check the current code, but it's supposed to be there).

--david

#8 lore raw
In reply to KAMEZAWA Hiroyuki
Quoted 3 lines

/dev/[k]mem should use the EFI memory attributes to determine accessibility. This is being done to some degree already (well, I didn't check the current code, but it's supposed to be there).

The current code relies on checking "PageUncached(page)" to decide whether to convert a region 7 access into a region 6 access.

But this patch would allow any address in region 6 to be accessed (and so we'd see an MCA if the address had been cached via a region 7 access ... or for that matter a user-mode access).

Now, I suppose we could build on this patch so that if a user tries to access a cacheable page through region 6, we'll quietly swap it for an cacheable access to the matching address in region 7. But I'm not sure whether this is really useful. Region 6 and 7 provide access to the same memory, but with different cache attributes.

It comes down to what the real semantics of /dev/kmem are supposed to be.

-Tony

#9 lore raw
In reply to KAMEZAWA Hiroyuki
Quoted 2 lines · Tony

It comes down to what the real semantics of /dev/kmem are supposed to be.

That is certainly true. I thought we had settled that pages with the WB attribute would _only_ be accessible in a cacheable fashion and pages without the WB attribute would _only_ be accessible uncached.

--david

#10 lore raw
In reply to KAMEZAWA Hiroyuki
Quoted 6 lines
Quoted 2 lines

It comes down to what the real semantics of /dev/kmem are supposed to be.

That is certainly true. I thought we had settled that pages with the WB attribute would _only_ be accessible in a cacheable fashion and pages without the WB attribute would _only_ be accessible uncached.

So what is the right thing to do when a user goes through /dev/kmem and tries to violate this rule? For region 7 addresses we silently redirect the access to region 6 for uncacheable pages. At the time this seemed a reasonable thing to do as it might be hard for a kmem using program to track down whether a page is marked uncacheable ... and the "kernel virtual = physical + PAGE_OFFSET" rule works on many architectures, so doing this may magically make some kmem readers get the right answers without crashing.

We could do a similar thing for region 6 and silently remap to region 7 for pages that should only be accessed cacheably ... but what if the user going through /dev/kmem and seeking into region 6 knows what they are doing and wants to perform uncacheable access? If the user is accessing region 6, then they didn't get there by blind chance.

Which gets back to the semantics of /dev/kmem.

-Tony

#11 lore raw
In reply to KAMEZAWA Hiroyuki

Hi, Tony

My idea is "A user needs O_SYNC flag to access uncachable area" like other archs.
With this, a user can avoid accessing region 6 by mistake, to some extent.

- Kamezawa

Quoted 27 lines · Tony
Quoted 6 lines
Quoted 2 lines

It comes down to what the real semantics of /dev/kmem are supposed to be.

That is certainly true. I thought we had settled that pages with the WB attribute would _only_ be accessible in a cacheable fashion and pages without the WB attribute would _only_ be accessible uncached.

So what is the right thing to do when a user goes through /dev/kmem and tries to violate this rule? For region 7 addresses we silently redirect the access to region 6 for uncacheable pages. At the time this seemed a reasonable thing to do as it might be hard for a kmem using program to track down whether a page is marked uncacheable ... and the "kernel virtual = physical + PAGE_OFFSET" rule works on many architectures, so doing this may magically make some kmem readers get the right answers without crashing.

We could do a similar thing for region 6 and silently remap to region 7 for pages that should only be accessed cacheably ... but what if the user going through /dev/kmem and seeking into region 6 knows what they are doing and wants to perform uncacheable access? If the user is accessing region 6, then they didn't get there by blind chance.

Which gets back to the semantics of /dev/kmem.

-Tony