Fix crash when ptrace poking hugepage areas

Fix crash when ptrace poking hugepage areas

[David Gibson]

Bill, does this look like the correct fix for the problem to you?  If
so, please apply Andrew.

set_page_dirty() will not cope with being handed a page * which is
part of a compound page, but not the master page in that compound
page.  This case can occur via access_process_vm() if you attempt to
write to another process's hugepage memory area using ptrace()
(causing an oops or hang).

This patch fixes the bug by first resolving the page * to the compound
page's master page.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Index: working-2.6/mm/page-writeback.c
===================================================================
--- working-2.6.orig/mm/page-writeback.c	2005-11-29 15:51:11.000000000 +1100
+++ working-2.6/mm/page-writeback.c	2005-11-29 15:52:09.000000000 +1100
@@ -660,7 +660,12 @@ EXPORT_SYMBOL(redirty_page_for_writepage
  */
 int fastcall set_page_dirty(struct page *page)
 {
-	struct address_space *mapping = page_mapping(page);
+	struct address_space *mapping;
+
+	if (unlikely(PageCompound(page)))
+		page = (struct page *)page_private(page);
+
+	mapping = page_mapping(page);
 
 	if (likely(mapping)) {
 		int (*spd)(struct page *) = mapping->a_ops->set_page_dirty;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix crash when ptrace poking hugepage areas

[Andrew Morton]

David Gibson <david@gibson.dropbear.id.au> wrote:
>
> Bill, does this look like the correct fix for the problem to you?  If
> so, please apply Andrew.
> 
> set_page_dirty() will not cope with being handed a page * which is
> part of a compound page, but not the master page in that compound
> page.  This case can occur via access_process_vm() if you attempt to
> write to another process's hugepage memory area using ptrace()
> (causing an oops or hang).
> 
> This patch fixes the bug by first resolving the page * to the compound
> page's master page.

We already have to handle this situation for direct-io read()s into
hugepages.  bio_set_pages_dirty() does

		if (page && !PageCompound(page))
			set_page_dirty_lock(page);

It's such a rare case that it's probably best to continue to do this in the
caller rather than in the callee.  That's access_process_vm().

Unless there's a reason why we actually want the compound page to be marked
dirty?  If there is, then direct-io has a problem.  

Re: Fix crash when ptrace poking hugepage areas

[David Gibson]

On Mon, Nov 28, 2005 at 09:18:07PM -0800, Andrew Morton wrote:
> David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > Bill, does this look like the correct fix for the problem to you?  If
> > so, please apply Andrew.
> > 
> > set_page_dirty() will not cope with being handed a page * which is
> > part of a compound page, but not the master page in that compound
> > page.  This case can occur via access_process_vm() if you attempt to
> > write to another process's hugepage memory area using ptrace()
> > (causing an oops or hang).
> > 
> > This patch fixes the bug by first resolving the page * to the compound
> > page's master page.
> 
> We already have to handle this situation for direct-io read()s into
> hugepages.  bio_set_pages_dirty() does
> 
> 		if (page && !PageCompound(page))
> 			set_page_dirty_lock(page);
> 
> It's such a rare case that it's probably best to continue to do this in the
> caller rather than in the callee.  That's access_process_vm().

Good call, revised patch below.

> Unless there's a reason why we actually want the compound page to be marked
> dirty?  If there is, then direct-io has a problem.  

I don't think so.  Since hugepages are never disk-backed, I think the
PageDirty flag is more or less irrelevant.

Fix crash when ptrace poking hugepage areas

set_page_dirty() will not cope with being handed a page * which is
part of a compound page, but not the master page in that compound
page.  This case can occur via access_process_vm() if you attemp to
write to another process's hugepage memory area using ptrace()
(causing an oops or hang).

This patch fixes the bug by only calling set_page_dirty() from
access_process_vm() if the page is not a compound page.  We already
use a similar fix in bio_set_pages_dirty() for the case of direct io
to hugepages.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Index: working-2.6/kernel/ptrace.c
===================================================================
--- working-2.6.orig/kernel/ptrace.c	2005-11-29 16:37:15.000000000 +1100
+++ working-2.6/kernel/ptrace.c	2005-11-29 16:37:32.000000000 +1100
@@ -241,7 +241,8 @@ int access_process_vm(struct task_struct
 		if (write) {
 			copy_to_user_page(vma, page, addr,
 					  maddr + offset, buf, bytes);
-			set_page_dirty_lock(page);
+			if (!PageCompound(page))
+				set_page_dirty_lock(page);
 		} else {
 			copy_from_user_page(vma, page, addr,
 					    buf, maddr + offset, bytes);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: Fix crash when ptrace poking hugepage areas

[William Lee Irwin III]

On Tue, Nov 29, 2005 at 04:06:28PM +1100, David Gibson wrote:
> Bill, does this look like the correct fix for the problem to you?  If
> so, please apply Andrew.
> set_page_dirty() will not cope with being handed a page * which is
> part of a compound page, but not the master page in that compound
> page.  This case can occur via access_process_vm() if you attempt to
> write to another process's hugepage memory area using ptrace()
> (causing an oops or hang).
> This patch fixes the bug by first resolving the page * to the compound
> page's master page.
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

akpm had already responded, but my general response would have been
"Why on earth would you mark a hugepage dirty?" or similar.


-- wli

Re: Fix crash when ptrace poking hugepage areas

[William Lee Irwin III]

On Tue, Nov 29, 2005 at 04:41:36PM +1100, David Gibson wrote:
> This patch fixes the bug by only calling set_page_dirty() from
> access_process_vm() if the page is not a compound page.  We already
> use a similar fix in bio_set_pages_dirty() for the case of direct io
> to hugepages.
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Acked-by: William Irwin <wli@holomorphy.com>


-- wli