[PATCH] enable to read region 5 from /dev/kmem

[PATCH] enable to read region 5 from /dev/kmem

[KAMEZAWA Hiroyuki]

[-- Attachment #1: Type: text/plain, Size: 120 bytes --]

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

Regards,
-- Kamezawa Hiroyuki.

[-- Attachment #2: ia64_region5_kmem.patch --]
[-- Type: text/plain, Size: 1831 bytes --]


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 :).

Signed-Off-By: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


---

 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(-)

diff -puN include/asm-ia64/uaccess.h~ia64_region5_kmem include/asm-ia64/uaccess.h
--- linux-2.6.13-rc1/include/asm-ia64/uaccess.h~ia64_region5_kmem	2005-07-01 17:10:05.000000000 +0900
+++ linux-2.6.13-rc1-kamezawa/include/asm-ia64/uaccess.h	2005-07-01 17:12:51.000000000 +0900
@@ -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
diff -puN drivers/char/mem.c~ia64_region5_kmem drivers/char/mem.c
--- linux-2.6.13-rc1/drivers/char/mem.c~ia64_region5_kmem	2005-07-01 17:13:06.000000000 +0900
+++ linux-2.6.13-rc1-kamezawa/drivers/char/mem.c	2005-07-01 17:13:15.000000000 +0900
@@ -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;

_

RE: [PATCH] enable to read region 5 from /dev/kmem

[Luck, Tony]

>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

Re: [PATCH] enable to read region 5 from /dev/kmem

[KAMEZAWA Hiroyuki]

Luck, Tony wrote:
>>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

> 
>  		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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH] enable to read region 5 from /dev/kmem

[Chen, Kenneth W]

KAMEZAWA Hiroyuki wrote on Monday, July 11, 2005 5:14 PM
> 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.

Re: [PATCH] enable to read region 5 from /dev/kmem

[KAMEZAWA Hiroyuki]

Chen, Kenneth W wrote:
> KAMEZAWA Hiroyuki wrote on Monday, July 11, 2005 5:14 PM
> 
>>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

Re: [PATCH] enable to read region 5 from /dev/kmem

[KAMEZAWA Hiroyuki]

Luck, Tony wrote:
>>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
> 
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.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

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

RE: [PATCH] enable to read region 5 from /dev/kmem

[Luck, 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.

-Tony

Re: [PATCH] enable to read region 5 from /dev/kmem

[david mosberger]

On 7/12/05, Luck, Tony <tony.luck@intel.com> wrote:
> 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

RE: [PATCH] enable to read region 5 from /dev/kmem

[Luck, Tony]

>/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

Re: [PATCH] enable to read region 5 from /dev/kmem

[david mosberger]

On 7/12/05, Luck, Tony <tony.luck@intel.com> wrote:

> 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

RE: [PATCH] enable to read region 5 from /dev/kmem

[Luck, 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.

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

Re: [PATCH] enable to read region 5 from /dev/kmem

[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


Luck, Tony wrote:
>>>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
>