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

Duane Griffin
2007-10-31T10:27:57Z

On 31/10/2007, Nick Piggin <npiggin@suse.de> wrote:
> Well that's probably the best bug report I've ever had, thanks Duane!

Aw, shucks!

> The issue is a silly thinko -- I didn't pay enough attention to the ptrace
> rules in filemap_fault :( partly I think that's because I don't understand
> them and am scared of them ;)
>
> The following minimal patch fixes it here, and should probably be applied to
> 2.6.23 stable as well as 2.6.24. If you could verify it, that would be much
> appreciated.

I can confirm that it fixes the problem, thanks Nick!

Feel free to add:
Tested-by: Duane Griffin <duaneg@dghda.com>

> 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's a bit hairy to force insert page into pagecache and pte into pagetables
> here, given the races.
>
> 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...
>
> Thanks,
> Nick
>
> --
> Duane Griffin noticed a 2.6.23 regression that will cause gdb to hang when
> it tries to access the memory of another process beyond i_size.
>
> This is because the solution to the fault vs invalidate race requires that
> we recheck i_size after the page lock is taken. However in that case, I
> had not accounted for the fact that ptracers are granted an exception to this
> rule.
>
> Cc: Duane Griffin <duaneg@dghda.com>
> Cc: stable@kernel.org
> Signed-off-by: Nick Piggin <npiggin@suse.de
> ---
> Index: linux-2.6/mm/filemap.c
> ===================================================================
> --- linux-2.6.orig/mm/filemap.c
> +++ linux-2.6/mm/filemap.c
> @@ -1374,7 +1374,7 @@ retry_find:
>
> /* Must recheck i_size under page lock */
> size = (i_size_read(inode) + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
> - if (unlikely(vmf->pgoff >= size)) {
> + if (unlikely(vmf->pgoff >= size) && vma->vm_mm == current->mm) {
> unlock_page(page);
> page_cache_release(page);
> goto outside_data_content;
>

Cheers,
Duane.

--
"I never could learn to drink that blood and call it wine" - Bob Dylan