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@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx>

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/