linux-kernel - Re: 2.6.25-rc7-git2: Reported regressions from 2.6.24

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]

Date:	Sat, 29 Mar 2008 13:42:14 -0700 (PDT)
From:	Christoph Lameter <clameter@....com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	"Rafael J. Wysocki" <rjw@...k.pl>,
	Pawel Staszewski <pstaszewski@...com.pl>,
	LKML <linux-kernel@...r.kernel.org>,
	Adrian Bunk <bunk@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Natalie Protasevich <protasnb@...il.com>
Subject: Re: 2.6.25-rc7-git2: Reported regressions from 2.6.24

On Fri, 28 Mar 2008, Linus Torvalds wrote:

> .. where kmap_atomic() on x86 does:
> 
> 	kmap_atomic() ->
> 	  kmap_atomic_prot() ->
> 	    debug_kmap_atomic_prot() ->
> 	      if (in_irq())
> 	        WARN_ON_ONCE()
> 
> none of which are at all conditional on __GFP_HIGHMEM.

kmap check for PageHighmem and does not do a kmap for regular pages.
So this is actually okay. If the allocation that was performed does not 
allow GFP_HIGHMEM then the kmap will never use a real mapping. The check 
should not trigger.

> > Then clear_highpage calls additional checking functions that have 
> > the effect of generally forbiding zeroing in interrupt context if 
> > CONFIG_HIGHMEM is set. This is wrong and needs to be fixed.
> 
> No. Dammit, the bug is in SLUB.
> 
> If SLUB *ever* calls the page allocator with __GFP_ZERO set, it's a 
> bug, and that has nothing to do with GFP_ATOMIC or anything else. Because 
> SLUB uses its own logic for clearing the result.

Yes it uses its own logic if the object is managed by SLUB but not if the 
object is too big and/or the allocation forwarded to the page allocator 
or for other internal allocations of buffers etc.
 
> Why cannot you just admit it?

Admitting something that is not true is rather difficult.

> Now, _outside_ of SLUB there appear to be other users too, and those users 
> need to either be fixed or we need to allow __GFP_ZERO togethe with 
> GFP_ATOMIC. But the fact is, SLUB had a really stupid bug that it 
> shouldn't have had.

So what you want is to forbid any use of

	alloc_pages(__GFP_ZERO|...)

from an interrupt context? That works fine on most platforms and used to 
work fine on x86 as well until the check was added on January 30th.

If we really want this then the check in prep_zero_page should be changed 
too:

---
 mm/page_alloc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c	2008-03-29 13:40:42.166669333 -0700
+++ linux-2.6/mm/page_alloc.c	2008-03-29 13:41:21.039168276 -0700
@@ -317,7 +317,7 @@ static inline void prep_zero_page(struct
 	 * clear_highpage() will use KM_USER0, so it's a bug to use __GFP_ZERO
 	 * and __GFP_HIGHMEM from hard or soft interrupt context.
 	 */
-	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
+	VM_BUG_ON(in_interrupt());
 	for (i = 0; i < (1 << order); i++)
 		clear_highpage(page + i);
 }
--
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/