Messages in this thread | | | Date | Fri, 20 Feb 2015 10:38:28 -0800 | Subject | Re: [PATCH] x86/compressed: remove unnecessary check | From | Kees Cook <> |
| |
On Fri, Feb 20, 2015 at 9:55 AM, Alexander Kuleshov <kuleshovmail@gmail.com> wrote: > region.start is entry->addr, but we already did check for case when > entry->addr + entry->size < minimum, so no need to check that > region.start is less than minimum.
Nope, that's not correct. Those are separate checks. For example, a region 20..130 would be seen as:
start = 20 size = 110
With minimum = 50, the first check eliminates anything with size <= 30 (since the region would be entirely (20...50) below the minimum).
The second check (the one you're suggesting to remove) raises the bottom of the region up to the minimum:
start = 50
And later on ("Reduce size by any delta from the original address."), the size is also adjusted to notice any changes:
size = 80
Now the region is 50...130: the bottom is cut off to adapt to the minimum location.
Is there some situation you found where this logic isn't working?
-Kees
> > Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com> > --- > arch/x86/boot/compressed/aslr.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c > index bb13763..13424a9 100644 > --- a/arch/x86/boot/compressed/aslr.c > +++ b/arch/x86/boot/compressed/aslr.c > @@ -249,11 +249,7 @@ static void process_e820_entry(struct e820entry *entry, > > region.start = entry->addr; > region.size = entry->size; > - > - /* Potentially raise address to minimum location. */ > - if (region.start < minimum) > - region.start = minimum; > - > + > /* Potentially raise address to meet alignment requirements. */ > region.start = ALIGN(region.start, CONFIG_PHYSICAL_ALIGN); > > -- > 2.3.0.80.g18d0fec >
-- Kees Cook Chrome OS Security
| |