Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1440055 > unrolled thread

Re: [tip:x86/boot] x86/KASLR: Fix boot crash with certain memory configurations

Started byBaoquan He <bhe@redhat.com>
First post2016-07-10 11:20 +0200
Last post2016-07-10 17:20 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [tip:x86/boot] x86/KASLR: Fix boot crash with certain memory  configurations Baoquan He <bhe@redhat.com> - 2016-07-10 11:20 +0200
    Re: [tip:x86/boot] x86/KASLR: Fix boot crash with certain memory  configurations Ingo Molnar <mingo@kernel.org> - 2016-07-10 13:30 +0200
      Re: [tip:x86/boot] x86/KASLR: Fix boot crash with certain memory  configurations Baoquan He <bhe@redhat.com> - 2016-07-10 17:20 +0200

#1440055 — Re: [tip:x86/boot] x86/KASLR: Fix boot crash with certain memory configurations

FromBaoquan He <bhe@redhat.com>
Date2016-07-10 11:20 +0200
SubjectRe: [tip:x86/boot] x86/KASLR: Fix boot crash with certain memory configurations
Message-ID<rTj45-76I-1@gated-at.bofh.it>
Hi Ingo,

I am sorry the previous post didn't contain formal patch log. I made a
new one as below. The boot crash could not only happen with certain
memory. Because of this code bug the regions which need be avoided like
the zipped kernel with its unzipping running code, initrd, kernel
command line could be corrupted if mem_avoid_overlap() can't find the
overlap region with the lowest address. But it's very lucky that
Xiaolong's system which has only 300M memory can always reproduce it.
I checked the boog log and found on his system no any other slot can be
chosen except for the original one. If we have a system with large memory
it may not be easy to hit it, at least with low probability since there
are many candidate slots.



From 8f48aa39f3e49f9c1a9bb8ee61547dda7c2c05c3 Mon Sep 17 00:00:00 2001
From: Baoquan He <bhe@redhat.com>
Date: Fri, 1 Jul 2016 15:34:40 +0800
Subject: [PATCH] x86/KASLR: Fix boot crash caused by wrongly chosen kernel
 physical address

System halted with the separate randomization code applied. With debug printing
we got the reason that the chosen kernel physical address randomly is overlapped
with input dada. So input data and its running space must be corrupted during
decompressing kernel, then boot crash happened.

The root cause is that in function mem_avoid_overlap() local variable 'earliest'
is not updated correctly. Function mem_avoid_overlap is used to find the overlap
region with the lowest address, and 'earliest' is used to track the lowest address.
Decompressing kernel could step into those regions which need be avoided if we
didn't handle these overlap region correctly. So fix the code bug now.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/boot/compressed/kaslr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 304c5c3..8e1fdf7 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -285,6 +285,7 @@ static bool mem_avoid_overlap(struct mem_vector *img,
 		if (mem_overlaps(img, &mem_avoid[i]) &&
 		    mem_avoid[i].start < earliest) {
 			*overlap = mem_avoid[i];
+			earliest = overlap->start;
 			is_overlapping = true;
 		}
 	}
@@ -299,6 +300,7 @@ static bool mem_avoid_overlap(struct mem_vector *img,
 
 		if (mem_overlaps(img, &avoid) && (avoid.start < earliest)) {
 			*overlap = avoid;
+			earliest = overlap->start;
 			is_overlapping = true;
 		}
 
-- 
2.5.5

[toc] | [next] | [standalone]


#1440083

FromIngo Molnar <mingo@kernel.org>
Date2016-07-10 13:30 +0200
Message-ID<rTl5T-8m4-5@gated-at.bofh.it>
In reply to#1440055
* Baoquan He <bhe@redhat.com> wrote:

> Hi Ingo,
> 
> I am sorry the previous post didn't contain formal patch log. I made a new one 
> as below. The boot crash could not only happen with certain memory. Because of 
> this code bug the regions which need be avoided like the zipped kernel with its 
> unzipping running code, initrd, kernel command line could be corrupted if 
> mem_avoid_overlap() can't find the overlap region with the lowest address. But 
> it's very lucky that Xiaolong's system which has only 300M memory can always 
> reproduce it. I checked the boog log and found on his system no any other slot 
> can be chosen except for the original one. If we have a system with large memory 
> it may not be easy to hit it, at least with low probability since there are many 
> candidate slots.

So if it's just a better changelog that what I wrote I wouldn't rebase the commit: 
there's other commits on top of the fix meanwhile. Should a rebase become 
necessary at a later point I'll update it with your extended changelog.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]


#1440118

FromBaoquan He <bhe@redhat.com>
Date2016-07-10 17:20 +0200
Message-ID<rToGt-2li-13@gated-at.bofh.it>
In reply to#1440083
On 07/10/16 at 01:24pm, Ingo Molnar wrote:
> 
> * Baoquan He <bhe@redhat.com> wrote:
> 
> > Hi Ingo,
> > 
> > I am sorry the previous post didn't contain formal patch log. I made a new one 
> > as below. The boot crash could not only happen with certain memory. Because of 
> > this code bug the regions which need be avoided like the zipped kernel with its 
> > unzipping running code, initrd, kernel command line could be corrupted if 
> > mem_avoid_overlap() can't find the overlap region with the lowest address. But 
> > it's very lucky that Xiaolong's system which has only 300M memory can always 
> > reproduce it. I checked the boog log and found on his system no any other slot 
> > can be chosen except for the original one. If we have a system with large memory 
> > it may not be easy to hit it, at least with low probability since there are many 
> > candidate slots.
> 
> So if it's just a better changelog that what I wrote I wouldn't rebase the commit: 
> there's other commits on top of the fix meanwhile. Should a rebase become 
> necessary at a later point I'll update it with your extended changelog.

Yes, it's just a changelog update, no need to rebase it now.

Thanks
Baoquan

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web