All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <jweiner@redhat.com>
To: Hillf Danton <dhillf@gmail.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/huge_memory: Clean up typo when copying user highpage
Date: Wed, 12 Oct 2011 11:56:46 -0400	[thread overview]
Message-ID: <20111012155646.GC6478@redhat.com> (raw)
In-Reply-To: <CAJd=RBBuwmcV8srUyPGnKUp=RPKvsSd+4BbLrh--aHFGC5s7+g@mail.gmail.com>

On Wed, Oct 12, 2011 at 10:39:36PM +0800, Hillf Danton wrote:
> Hi Andrea
> 
> When copying user highpage, the PAGE_SHIFT in the third parameter is a typo,
> I think, and is replaced with PAGE_SIZE.

This is a pretty nasty data corruption bug, so 'clean up' might be a
bit of an understatement ;-)

Nice catch.

Would you mind extending the changelog to include a problem
description?  Feel free to steal from this:

	The THP copy-on-write handler falls back to regular-sized
	pages for a huge page replacement upon allocation failure or
	if THP has been individually disabled in the target VMA.  The
	loop responsible for copying page-sized chunks accidentally
	uses multiples of PAGE_SHIFT instead of PAGE_SIZE as the byte
	offset into the original huge page, though, and the
	COW-breaking task ends up with a corrupt copy of the data.

> Signed-off-by: Hillf Danton <dhillf@gmail.com>

Acked-by: Johannes Weiner <jweiner@redhat.com>

> --- a/mm/huge_memory.c	Sat Aug 13 11:45:14 2011
> +++ b/mm/huge_memory.c	Wed Oct 12 22:26:15 2011
> @@ -829,7 +829,7 @@ static int do_huge_pmd_wp_page_fallback(
> 
>  	for (i = 0; i < HPAGE_PMD_NR; i++) {
>  		copy_user_highpage(pages[i], page + i,
> -				   haddr + PAGE_SHIFT*i, vma);
> +				   haddr + PAGE_SIZE * i, vma);
>  		__SetPageUptodate(pages[i]);
>  		cond_resched();
>  	}

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <jweiner@redhat.com>
To: Hillf Danton <dhillf@gmail.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/huge_memory: Clean up typo when copying user highpage
Date: Wed, 12 Oct 2011 11:56:46 -0400	[thread overview]
Message-ID: <20111012155646.GC6478@redhat.com> (raw)
In-Reply-To: <CAJd=RBBuwmcV8srUyPGnKUp=RPKvsSd+4BbLrh--aHFGC5s7+g@mail.gmail.com>

On Wed, Oct 12, 2011 at 10:39:36PM +0800, Hillf Danton wrote:
> Hi Andrea
> 
> When copying user highpage, the PAGE_SHIFT in the third parameter is a typo,
> I think, and is replaced with PAGE_SIZE.

This is a pretty nasty data corruption bug, so 'clean up' might be a
bit of an understatement ;-)

Nice catch.

Would you mind extending the changelog to include a problem
description?  Feel free to steal from this:

	The THP copy-on-write handler falls back to regular-sized
	pages for a huge page replacement upon allocation failure or
	if THP has been individually disabled in the target VMA.  The
	loop responsible for copying page-sized chunks accidentally
	uses multiples of PAGE_SHIFT instead of PAGE_SIZE as the byte
	offset into the original huge page, though, and the
	COW-breaking task ends up with a corrupt copy of the data.

> Signed-off-by: Hillf Danton <dhillf@gmail.com>

Acked-by: Johannes Weiner <jweiner@redhat.com>

> --- a/mm/huge_memory.c	Sat Aug 13 11:45:14 2011
> +++ b/mm/huge_memory.c	Wed Oct 12 22:26:15 2011
> @@ -829,7 +829,7 @@ static int do_huge_pmd_wp_page_fallback(
> 
>  	for (i = 0; i < HPAGE_PMD_NR; i++) {
>  		copy_user_highpage(pages[i], page + i,
> -				   haddr + PAGE_SHIFT*i, vma);
> +				   haddr + PAGE_SIZE * i, vma);
>  		__SetPageUptodate(pages[i]);
>  		cond_resched();
>  	}

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-10-12 15:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-12 14:39 [PATCH] mm/huge_memory: Clean up typo when copying user highpage Hillf Danton
2011-10-12 14:39 ` Hillf Danton
2011-10-12 15:56 ` Johannes Weiner [this message]
2011-10-12 15:56   ` Johannes Weiner
2011-10-12 17:51 ` Andrea Arcangeli
2011-10-12 17:51   ` Andrea Arcangeli
2011-10-12 20:42   ` Andrew Morton
2011-10-12 20:42     ` Andrew Morton
2011-10-12 22:24     ` Andrea Arcangeli
2011-10-12 22:24       ` Andrea Arcangeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111012155646.GC6478@redhat.com \
    --to=jweiner@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dhillf@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.