linux-kernel - Re: [PATCH] x86: seperate reserve_early and reserve_early_overlap

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]

Message-ID: <4B17199F.1000804@zytor.com>
Date:	Wed, 02 Dec 2009 17:51:27 -0800
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Yinghai Lu <yinghai@...nel.org>
CC:	Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
	Jeremy Fitzhardinge <jeremy@...p.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86: seperate reserve_early and reserve_early_overlap_check

On 11/25/2009 12:58 AM, Yinghai Lu wrote:
> 
> when the area is from find_e820_area(), it could be overlapped with others.
> 
> so just add it directly. the new reserve_early()
> 
> and rename old reserve_early() to reserve_early_overlap_check()
> 
> Signed-off-by: Yinghai Lu <yinghai@...nel.org>

Hi Yinghai,

I had this patch in my queue but it looks like I had overlooked it as it
arrived during the U.S. holiday; I apologize profusely!

I'm concerned about several things with this patch, which doesn't mean
it isn't fulfilling a genuine need:

1. Renaming reserve_early() to reserve_early_overlap_check() is most
likely going to get overlooked, and people will use the "new"
reserve_early() thinking that they got the old one.

2. This creates overlapping ranges in the reservation array itself.

What it looks to me is what we need is actually a
reserve_early_clobber() which does what the current reserve_early() does
except that it ignores the overlap_ok flag on existing reservations.

I have attached an untested patch to do that.  Note that I don't have
any callers for reserve_early_clobber(), since one effect of changing
the semantics of an existing function in the way you did is that the
patch contains the call sites that *didn't* need modification rather
than the one that *did* need modification.  The call sites that want the
new semantics need to be modified.  As such, it's possible that the
comment I added is completely wrong, I really need some further
information on this.

[Note: the function __reserve_early() hasn't actually changed; I just
moved it ahead of drop_overlaps().]

Sorry again for the delay.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

View attachment "diff" of type "text/plain" (4924 bytes)