[PATCH] powerpc: Fix size check for hugetlbfs

[PATCH] powerpc: Fix size check for hugetlbfs

[Benjamin Herrenschmidt]

My "slices" address space management code that was added in 2.6.22
implementation of get_unmapped_area() doesn't properly check that the
size is a multiple of the requested page size. This allows userland to
create VMAs that aren't a multiple of the huge page size with hugetlbfs
(since hugetlbfs entirely relies on get_unmapped_area() to do that
checking) which leads to a kernel BUG() when such areas are torn down.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

This should go to 2.6.22.x stable in addition to current 2.6.23-*

Index: linux-work/arch/powerpc/mm/slice.c
===================================================================
--- linux-work.orig/arch/powerpc/mm/slice.c	2007-08-08 15:16:06.000000000 +1000
+++ linux-work/arch/powerpc/mm/slice.c	2007-08-08 15:16:41.000000000 +1000
@@ -405,6 +405,8 @@ unsigned long slice_get_unmapped_area(un
 
 	if (len > mm->task_size)
 		return -ENOMEM;
+	if (len & ((1ul << pshift) - 1))
+		return -EINVAL;
 	if (fixed && (addr & ((1ul << pshift) - 1)))
 		return -EINVAL;
 	if (fixed && addr > (mm->task_size - len))

patch powerpc-fix-size-check-for-hugetlbfs.patch queued to -stable tree

[gregkh]

This is a note to let you know that we have just queued up the patch titled

     Subject: powerpc: Fix size check for hugetlbfs

to the 2.6.22-stable tree.  Its filename is

     powerpc-fix-size-check-for-hugetlbfs.patch

A git repo of this tree can be found at 
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary


>From benh@kernel.crashing.org  Mon Aug 13 16:17:09 2007
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 08 Aug 2007 15:44:15 +1000
Subject: powerpc: Fix size check for hugetlbfs
To: linuxppc-dev list <linuxppc-dev@ozlabs.org>
Cc: Paul Mackerras <paulus@samba.org>, stable@kernel.org
Message-ID: <1186551855.938.164.camel@localhost.localdomain>

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

My "slices" address space management code that was added in 2.6.22
implementation of get_unmapped_area() doesn't properly check that the
size is a multiple of the requested page size. This allows userland to
create VMAs that aren't a multiple of the huge page size with hugetlbfs
(since hugetlbfs entirely relies on get_unmapped_area() to do that
checking) which leads to a kernel BUG() when such areas are torn down.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

--- linux-work.orig/arch/powerpc/mm/slice.c	2007-08-08 15:16:06.000000000 +1000
+++ linux-work/arch/powerpc/mm/slice.c	2007-08-08 15:16:41.000000000 +1000
@@ -405,6 +405,8 @@ unsigned long slice_get_unmapped_area(un
 
 	if (len > mm->task_size)
 		return -ENOMEM;
+	if (len & ((1ul << pshift) - 1))
+		return -EINVAL;
 	if (fixed && (addr & ((1ul << pshift) - 1)))
 		return -EINVAL;
 	if (fixed && addr > (mm->task_size - len))


_______________________________________________
stable mailing list
stable@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/stable



Patches currently in stable-queue which might be from benh@kernel.crashing.org are

queue-2.6.22/ppc-revert-don-t-complain-if-size-cells-0-in-prom_parse.patch
queue-2.6.22/ppc-revert-add-mdio-to-bus-scan-id-list-for-platforms-with-qe-uec.patch
queue-2.6.22/powerpc-fix-size-check-for-hugetlbfs.patch

Re: patch powerpc-fix-size-check-for-hugetlbfs.patch queued to -stable tree

[David Gibson]

On Mon, Aug 13, 2007 at 04:26:27PM -0700, gregkh@suse.de wrote:
> 
> This is a note to let you know that we have just queued up the patch titled
> 
>      Subject: powerpc: Fix size check for hugetlbfs
> 
> to the 2.6.22-stable tree.  Its filename is
> 
>      powerpc-fix-size-check-for-hugetlbfs.patch
> 
> A git repo of this tree can be found at 
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> 
> >From benh@kernel.crashing.org  Mon Aug 13 16:17:09 2007
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Wed, 08 Aug 2007 15:44:15 +1000
> Subject: powerpc: Fix size check for hugetlbfs
> To: linuxppc-dev list <linuxppc-dev@ozlabs.org>
> Cc: Paul Mackerras <paulus@samba.org>, stable@kernel.org
> Message-ID: <1186551855.938.164.camel@localhost.localdomain>
> 
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> My "slices" address space management code that was added in 2.6.22
> implementation of get_unmapped_area() doesn't properly check that the
> size is a multiple of the requested page size. This allows userland to
> create VMAs that aren't a multiple of the huge page size with hugetlbfs
> (since hugetlbfs entirely relies on get_unmapped_area() to do that
> checking) which leads to a kernel BUG() when such areas are torn down.

Ok, I said I was going to look into a libhugetlbfs testcase for this.
Doesn't appear there's specifically a testcase for misaligned size -
I'll add one.

However, it seems the current kernel, on ppc64, gives a testcase
failure on 'misaligned_offset', because it's not failing a mapping
with a non-hugepage aligned file offset.  I'm not sure (yet) if this
failure is also caused by the new slice code, but it seems a likely
candidate.

Still investigating...

-- 
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: patch powerpc-fix-size-check-for-hugetlbfs.patch queued to -stable tree

[David Gibson]

On Tue, Aug 14, 2007 at 04:45:34PM +1000, David Gibson wrote:
> On Mon, Aug 13, 2007 at 04:26:27PM -0700, gregkh@suse.de wrote:
> > 
> > This is a note to let you know that we have just queued up the patch titled
> > 
> >      Subject: powerpc: Fix size check for hugetlbfs
> > 
> > to the 2.6.22-stable tree.  Its filename is
> > 
> >      powerpc-fix-size-check-for-hugetlbfs.patch
> > 
> > A git repo of this tree can be found at 
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > 
> > >From benh@kernel.crashing.org  Mon Aug 13 16:17:09 2007
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Date: Wed, 08 Aug 2007 15:44:15 +1000
> > Subject: powerpc: Fix size check for hugetlbfs
> > To: linuxppc-dev list <linuxppc-dev@ozlabs.org>
> > Cc: Paul Mackerras <paulus@samba.org>, stable@kernel.org
> > Message-ID: <1186551855.938.164.camel@localhost.localdomain>
> > 
> > From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > 
> > My "slices" address space management code that was added in 2.6.22
> > implementation of get_unmapped_area() doesn't properly check that the
> > size is a multiple of the requested page size. This allows userland to
> > create VMAs that aren't a multiple of the huge page size with hugetlbfs
> > (since hugetlbfs entirely relies on get_unmapped_area() to do that
> > checking) which leads to a kernel BUG() when such areas are torn down.
> 
> Ok, I said I was going to look into a libhugetlbfs testcase for this.
> Doesn't appear there's specifically a testcase for misaligned size -
> I'll add one.
> 
> However, it seems the current kernel, on ppc64, gives a testcase
> failure on 'misaligned_offset', because it's not failing a mapping
> with a non-hugepage aligned file offset.  I'm not sure (yet) if this
> failure is also caused by the new slice code, but it seems a likely
> candidate.

I have written a new test for the handling of misaligned addresses,
lengths and offsets for libhugetlbfs.  With it I've verified that this
patch does correct a serious problem with length handling - without
the patch this new testcase causes an oops for me.  It's simpler than
the 'misaligned_offset' testcase in which the misaligned mapping was
just a step to triggering a different and much more subtle kernel bug
(where the misaligned mapping failed, but corrupted some pagetables in
the process).

But the testcase still fails because a mapping with misaligned offset
still succeeds.  Still don't know if this is related to the same slice
changes or not.  Need to do a bisect, but I'm putting it off because I
have some other bugs to chase.

-- 
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: patch powerpc-fix-size-check-for-hugetlbfs.patch queued to -stable tree

[David Gibson]

[snip]
> I have written a new test for the handling of misaligned addresses,
> lengths and offsets for libhugetlbfs.  With it I've verified that this
> patch does correct a serious problem with length handling - without
> the patch this new testcase causes an oops for me.  It's simpler than
> the 'misaligned_offset' testcase in which the misaligned mapping was
> just a step to triggering a different and much more subtle kernel bug
> (where the misaligned mapping failed, but corrupted some pagetables in
> the process).
> 
> But the testcase still fails because a mapping with misaligned offset
> still succeeds.  Still don't know if this is related to the same slice
> changes or not.  Need to do a bisect, but I'm putting it off because I
> have some other bugs to chase.

Ok, I've identified the cause of the problems with misaligned offset.
It's not with the slice code per se, but it did come in with BenH's
series of get_unmapped_area() reworks leading up to commit
4b1d89290b62bb2db476c94c82cf7442aab440c8.

Prior to that, checking that the pgoff was hugepage aligned was the
responsibility of prepare_hugepage_range().  But after that batch of
patches, prepare_hugepage_range() is only called for non-MAP_FIXED
mappings, hence the bug.

I'm not immediately sure how best to fix this:
	- Replace the prepare_hugepage_range() call for non-FIXED
mappings.  That will mean two different paths making calls to
prepare_hugepage_range(), which is a bit ugly.  Especially since the
call for the non-FIXED case will be for an area that is already
hugepage prepared (on powerpc, the only arch with non-trivial
"preparation" necessary) so we'll just be relying on the fact that
prepare_hugepage_range() is idempotent.
	- Remove the test from prepare_hugepage_range() and instead
put it in each of the hugepage get_unmapped_area() variants.  Removes
the ugliness above, traps the problem reasonably early, keeps the test
for pgoff alignment close to the test for address and length
alignment, but means making the check in each of a half-dozen or so
arch versions of hugetlb_get_unmapped_area()
	- Remove the test from prepare_hugepage_range() and instead
but it in the hugetlbfs ->mmap() callback.  Means the fix goes in one
place only, but it's far removed from the similar tests for address
and length alignment.  Also, the mmap() gets quite a long way in
before we catch the misalignment, and we have to roll-back creating
the vma and so forth.

Thoughts?

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