[PATCH] tmpfs: restore missing clear_highpage

[PATCH] tmpfs: restore missing clear_highpage

[Hugh Dickins]

tmpfs was misconverted to __GFP_ZERO in 2.6.11.  There's an unusual case in
which shmem_getpage receives the page from its caller instead of allocating.
We must cover this case by clear_highpage before SetPageUptodate, as before.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
Desirable in 2.6.23-stable and 2.6.22-stable and 2.6.16-stable
(and any other 2.6-stable if not already at end of life).

 mm/shmem.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- 2.6.24-rc3-git/mm/shmem.c	2007-10-31 06:18:05.000000000 +0000
+++ linux/mm/shmem.c	2007-11-28 17:01:20.000000000 +0000
@@ -1072,7 +1072,7 @@ shmem_alloc_page(gfp_t gfp, struct shmem
 	pvma.vm_policy = mpol_shared_policy_lookup(&info->policy, idx);
 	pvma.vm_pgoff = idx;
 	pvma.vm_end = PAGE_SIZE;
-	page = alloc_page_vma(gfp | __GFP_ZERO, &pvma, 0);
+	page = alloc_page_vma(gfp, &pvma, 0);
 	mpol_free(pvma.vm_policy);
 	return page;
 }
@@ -1093,7 +1093,7 @@ shmem_swapin(struct shmem_inode_info *in
 static inline struct page *
 shmem_alloc_page(gfp_t gfp,struct shmem_inode_info *info, unsigned long idx)
 {
-	return alloc_page(gfp | __GFP_ZERO);
+	return alloc_page(gfp);
 }
 #endif
 
@@ -1306,6 +1306,7 @@ repeat:
 
 		info->alloced++;
 		spin_unlock(&info->lock);
+		clear_highpage(filepage);
 		flush_dcache_page(filepage);
 		SetPageUptodate(filepage);
 	}

Re: [PATCH] tmpfs: restore missing clear_highpage

[Adrian Bunk]

On Wed, Nov 28, 2007 at 06:55:10PM +0000, Hugh Dickins wrote:
> tmpfs was misconverted to __GFP_ZERO in 2.6.11.  There's an unusual case in
> which shmem_getpage receives the page from its caller instead of allocating.
> We must cover this case by clear_highpage before SetPageUptodate, as before.
> 
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---
> Desirable in 2.6.23-stable and 2.6.22-stable and 2.6.16-stable
>...

Thanks, applied to 2.6.16.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] tmpfs: restore missing clear_highpage

[Chuck Ebbert]

On 11/28/2007 01:55 PM, Hugh Dickins wrote:
> tmpfs was misconverted to __GFP_ZERO in 2.6.11.  There's an unusual case in
> which shmem_getpage receives the page from its caller instead of allocating.
> We must cover this case by clear_highpage before SetPageUptodate, as before.
> 
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---

What are the symptoms of the bug this fixes?

Re: [PATCH] tmpfs: restore missing clear_highpage

[Hugh Dickins]

On Tue, 11 Dec 2007, Chuck Ebbert wrote:
> On 11/28/2007 01:55 PM, Hugh Dickins wrote:
> > tmpfs was misconverted to __GFP_ZERO in 2.6.11.  There's an unusual case in
> > which shmem_getpage receives the page from its caller instead of allocating.
> > We must cover this case by clear_highpage before SetPageUptodate, as before.
> > 
> > Signed-off-by: Hugh Dickins <hugh@veritas.com>
> > ---
> 
> What are the symptoms of the bug this fixes?

I've not seen it in practice, just noticed it while working on that
area in the code.  What's the polite way of describing these things
in public?  It's a vulnerability which might allow an attacker to
access data from inside the kernel which should have been zeroed -
in very limited circumstances I'd prefer not to have to devise and
announce.

It would also be wrong data, so could for example crash any program
rightly relying on uninitialized static data to be zeroed - in the
unlikely event that its data was coming via this route (in most setups
it never can do, perhaps I'd conclude that's true of all setups).  It
has escaped notice for nearly three years, so it's not a commonplace.

Further discussion offline if you like!

Hugh

Re: [PATCH] tmpfs: restore missing clear_highpage

[Adrian Bunk]

On Wed, Dec 12, 2007 at 05:01:51AM +0000, Hugh Dickins wrote:
> On Tue, 11 Dec 2007, Chuck Ebbert wrote:
> > On 11/28/2007 01:55 PM, Hugh Dickins wrote:
> > > tmpfs was misconverted to __GFP_ZERO in 2.6.11.  There's an unusual case in
> > > which shmem_getpage receives the page from its caller instead of allocating.
> > > We must cover this case by clear_highpage before SetPageUptodate, as before.
> > > 
> > > Signed-off-by: Hugh Dickins <hugh@veritas.com>
> > > ---
> > 
> > What are the symptoms of the bug this fixes?
> 
> I've not seen it in practice, just noticed it while working on that
> area in the code.  What's the polite way of describing these things
> in public?  It's a vulnerability which might allow an attacker to
> access data from inside the kernel which should have been zeroed -
> in very limited circumstances I'd prefer not to have to devise and
> announce.
> 
> It would also be wrong data, so could for example crash any program
> rightly relying on uninitialized static data to be zeroed - in the
> unlikely event that its data was coming via this route (in most setups
> it never can do, perhaps I'd conclude that's true of all setups).  It
> has escaped notice for nearly three years, so it's not a commonplace.
> 
> Further discussion offline if you like!

Can we get or is there already a CVE number?

> Hugh

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH] tmpfs: restore missing clear_highpage

[Hugh Dickins]

On Sat, 15 Dec 2007, Adrian Bunk wrote:
> On Wed, Dec 12, 2007 at 05:01:51AM +0000, Hugh Dickins wrote:
> > On Tue, 11 Dec 2007, Chuck Ebbert wrote:
> > > On 11/28/2007 01:55 PM, Hugh Dickins wrote:
> > > > tmpfs was misconverted to __GFP_ZERO in 2.6.11.  There's an unusual case in
> > > > which shmem_getpage receives the page from its caller instead of allocating.
> > > > We must cover this case by clear_highpage before SetPageUptodate, as before.
> > > > 
> > > > Signed-off-by: Hugh Dickins <hugh@veritas.com>
> > > > ---
> > > 
> > > What are the symptoms of the bug this fixes?
> > 
> > I've not seen it in practice, just noticed it while working on that
> > area in the code.  What's the polite way of describing these things
> > in public?  It's a vulnerability which might allow an attacker to
> > access data from inside the kernel which should have been zeroed -
> > in very limited circumstances I'd prefer not to have to devise and
> > announce.
> > 
> > It would also be wrong data, so could for example crash any program
> > rightly relying on uninitialized static data to be zeroed - in the
> > unlikely event that its data was coming via this route (in most setups
> > it never can do, perhaps I'd conclude that's true of all setups).  It
> > has escaped notice for nearly three years, so it's not a commonplace.
> > 
> > Further discussion offline if you like!
> 
> Can we get or is there already a CVE number?

I've not asked for one, so I don't suppose there is one: do go ahead
and get one if you wish (though of course it would have been better
earlier, so 2.6.22.15 and 2.6.23.10 could have cited it along with
their fix: sorry about that).

Hugh