linux-kernel - Re: 2.6.23 regression: accessing invalid mmap'ed memory from gdb causes unkillable spinning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.0.999.0710310756220.30120@woody.linux-foundation.org>
Date:	Wed, 31 Oct 2007 08:11:10 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Nick Piggin <npiggin@...e.de>
cc:	Duane Griffin <duaneg@...da.com>,
	linux-kernel Mailing List <linux-kernel@...r.kernel.org>,
	stable@...nel.org
Subject: Re: 2.6.23 regression: accessing invalid mmap'ed memory from gdb
 causes unkillable spinning



On Wed, 31 Oct 2007, Nick Piggin wrote:
> 
> However I actually don't really like how this all works. I don't like that
> filemap.c should have to know about ptrace, or exactly what ptrace wants here.

It shouldn't. It should just fail when it fails. Then, handle_mm_fault() 
should return an error code, which should cause get_user_pages() to return 
an error code. Which should make ptrace just stop.

So I think your patch is wrong. mm/filemap.c should *not* care about who 
does the fault.  I think the proper patch is something untested like the 
appended...

> It's a bit hairy to force insert page into pagecache and pte into pagetables
> here, given the races.

It's also wrong. They shouldn't be in the page cache, since that can cause 
problems with truncate etc. Maybe it doesn't any more, but it's reasonable 
to believe that a page outside of i_size should not exist.

> In access_process_vm, can't we just zero fill in the case of a sigbus? Linus?
> That will also avoid changing applicatoin behaviour due to a gdb read...

access_process_vm() should just return how many bytes it could fill (which 
means a truncated copy - very including zero bytes - for an error), and 
the caller should decide what the right thing to do is.

But who knows, maybe I missed something.

Duane? Does this fix things for you?

			Linus

---
 mm/filemap.c |   13 ++-----------
 1 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 9940895..188cf5f 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1300,7 +1300,7 @@ int filemap_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 
 	size = (i_size_read(inode) + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
 	if (vmf->pgoff >= size)
-		goto outside_data_content;
+		return VM_FAULT_SIGBUS;
 
 	/* If we don't want any read-ahead, don't bother */
 	if (VM_RandomReadHint(vma))
@@ -1377,7 +1377,7 @@ retry_find:
 	if (unlikely(vmf->pgoff >= size)) {
 		unlock_page(page);
 		page_cache_release(page);
-		goto outside_data_content;
+		return VM_FAULT_SIGBUS;
 	}
 
 	/*
@@ -1388,15 +1388,6 @@ retry_find:
 	vmf->page = page;
 	return ret | VM_FAULT_LOCKED;
 
-outside_data_content:
-	/*
-	 * An external ptracer can access pages that normally aren't
-	 * accessible..
-	 */
-	if (vma->vm_mm == current->mm)
-		return VM_FAULT_SIGBUS;
-
-	/* Fall through to the non-read-ahead case */
 no_cached_page:
 	/*
 	 * We're only likely to ever get here if MADV_RANDOM is in
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/