linux-kernel - Re: Linux 2.6.20-rc7

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-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0701311609210.3632@woody.linux-foundation.org>
Date:	Wed, 31 Jan 2007 16:19:06 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	PaweÅ Sikora <pluto@...k.net>
cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Christoph Hellwig <hch@....de>,
	Pekka Enberg <penberg@...helsinki.fi>,
	"Siddha, Suresh B" <suresh.b.siddha@...el.com>,
	Martin Peschke <mp3@...ibm.com>, Andrew Morton <akpm@...l.org>,
	Adrian Bunk <bunk@...sta.de>,
	Nick Piggin <nickpiggin@...oo.com.au>
Subject: Re: Linux 2.6.20-rc7

On Thu, 1 Feb 2007, PaweÅ Sikora wrote:
> 
> 2.6.18 works, 2.6.19-rc1 doesn't work.
> git bisect found this bad commit:

Git bisect rocks.

I'll give myself yet abother pat on the back for writing it. You can never 
encourage genius like that too much.

> commit e80ee884ae0e3794ef2b65a18a767d502ad712ee
> Author: Nick Piggin <nickpiggin@...oo.com.au>
> Date:   Wed Oct 4 02:15:23 2006 -0700
> 
>     [PATCH] mm: micro optimise zone_watermark_ok
>
> reverting mentioned commit removes the oops.

Ok, that commit is just totally broken.

If "free_pages" turns negative (which it can, since it's just doing a

	long free_pages = z->free_pages - (1 << order) + 1;

to initialize it, and for all we know, you have an empty or close-to-empty 
zone or two, the whole test to do

	if (free_pages <= min + z->lowmem_reserve[classzone_idx])
		return 0;

gets broken, because the negative 'free_pages' will look like a huge 
unsigned positive number (and we'll make it unsigned becaue 'min' got 
turned unsigned). There was a reason that thing was signed in the first 
place, and neither me nor Andrew noticed.

Bad Nick. And bad me and Andrew for not noticing.

I should either revert that commit or just check for "free_pages" being 
negative. The latter, in many ways, is probably better, because generally 
we simply should never work with negative numbers in the kernel, so when 
something potentially goes negative, we're probably just better off always 
testing it explicitly anyway.

Nick, Andrew, any preferences? 

			Linus