Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-17 Thread Michal Hocko
On Tue 17-07-18 09:51:20, Baoquan He wrote:
> On 07/16/18 at 05:24pm, Michal Hocko wrote:
> > On Mon 16-07-18 21:02:02, Baoquan He wrote:
> > > On 07/16/18 at 01:38pm, Michal Hocko wrote:
> > > > On Fri 13-07-18 07:52:40, Baoquan He wrote:
> > > > > Hi Michal,
> > > > > 
> > > > > On 07/12/18 at 02:32pm, Michal Hocko wrote:
> > > > [...]
> > > > > > I am not able to find the beginning of the email thread right now. 
> > > > > > Could
> > > > > > you summarize what is the actual problem please?
> > > > > 
> > > > > The bug is found on x86 now. 
> > > > > 
> > > > > When added "kernelcore=" or "movablecore=" into kernel command line,
> > > > > kernel memory is spread evenly among nodes. However, this is right 
> > > > > when
> > > > > KASLR is not enabled, then kernel will be at 16M of place in x86 arch.
> > > > > If KASLR enabled, it could be put any place from 16M to 64T randomly.
> > > > >  
> > > > > Consider a scenario, we have 10 nodes, and each node has 20G memory, 
> > > > > and
> > > > > we specify "kernelcore=50%", means each node will take 10G for
> > > > > kernelcore, 10G for movable area. But this doesn't take kernel 
> > > > > position
> > > > > into consideration. E.g if kernel is put at 15G of 2nd node, namely
> > > > > node1. Then we think on node1 there's 10G for kernelcore, 10G for
> > > > > movable, in fact there's only 5G available for movable, just after
> > > > > kernel.
> > > > 
> > > > OK, I guess I see that part. But who is going to use movablecore along
> > > > with KASLR enabled? I mean do we really have to support those two
> > > > obscure command line parameters for KASLR?
> > > 
> > > Not very sure whether we have to support both of those to work with
> > > KASLR. Maybe it's time to make clear of it now.
> > 
> > Yes, I would really like to deprecate this. It is an ugly piece of code
> > and it's far from easily maintainable as well.
> > 
> > > For 'kernelcore=mirror', we have solved the conflict to make it work well
> > > with KASLR. For 'movable_node' conflict with KASLR, Chao is posting
> > > patches to fix it. As for 'kernelcore=' and 'movablecore=', 
> > > 
> > > 1) solve the conflict between them with KASLR in
> > >find_zone_movable_pfns_for_nodes();
> > > 2) disable KASLR when 'kernelcore=' | 'movablecore=' is set;
> > > 3) disable 'kernelcore=' | 'movablecore=' when KASLR is enabled;
> > > 4) add note in doc to notice people to not add them at the same time;
> > 
> > I would simply warn that those kernel parameters are not supported
> > anymore. If somebody shows up with a valid usecase we can reconsider.
> 
> OK, got it. The use case I can think of is that people want to check 
> hotplug on system w/o hotplug ACPI info.

Let's see whether there is really somebody like that and complain.
 
> I am fine with warning people they are not supported. Should I post a
> patch to address this, or you will do it? Both is fine to me.

I will happily ack it if you create a patch.

-- 
Michal Hocko
SUSE Labs


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-16 Thread Baoquan He
On 07/16/18 at 05:24pm, Michal Hocko wrote:
> On Mon 16-07-18 21:02:02, Baoquan He wrote:
> > On 07/16/18 at 01:38pm, Michal Hocko wrote:
> > > On Fri 13-07-18 07:52:40, Baoquan He wrote:
> > > > Hi Michal,
> > > > 
> > > > On 07/12/18 at 02:32pm, Michal Hocko wrote:
> > > [...]
> > > > > I am not able to find the beginning of the email thread right now. 
> > > > > Could
> > > > > you summarize what is the actual problem please?
> > > > 
> > > > The bug is found on x86 now. 
> > > > 
> > > > When added "kernelcore=" or "movablecore=" into kernel command line,
> > > > kernel memory is spread evenly among nodes. However, this is right when
> > > > KASLR is not enabled, then kernel will be at 16M of place in x86 arch.
> > > > If KASLR enabled, it could be put any place from 16M to 64T randomly.
> > > >  
> > > > Consider a scenario, we have 10 nodes, and each node has 20G memory, and
> > > > we specify "kernelcore=50%", means each node will take 10G for
> > > > kernelcore, 10G for movable area. But this doesn't take kernel position
> > > > into consideration. E.g if kernel is put at 15G of 2nd node, namely
> > > > node1. Then we think on node1 there's 10G for kernelcore, 10G for
> > > > movable, in fact there's only 5G available for movable, just after
> > > > kernel.
> > > 
> > > OK, I guess I see that part. But who is going to use movablecore along
> > > with KASLR enabled? I mean do we really have to support those two
> > > obscure command line parameters for KASLR?
> > 
> > Not very sure whether we have to support both of those to work with
> > KASLR. Maybe it's time to make clear of it now.
> 
> Yes, I would really like to deprecate this. It is an ugly piece of code
> and it's far from easily maintainable as well.
> 
> > For 'kernelcore=mirror', we have solved the conflict to make it work well
> > with KASLR. For 'movable_node' conflict with KASLR, Chao is posting
> > patches to fix it. As for 'kernelcore=' and 'movablecore=', 
> > 
> > 1) solve the conflict between them with KASLR in
> >find_zone_movable_pfns_for_nodes();
> > 2) disable KASLR when 'kernelcore=' | 'movablecore=' is set;
> > 3) disable 'kernelcore=' | 'movablecore=' when KASLR is enabled;
> > 4) add note in doc to notice people to not add them at the same time;
> 
> I would simply warn that those kernel parameters are not supported
> anymore. If somebody shows up with a valid usecase we can reconsider.

OK, got it. The use case I can think of is that people want to check 
hotplug on system w/o hotplug ACPI info.

I am fine with warning people they are not supported. Should I post a
patch to address this, or you will do it? Both is fine to me.

> 
> > 2) and 3) may need be fixed in arch/x86 code. As long as come to an
> > agreement, any one is fine to me.
> > > 
> > > In fact I would be much more concerned about memory hotplug and
> > > pre-defined movable nodes. Does the current KASLR code work in that
> > > case?
> > 
> > As said above, kernelcore=mirror works well with KASLR now. Making
> > 'movable_node' work with KASLR is in progress.
> 
> OK, thanks for the info.

You are welcome.

Thanks
Baoquan


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-16 Thread Michal Hocko
On Mon 16-07-18 21:02:02, Baoquan He wrote:
> On 07/16/18 at 01:38pm, Michal Hocko wrote:
> > On Fri 13-07-18 07:52:40, Baoquan He wrote:
> > > Hi Michal,
> > > 
> > > On 07/12/18 at 02:32pm, Michal Hocko wrote:
> > [...]
> > > > I am not able to find the beginning of the email thread right now. Could
> > > > you summarize what is the actual problem please?
> > > 
> > > The bug is found on x86 now. 
> > > 
> > > When added "kernelcore=" or "movablecore=" into kernel command line,
> > > kernel memory is spread evenly among nodes. However, this is right when
> > > KASLR is not enabled, then kernel will be at 16M of place in x86 arch.
> > > If KASLR enabled, it could be put any place from 16M to 64T randomly.
> > >  
> > > Consider a scenario, we have 10 nodes, and each node has 20G memory, and
> > > we specify "kernelcore=50%", means each node will take 10G for
> > > kernelcore, 10G for movable area. But this doesn't take kernel position
> > > into consideration. E.g if kernel is put at 15G of 2nd node, namely
> > > node1. Then we think on node1 there's 10G for kernelcore, 10G for
> > > movable, in fact there's only 5G available for movable, just after
> > > kernel.
> > 
> > OK, I guess I see that part. But who is going to use movablecore along
> > with KASLR enabled? I mean do we really have to support those two
> > obscure command line parameters for KASLR?
> 
> Not very sure whether we have to support both of those to work with
> KASLR. Maybe it's time to make clear of it now.

Yes, I would really like to deprecate this. It is an ugly piece of code
and it's far from easily maintainable as well.

> For 'kernelcore=mirror', we have solved the conflict to make it work well
> with KASLR. For 'movable_node' conflict with KASLR, Chao is posting
> patches to fix it. As for 'kernelcore=' and 'movablecore=', 
> 
> 1) solve the conflict between them with KASLR in
>find_zone_movable_pfns_for_nodes();
> 2) disable KASLR when 'kernelcore=' | 'movablecore=' is set;
> 3) disable 'kernelcore=' | 'movablecore=' when KASLR is enabled;
> 4) add note in doc to notice people to not add them at the same time;

I would simply warn that those kernel parameters are not supported
anymore. If somebody shows up with a valid usecase we can reconsider.

> 2) and 3) may need be fixed in arch/x86 code. As long as come to an
> agreement, any one is fine to me.
> > 
> > In fact I would be much more concerned about memory hotplug and
> > pre-defined movable nodes. Does the current KASLR code work in that
> > case?
> 
> As said above, kernelcore=mirror works well with KASLR now. Making
> 'movable_node' work with KASLR is in progress.

OK, thanks for the info.

-- 
Michal Hocko
SUSE Labs


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-16 Thread Baoquan He
On 07/16/18 at 01:38pm, Michal Hocko wrote:
> On Fri 13-07-18 07:52:40, Baoquan He wrote:
> > Hi Michal,
> > 
> > On 07/12/18 at 02:32pm, Michal Hocko wrote:
> [...]
> > > I am not able to find the beginning of the email thread right now. Could
> > > you summarize what is the actual problem please?
> > 
> > The bug is found on x86 now. 
> > 
> > When added "kernelcore=" or "movablecore=" into kernel command line,
> > kernel memory is spread evenly among nodes. However, this is right when
> > KASLR is not enabled, then kernel will be at 16M of place in x86 arch.
> > If KASLR enabled, it could be put any place from 16M to 64T randomly.
> >  
> > Consider a scenario, we have 10 nodes, and each node has 20G memory, and
> > we specify "kernelcore=50%", means each node will take 10G for
> > kernelcore, 10G for movable area. But this doesn't take kernel position
> > into consideration. E.g if kernel is put at 15G of 2nd node, namely
> > node1. Then we think on node1 there's 10G for kernelcore, 10G for
> > movable, in fact there's only 5G available for movable, just after
> > kernel.
> 
> OK, I guess I see that part. But who is going to use movablecore along
> with KASLR enabled? I mean do we really have to support those two
> obscure command line parameters for KASLR?

Not very sure whether we have to support both of those to work with
KASLR. Maybe it's time to make clear of it now.

For 'kernelcore=mirror', we have solved the conflict to make it work well
with KASLR. For 'movable_node' conflict with KASLR, Chao is posting
patches to fix it. As for 'kernelcore=' and 'movablecore=', 

1) solve the conflict between them with KASLR in
   find_zone_movable_pfns_for_nodes();
2) disable KASLR when 'kernelcore=' | 'movablecore=' is set;
3) disable 'kernelcore=' | 'movablecore=' when KASLR is enabled;
4) add note in doc to notice people to not add them at the same time;

2) and 3) may need be fixed in arch/x86 code. As long as come to an
agreement, any one is fine to me.
> 
> In fact I would be much more concerned about memory hotplug and
> pre-defined movable nodes. Does the current KASLR code work in that
> case?

As said above, kernelcore=mirror works well with KASLR now. Making
'movable_node' work with KASLR is in progress.

Thanks
Baoquan


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-16 Thread Michal Hocko
On Fri 13-07-18 07:52:40, Baoquan He wrote:
> Hi Michal,
> 
> On 07/12/18 at 02:32pm, Michal Hocko wrote:
[...]
> > I am not able to find the beginning of the email thread right now. Could
> > you summarize what is the actual problem please?
> 
> The bug is found on x86 now. 
> 
> When added "kernelcore=" or "movablecore=" into kernel command line,
> kernel memory is spread evenly among nodes. However, this is right when
> KASLR is not enabled, then kernel will be at 16M of place in x86 arch.
> If KASLR enabled, it could be put any place from 16M to 64T randomly.
>  
> Consider a scenario, we have 10 nodes, and each node has 20G memory, and
> we specify "kernelcore=50%", means each node will take 10G for
> kernelcore, 10G for movable area. But this doesn't take kernel position
> into consideration. E.g if kernel is put at 15G of 2nd node, namely
> node1. Then we think on node1 there's 10G for kernelcore, 10G for
> movable, in fact there's only 5G available for movable, just after
> kernel.

OK, I guess I see that part. But who is going to use movablecore along
with KASLR enabled? I mean do we really have to support those two
obscure command line parameters for KASLR?

In fact I would be much more concerned about memory hotplug and
pre-defined movable nodes. Does the current KASLR code work in that
case?
-- 
Michal Hocko
SUSE Labs


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-12 Thread Chao Fan
On Fri, Jul 13, 2018 at 07:52:40AM +0800, Baoquan He wrote:
>Hi Michal,
>
>On 07/12/18 at 02:32pm, Michal Hocko wrote:
>> On Thu 12-07-18 14:01:15, Chao Fan wrote:
>> > On Thu, Jul 12, 2018 at 01:49:49PM +0800, Dou Liyang wrote:
>> > >Hi Baoquan,
>> > >
>> > >At 07/11/2018 08:40 PM, Baoquan He wrote:
>> > >> Please try this v3 patch:
>> > >> >>From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
>> > >> From: Baoquan He 
>> > >> Date: Wed, 11 Jul 2018 20:31:51 +0800
>> > >> Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text
>> > >> 
>> > >> In find_zone_movable_pfns_for_nodes(), when try to find the starting
>> > >> PFN movable zone begins in each node, kernel text position is not
>> > >> considered. KASLR may put kernel after which movable zone begins.
>> > >> 
>> > >> Fix it by finding movable zone after kernel text on that node.
>> > >> 
>> > >> Signed-off-by: Baoquan He 
>> > >
>> > >
>> > >You fix this in the _zone_init side_. This may make the 'kernelcore=' or
>> > >'movablecore=' failed if the KASLR puts the kernel back the tail of the
>> > >last node, or more.
>> > 
>> > I think it may not fail.
>> > There is a 'restart' to do another pass.
>> > 
>> > >
>> > >Due to we have fix the mirror memory in KASLR side, and Chao is trying
>> > >to fix the 'movable_node' in KASLR side. Have you had a chance to fix
>> > >this in the KASLR side.
>> > >
>> > 
>> > I think it's better to fix here, but not KASLR side.
>> > Cause much more code will be change if doing it in KASLR side.
>> > Since we didn't parse 'kernelcore' in compressed code, and you can see
>> > the distribution of ZONE_MOVABLE need so much code, so we do not need
>> > to do so much job in KASLR side. But here, several lines will be OK.
>> 
>> I am not able to find the beginning of the email thread right now. Could
>> you summarize what is the actual problem please?
>
>The bug is found on x86 now. 
>
>When added "kernelcore=" or "movablecore=" into kernel command line,
>kernel memory is spread evenly among nodes. However, this is right when
>KASLR is not enabled, then kernel will be at 16M of place in x86 arch.
>If KASLR enabled, it could be put any place from 16M to 64T randomly.
> 
>Consider a scenario, we have 10 nodes, and each node has 20G memory, and
>we specify "kernelcore=50%", means each node will take 10G for
>kernelcore, 10G for movable area. But this doesn't take kernel position
>into consideration. E.g if kernel is put at 15G of 2nd node, namely
>node1. Then we think on node1 there's 10G for kernelcore, 10G for
>movable, in fact there's only 5G available for movable, just after
>kernel.
>
>I made a v4 patch which possibly can fix it.
>
>
>From dbcac3631863aed556dc2c4ff1839772dfd02d18 Mon Sep 17 00:00:00 2001
>From: Baoquan He 
>Date: Fri, 13 Jul 2018 07:49:29 +0800
>Subject: [PATCH v4] mm, page_alloc: find movable zone after kernel text
>
>In find_zone_movable_pfns_for_nodes(), when try to find the starting
>PFN movable zone begins at in each node, kernel text position is not
>considered. KASLR may put kernel after which movable zone begins.
>
>Fix it by finding movable zone after kernel text on that node.
>
>Signed-off-by: Baoquan He 

You can post it as alone PATCH, then I will test it next week.

Thanks,
Chao Fan

>---
> mm/page_alloc.c | 15 +--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
>diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>index 1521100f1e63..5bc1a47dafda 100644
>--- a/mm/page_alloc.c
>+++ b/mm/page_alloc.c
>@@ -6547,7 +6547,7 @@ static unsigned long __init 
>early_calculate_totalpages(void)
> static void __init find_zone_movable_pfns_for_nodes(void)
> {
>   int i, nid;
>-  unsigned long usable_startpfn;
>+  unsigned long usable_startpfn, kernel_endpfn, arch_startpfn;
>   unsigned long kernelcore_node, kernelcore_remaining;
>   /* save the state before borrow the nodemask */
>   nodemask_t saved_node_state = node_states[N_MEMORY];
>@@ -6649,8 +6649,9 @@ static void __init find_zone_movable_pfns_for_nodes(void)
>   if (!required_kernelcore || required_kernelcore >= totalpages)
>   goto out;
> 
>+  kernel_endpfn = PFN_UP(__pa_symbol(_end));
>   /* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */
>-  usable_startpfn = arch_zone_lowest_possible_pfn[movable_zone];
>+  arch_startpfn = arch_zone_lowest_possible_pfn[movable_zone];
> 
> restart:
>   /* Spread kernelcore memory as evenly as possible throughout nodes */
>@@ -6659,6 +6660,16 @@ static void __init 
>find_zone_movable_pfns_for_nodes(void)
>   unsigned long start_pfn, end_pfn;
> 
>   /*
>+   * KASLR may put kernel near tail of node memory,
>+   * start after kernel on that node to find PFN
>+   * at which zone begins.
>+   */
>+  if (pfn_to_nid(kernel_endpfn) == nid)
>+  usable_startpfn = max(arch_startpfn, kernel_endpfn);
>

Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-12 Thread Baoquan He
Hi Michal,

On 07/12/18 at 02:32pm, Michal Hocko wrote:
> On Thu 12-07-18 14:01:15, Chao Fan wrote:
> > On Thu, Jul 12, 2018 at 01:49:49PM +0800, Dou Liyang wrote:
> > >Hi Baoquan,
> > >
> > >At 07/11/2018 08:40 PM, Baoquan He wrote:
> > >> Please try this v3 patch:
> > >> >>From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
> > >> From: Baoquan He 
> > >> Date: Wed, 11 Jul 2018 20:31:51 +0800
> > >> Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text
> > >> 
> > >> In find_zone_movable_pfns_for_nodes(), when try to find the starting
> > >> PFN movable zone begins in each node, kernel text position is not
> > >> considered. KASLR may put kernel after which movable zone begins.
> > >> 
> > >> Fix it by finding movable zone after kernel text on that node.
> > >> 
> > >> Signed-off-by: Baoquan He 
> > >
> > >
> > >You fix this in the _zone_init side_. This may make the 'kernelcore=' or
> > >'movablecore=' failed if the KASLR puts the kernel back the tail of the
> > >last node, or more.
> > 
> > I think it may not fail.
> > There is a 'restart' to do another pass.
> > 
> > >
> > >Due to we have fix the mirror memory in KASLR side, and Chao is trying
> > >to fix the 'movable_node' in KASLR side. Have you had a chance to fix
> > >this in the KASLR side.
> > >
> > 
> > I think it's better to fix here, but not KASLR side.
> > Cause much more code will be change if doing it in KASLR side.
> > Since we didn't parse 'kernelcore' in compressed code, and you can see
> > the distribution of ZONE_MOVABLE need so much code, so we do not need
> > to do so much job in KASLR side. But here, several lines will be OK.
> 
> I am not able to find the beginning of the email thread right now. Could
> you summarize what is the actual problem please?

The bug is found on x86 now. 

When added "kernelcore=" or "movablecore=" into kernel command line,
kernel memory is spread evenly among nodes. However, this is right when
KASLR is not enabled, then kernel will be at 16M of place in x86 arch.
If KASLR enabled, it could be put any place from 16M to 64T randomly.
 
Consider a scenario, we have 10 nodes, and each node has 20G memory, and
we specify "kernelcore=50%", means each node will take 10G for
kernelcore, 10G for movable area. But this doesn't take kernel position
into consideration. E.g if kernel is put at 15G of 2nd node, namely
node1. Then we think on node1 there's 10G for kernelcore, 10G for
movable, in fact there's only 5G available for movable, just after
kernel.

I made a v4 patch which possibly can fix it.


>From dbcac3631863aed556dc2c4ff1839772dfd02d18 Mon Sep 17 00:00:00 2001
From: Baoquan He 
Date: Fri, 13 Jul 2018 07:49:29 +0800
Subject: [PATCH v4] mm, page_alloc: find movable zone after kernel text

In find_zone_movable_pfns_for_nodes(), when try to find the starting
PFN movable zone begins at in each node, kernel text position is not
considered. KASLR may put kernel after which movable zone begins.

Fix it by finding movable zone after kernel text on that node.

Signed-off-by: Baoquan He 
---
 mm/page_alloc.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1521100f1e63..5bc1a47dafda 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6547,7 +6547,7 @@ static unsigned long __init 
early_calculate_totalpages(void)
 static void __init find_zone_movable_pfns_for_nodes(void)
 {
int i, nid;
-   unsigned long usable_startpfn;
+   unsigned long usable_startpfn, kernel_endpfn, arch_startpfn;
unsigned long kernelcore_node, kernelcore_remaining;
/* save the state before borrow the nodemask */
nodemask_t saved_node_state = node_states[N_MEMORY];
@@ -6649,8 +6649,9 @@ static void __init find_zone_movable_pfns_for_nodes(void)
if (!required_kernelcore || required_kernelcore >= totalpages)
goto out;
 
+   kernel_endpfn = PFN_UP(__pa_symbol(_end));
/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */
-   usable_startpfn = arch_zone_lowest_possible_pfn[movable_zone];
+   arch_startpfn = arch_zone_lowest_possible_pfn[movable_zone];
 
 restart:
/* Spread kernelcore memory as evenly as possible throughout nodes */
@@ -6659,6 +6660,16 @@ static void __init find_zone_movable_pfns_for_nodes(void)
unsigned long start_pfn, end_pfn;
 
/*
+* KASLR may put kernel near tail of node memory,
+* start after kernel on that node to find PFN
+* at which zone begins.
+*/
+   if (pfn_to_nid(kernel_endpfn) == nid)
+   usable_startpfn = max(arch_startpfn, kernel_endpfn);
+   else
+   usable_startpfn = arch_startpfn;
+
+   /*
 * Recalculate kernelcore_node if the division per node
 * now exceeds what is necessary to satisfy the req

Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-12 Thread Michal Hocko
On Thu 12-07-18 14:01:15, Chao Fan wrote:
> On Thu, Jul 12, 2018 at 01:49:49PM +0800, Dou Liyang wrote:
> >Hi Baoquan,
> >
> >At 07/11/2018 08:40 PM, Baoquan He wrote:
> >> Please try this v3 patch:
> >> >>From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
> >> From: Baoquan He 
> >> Date: Wed, 11 Jul 2018 20:31:51 +0800
> >> Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text
> >> 
> >> In find_zone_movable_pfns_for_nodes(), when try to find the starting
> >> PFN movable zone begins in each node, kernel text position is not
> >> considered. KASLR may put kernel after which movable zone begins.
> >> 
> >> Fix it by finding movable zone after kernel text on that node.
> >> 
> >> Signed-off-by: Baoquan He 
> >
> >
> >You fix this in the _zone_init side_. This may make the 'kernelcore=' or
> >'movablecore=' failed if the KASLR puts the kernel back the tail of the
> >last node, or more.
> 
> I think it may not fail.
> There is a 'restart' to do another pass.
> 
> >
> >Due to we have fix the mirror memory in KASLR side, and Chao is trying
> >to fix the 'movable_node' in KASLR side. Have you had a chance to fix
> >this in the KASLR side.
> >
> 
> I think it's better to fix here, but not KASLR side.
> Cause much more code will be change if doing it in KASLR side.
> Since we didn't parse 'kernelcore' in compressed code, and you can see
> the distribution of ZONE_MOVABLE need so much code, so we do not need
> to do so much job in KASLR side. But here, several lines will be OK.

I am not able to find the beginning of the email thread right now. Could
you summarize what is the actual problem please?
-- 
Michal Hocko
SUSE Labs


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-12 Thread Baoquan He
On 07/12/18 at 09:19am, Chao Fan wrote:
> On Wed, Jul 11, 2018 at 08:40:08PM +0800, Baoquan He wrote:
> >Please try this v3 patch:
> >
> >From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
> >From: Baoquan He 
> >Date: Wed, 11 Jul 2018 20:31:51 +0800
> >Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text
> >
> >In find_zone_movable_pfns_for_nodes(), when try to find the starting
> >PFN movable zone begins in each node, kernel text position is not
> >considered. KASLR may put kernel after which movable zone begins.
> >
> >Fix it by finding movable zone after kernel text on that node.
> >
> >Signed-off-by: Baoquan He 
> >---
> > mm/page_alloc.c | 20 +++-
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >index 1521100..390eb35 100644
> >--- a/mm/page_alloc.c
> >+++ b/mm/page_alloc.c
> >@@ -6547,7 +6547,7 @@ static unsigned long __init 
> >early_calculate_totalpages(void)
> > static void __init find_zone_movable_pfns_for_nodes(void)
> > {
> > int i, nid;
> >-unsigned long usable_startpfn;
> >+unsigned long usable_startpfn, real_startpfn;
> > unsigned long kernelcore_node, kernelcore_remaining;
> > /* save the state before borrow the nodemask */
> > nodemask_t saved_node_state = node_states[N_MEMORY];
> >@@ -6681,10 +6681,20 @@ static void __init 
> >find_zone_movable_pfns_for_nodes(void)
> > if (start_pfn >= end_pfn)
> > continue;
> 
> Hi Baoquan,
> 
> Thanks for your quick reply and PATCH.
> I think it can work well after reviewing the code. But I think the new
> variable 'real_startpfn' is unnecessary. How about this:
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 6d00f746c2fd..0fc9c4283947 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6492,6 +6492,10 @@ static void __init 
> find_zone_movable_pfns_for_nodes(void)
> if (start_pfn >= end_pfn)
> continue;
> 
> +   if (pfn_to_nid(PFN_UP(_etext)) == i)
> +   usable_startpfn = max(usable_startpfn,
> + PFN_UP(_etext));
> +
> /* Account for what is only usable for kernelcore */
> if (start_pfn < usable_startpfn) {
> unsigned long kernel_pages;
> 
> I think the logic of these two method are the same, and this method
> change less code. If I am wrong, please let me know.

Might be not. Need consider usable_startpfn and kernel_pfn are in the
same node, or in different node, two cases.

I will correct code after fix the compiling error.
> 
> 
> > 
> >+/*
> >+ * KASLR may put kernel near tail of node memory,
> >+ * start after kernel on that node to find PFN
> >+ * which zone begins.
> >+ */
> >+if (pfn_to_nid(PFN_UP(_etext)) == i)
> >+real_startpfn = max(usable_startpfn,
> >+PFN_UP(_etext))
> >+else
> >+real_startpfn = usable_startpfn;
> > /* Account for what is only usable for kernelcore */
> >-if (start_pfn < usable_startpfn) {
> >+if (start_pfn < real_startpfn) {
> > unsigned long kernel_pages;
> >-kernel_pages = min(end_pfn, usable_startpfn)
> >+kernel_pages = min(end_pfn, real_startpfn)
> > - start_pfn;
> > 
> > kernelcore_remaining -= min(kernel_pages,
> >@@ -6693,7 +6703,7 @@ static void __init 
> >find_zone_movable_pfns_for_nodes(void)
> > required_kernelcore);
> > 
> > /* Continue if range is now fully accounted */
> >-if (end_pfn <= usable_startpfn) {
> >+if (end_pfn <= real_startpfn) {
> > 
> > /*
> >  * Push zone_movable_pfn to the end so
> >@@ -6704,7 +6714,7 @@ static void __init 
> >find_zone_movable_pfns_for_nodes(void)
> > zone_movable_pfn[nid] = end_pfn;
> > continue;
> > }
> >-start_pfn = usable_startpfn;
> >+start_pfn = real_startpfn;
> > }
> > 
> > /*
> >-- 
> >2.1.0
> >
> >
> >
> 
> 


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Chao Fan
On Thu, Jul 12, 2018 at 01:49:49PM +0800, Dou Liyang wrote:
>Hi Baoquan,
>
>At 07/11/2018 08:40 PM, Baoquan He wrote:
>> Please try this v3 patch:
>> >>From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
>> From: Baoquan He 
>> Date: Wed, 11 Jul 2018 20:31:51 +0800
>> Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text
>> 
>> In find_zone_movable_pfns_for_nodes(), when try to find the starting
>> PFN movable zone begins in each node, kernel text position is not
>> considered. KASLR may put kernel after which movable zone begins.
>> 
>> Fix it by finding movable zone after kernel text on that node.
>> 
>> Signed-off-by: Baoquan He 
>
>
>You fix this in the _zone_init side_. This may make the 'kernelcore=' or
>'movablecore=' failed if the KASLR puts the kernel back the tail of the
>last node, or more.

I think it may not fail.
There is a 'restart' to do another pass.

>
>Due to we have fix the mirror memory in KASLR side, and Chao is trying
>to fix the 'movable_node' in KASLR side. Have you had a chance to fix
>this in the KASLR side.
>

I think it's better to fix here, but not KASLR side.
Cause much more code will be change if doing it in KASLR side.
Since we didn't parse 'kernelcore' in compressed code, and you can see
the distribution of ZONE_MOVABLE need so much code, so we do not need
to do so much job in KASLR side. But here, several lines will be OK.

Thanks,
Chao Fan

>
>> ---
>>   mm/page_alloc.c | 20 +++-
>>   1 file changed, 15 insertions(+), 5 deletions(-)
>> 
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 1521100..390eb35 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6547,7 +6547,7 @@ static unsigned long __init 
>> early_calculate_totalpages(void)
>>   static void __init find_zone_movable_pfns_for_nodes(void)
>>   {
>>  int i, nid;
>> -unsigned long usable_startpfn;
>> +unsigned long usable_startpfn, real_startpfn;
>>  unsigned long kernelcore_node, kernelcore_remaining;
>>  /* save the state before borrow the nodemask */
>>  nodemask_t saved_node_state = node_states[N_MEMORY];
>> @@ -6681,10 +6681,20 @@ static void __init 
>> find_zone_movable_pfns_for_nodes(void)
>>  if (start_pfn >= end_pfn)
>>  continue;
>> +/*
>> + * KASLR may put kernel near tail of node memory,
>> + * start after kernel on that node to find PFN
>> + * which zone begins.
>> + */
>> +if (pfn_to_nid(PFN_UP(_etext)) == i)
>
>Here, did you want to check the Node id? seems may be nid.
>
>and
>
>for_each_node_state(nid, N_MEMORY) {
>
>... seems check here is more suitable.
>
>   for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {
>
>   }
>}
>
>Thanks,
>   dou
>
>> +real_startpfn = max(usable_startpfn,
>> +PFN_UP(_etext))
>> +else
>> +real_startpfn = usable_startpfn;
>>  /* Account for what is only usable for kernelcore */
>> -if (start_pfn < usable_startpfn) {
>> +if (start_pfn < real_startpfn) {
>>  unsigned long kernel_pages;
>> -kernel_pages = min(end_pfn, usable_startpfn)
>> +kernel_pages = min(end_pfn, real_startpfn)
>>  - start_pfn;
>>  kernelcore_remaining -= min(kernel_pages,
>> @@ -6693,7 +6703,7 @@ static void __init 
>> find_zone_movable_pfns_for_nodes(void)
>>  required_kernelcore);
>>  /* Continue if range is now fully accounted */
>> -if (end_pfn <= usable_startpfn) {
>> +if (end_pfn <= real_startpfn) {
>>  /*
>>   * Push zone_movable_pfn to the end so
>> @@ -6704,7 +6714,7 @@ static void __init 
>> find_zone_movable_pfns_for_nodes(void)
>>  zone_movable_pfn[nid] = end_pfn;
>>  continue;
>>  }
>> -start_pfn = usable_startpfn;
>> +start_pfn = real_startpfn;
>>  }
>>  /*
>> 




Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Dou Liyang

Hi Baoquan,

At 07/11/2018 08:40 PM, Baoquan He wrote:

Please try this v3 patch:
>>From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
From: Baoquan He 
Date: Wed, 11 Jul 2018 20:31:51 +0800
Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text

In find_zone_movable_pfns_for_nodes(), when try to find the starting
PFN movable zone begins in each node, kernel text position is not
considered. KASLR may put kernel after which movable zone begins.

Fix it by finding movable zone after kernel text on that node.

Signed-off-by: Baoquan He 



You fix this in the _zone_init side_. This may make the 'kernelcore=' or
'movablecore=' failed if the KASLR puts the kernel back the tail of the
last node, or more.

Due to we have fix the mirror memory in KASLR side, and Chao is trying
to fix the 'movable_node' in KASLR side. Have you had a chance to fix
this in the KASLR side.



---
  mm/page_alloc.c | 20 +++-
  1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1521100..390eb35 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6547,7 +6547,7 @@ static unsigned long __init 
early_calculate_totalpages(void)
  static void __init find_zone_movable_pfns_for_nodes(void)
  {
int i, nid;
-   unsigned long usable_startpfn;
+   unsigned long usable_startpfn, real_startpfn;
unsigned long kernelcore_node, kernelcore_remaining;
/* save the state before borrow the nodemask */
nodemask_t saved_node_state = node_states[N_MEMORY];
@@ -6681,10 +6681,20 @@ static void __init 
find_zone_movable_pfns_for_nodes(void)
if (start_pfn >= end_pfn)
continue;
  
+			/*

+* KASLR may put kernel near tail of node memory,
+* start after kernel on that node to find PFN
+* which zone begins.
+*/
+   if (pfn_to_nid(PFN_UP(_etext)) == i)


Here, did you want to check the Node id? seems may be nid.

and

for_each_node_state(nid, N_MEMORY) {

... seems check here is more suitable.

for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {

}
}

Thanks,
dou


+   real_startpfn = max(usable_startpfn,
+   PFN_UP(_etext))
+   else
+   real_startpfn = usable_startpfn;
/* Account for what is only usable for kernelcore */
-   if (start_pfn < usable_startpfn) {
+   if (start_pfn < real_startpfn) {
unsigned long kernel_pages;
-   kernel_pages = min(end_pfn, usable_startpfn)
+   kernel_pages = min(end_pfn, real_startpfn)
- start_pfn;
  
  kernelcore_remaining -= min(kernel_pages,

@@ -6693,7 +6703,7 @@ static void __init find_zone_movable_pfns_for_nodes(void)
required_kernelcore);
  
  /* Continue if range is now fully accounted */

-   if (end_pfn <= usable_startpfn) {
+   if (end_pfn <= real_startpfn) {
  
  	/*

 * Push zone_movable_pfn to the end so
@@ -6704,7 +6714,7 @@ static void __init find_zone_movable_pfns_for_nodes(void)
zone_movable_pfn[nid] = end_pfn;
continue;
}
-   start_pfn = usable_startpfn;
+   start_pfn = real_startpfn;
}
  
  			/*







Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Chao Fan
On Wed, Jul 11, 2018 at 08:40:08PM +0800, Baoquan He wrote:
>Please try this v3 patch:
>
>From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
>From: Baoquan He 
>Date: Wed, 11 Jul 2018 20:31:51 +0800
>Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text
>
>In find_zone_movable_pfns_for_nodes(), when try to find the starting
>PFN movable zone begins in each node, kernel text position is not
>considered. KASLR may put kernel after which movable zone begins.
>
>Fix it by finding movable zone after kernel text on that node.
>
>Signed-off-by: Baoquan He 
>---
> mm/page_alloc.c | 20 +++-
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
>diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>index 1521100..390eb35 100644
>--- a/mm/page_alloc.c
>+++ b/mm/page_alloc.c
>@@ -6547,7 +6547,7 @@ static unsigned long __init 
>early_calculate_totalpages(void)
> static void __init find_zone_movable_pfns_for_nodes(void)
> {
>   int i, nid;
>-  unsigned long usable_startpfn;
>+  unsigned long usable_startpfn, real_startpfn;
>   unsigned long kernelcore_node, kernelcore_remaining;
>   /* save the state before borrow the nodemask */
>   nodemask_t saved_node_state = node_states[N_MEMORY];
>@@ -6681,10 +6681,20 @@ static void __init 
>find_zone_movable_pfns_for_nodes(void)
>   if (start_pfn >= end_pfn)
>   continue;

Hi Baoquan,

Thanks for your quick reply and PATCH.
I think it can work well after reviewing the code. But I think the new
variable 'real_startpfn' is unnecessary. How about this:

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 6d00f746c2fd..0fc9c4283947 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6492,6 +6492,10 @@ static void __init find_zone_movable_pfns_for_nodes(void)
if (start_pfn >= end_pfn)
continue;

+   if (pfn_to_nid(PFN_UP(_etext)) == i)
+   usable_startpfn = max(usable_startpfn,
+ PFN_UP(_etext));
+
/* Account for what is only usable for kernelcore */
if (start_pfn < usable_startpfn) {
unsigned long kernel_pages;

I think the logic of these two method are the same, and this method
change less code. If I am wrong, please let me know.

Thanks,
Chao Fan

> 
>+  /*
>+   * KASLR may put kernel near tail of node memory,
>+   * start after kernel on that node to find PFN
>+   * which zone begins.
>+   */
>+  if (pfn_to_nid(PFN_UP(_etext)) == i)
>+  real_startpfn = max(usable_startpfn,
>+  PFN_UP(_etext))
>+  else
>+  real_startpfn = usable_startpfn;
>   /* Account for what is only usable for kernelcore */
>-  if (start_pfn < usable_startpfn) {
>+  if (start_pfn < real_startpfn) {
>   unsigned long kernel_pages;
>-  kernel_pages = min(end_pfn, usable_startpfn)
>+  kernel_pages = min(end_pfn, real_startpfn)
>   - start_pfn;
> 
>   kernelcore_remaining -= min(kernel_pages,
>@@ -6693,7 +6703,7 @@ static void __init find_zone_movable_pfns_for_nodes(void)
>   required_kernelcore);
> 
>   /* Continue if range is now fully accounted */
>-  if (end_pfn <= usable_startpfn) {
>+  if (end_pfn <= real_startpfn) {
> 
>   /*
>* Push zone_movable_pfn to the end so
>@@ -6704,7 +6714,7 @@ static void __init find_zone_movable_pfns_for_nodes(void)
>   zone_movable_pfn[nid] = end_pfn;
>   continue;
>   }
>-  start_pfn = usable_startpfn;
>+  start_pfn = real_startpfn;
>   }
> 
>   /*
>-- 
>2.1.0
>
>
>




Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Baoquan He
On 07/11/18 at 06:49pm, Baoquan He wrote:
> On 07/11/18 at 06:41pm, Baoquan He wrote:
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 1521100..fe346b4 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -6678,6 +6678,8 @@ static void __init 
> > find_zone_movable_pfns_for_nodes(void)
> > unsigned long size_pages;
> >  
> > start_pfn = max(start_pfn, zone_movable_pfn[nid]);
> > +   /* KASLR may put kernel after 'start_pfn', start after 
> > kernel */
> > +   start_pfn = max(start_pfn, PAGE_ALIGN(_etext));
> 
> Sorry, I used wrong function.
> 
> Please try this one:
> 
> From 005435407a331ecf2803e5ebfdc44b8f5f8f9748 Mon Sep 17 00:00:00 2001
> From: Baoquan He 
> Date: Wed, 11 Jul 2018 18:30:04 +0800
> Subject: [PATCH v2] mm, page_alloc: find movable zone after kernel text
> 
> In find_zone_movable_pfns_for_nodes(), when try to find the starting
> PFN movable zone begins in each node, kernel text position is not
> considered. KASLR may put kernel after which movable zone begins.
> 
> Fix it by finding movable zone after kernel text.
> 
> Signed-off-by: Baoquan He 
> ---
>  mm/page_alloc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1521100..17584cc 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6678,6 +6678,8 @@ static void __init 
> find_zone_movable_pfns_for_nodes(void)
>   unsigned long size_pages;
>  
>   start_pfn = max(start_pfn, zone_movable_pfn[nid]);
> + /* KASLR may put kernel after 'start_pfn', start after 
> kernel */
> + start_pfn = max(start_pfn, PFN_UP(_etext));

It's wrong again, NACK, have posted v3 in this thread.

>   if (start_pfn >= end_pfn)
>   continue;
>  
> -- 
> 2.1.0
> 


Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Baoquan He
Please try this v3 patch:

>From 9850d3de9c02e570dc7572069a9749a8add4c4c7 Mon Sep 17 00:00:00 2001
From: Baoquan He 
Date: Wed, 11 Jul 2018 20:31:51 +0800
Subject: [PATCH v3] mm, page_alloc: find movable zone after kernel text

In find_zone_movable_pfns_for_nodes(), when try to find the starting
PFN movable zone begins in each node, kernel text position is not
considered. KASLR may put kernel after which movable zone begins.

Fix it by finding movable zone after kernel text on that node.

Signed-off-by: Baoquan He 
---
 mm/page_alloc.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1521100..390eb35 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6547,7 +6547,7 @@ static unsigned long __init 
early_calculate_totalpages(void)
 static void __init find_zone_movable_pfns_for_nodes(void)
 {
int i, nid;
-   unsigned long usable_startpfn;
+   unsigned long usable_startpfn, real_startpfn;
unsigned long kernelcore_node, kernelcore_remaining;
/* save the state before borrow the nodemask */
nodemask_t saved_node_state = node_states[N_MEMORY];
@@ -6681,10 +6681,20 @@ static void __init 
find_zone_movable_pfns_for_nodes(void)
if (start_pfn >= end_pfn)
continue;
 
+   /*
+* KASLR may put kernel near tail of node memory,
+* start after kernel on that node to find PFN
+* which zone begins.
+*/
+   if (pfn_to_nid(PFN_UP(_etext)) == i)
+   real_startpfn = max(usable_startpfn,
+   PFN_UP(_etext))
+   else
+   real_startpfn = usable_startpfn;
/* Account for what is only usable for kernelcore */
-   if (start_pfn < usable_startpfn) {
+   if (start_pfn < real_startpfn) {
unsigned long kernel_pages;
-   kernel_pages = min(end_pfn, usable_startpfn)
+   kernel_pages = min(end_pfn, real_startpfn)
- start_pfn;
 
kernelcore_remaining -= min(kernel_pages,
@@ -6693,7 +6703,7 @@ static void __init find_zone_movable_pfns_for_nodes(void)
required_kernelcore);
 
/* Continue if range is now fully accounted */
-   if (end_pfn <= usable_startpfn) {
+   if (end_pfn <= real_startpfn) {
 
/*
 * Push zone_movable_pfn to the end so
@@ -6704,7 +6714,7 @@ static void __init find_zone_movable_pfns_for_nodes(void)
zone_movable_pfn[nid] = end_pfn;
continue;
}
-   start_pfn = usable_startpfn;
+   start_pfn = real_startpfn;
}
 
/*
-- 
2.1.0



Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Baoquan He
On 07/11/18 at 06:41pm, Baoquan He wrote:
 
> Hmm, it's an issue, worth fixing it. Otherwise the size of
> movable area will be smaller than we expect when add "kernel_core="
> or "movable_core=".
> 
> Add a check in find_zone_movable_pfns_for_nodes(), and use min() to get
> the starting address of movable area between aligned '_etext'
> and start_pfn. It will go to label 'restart' to calculate the 2nd round
> if not satisfiled. 
> 
> Hi Chao,
> 
> Could you check if below patch works for you?
> 
> 
> From ab6e47c6a78d1a4ccb577b995b7b386f3149732f Mon Sep 17 00:00:00 2001
> From: Baoquan He 
> Date: Wed, 11 Jul 2018 18:30:04 +0800
> Subject: [PATCH] mm, page_alloc: find movable zone after kernel text
> 
> In find_zone_movable_pfns_for_nodes(), when try to find the starting
> PFN movable zone begins in each node, kernel text position is not
> considered. KASLR may put kernel after which movable zone begins.
> 
> Fix it by finding movable zone after kernel text.
> 
> Signed-off-by: Baoquan He 
> ---
>  mm/page_alloc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1521100..fe346b4 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6678,6 +6678,8 @@ static void __init 
> find_zone_movable_pfns_for_nodes(void)
>   unsigned long size_pages;
>  
>   start_pfn = max(start_pfn, zone_movable_pfn[nid]);
> + /* KASLR may put kernel after 'start_pfn', start after 
> kernel */
> + start_pfn = max(start_pfn, PAGE_ALIGN(_etext));

Sorry, I used wrong function.

Please try this one:

>From 005435407a331ecf2803e5ebfdc44b8f5f8f9748 Mon Sep 17 00:00:00 2001
From: Baoquan He 
Date: Wed, 11 Jul 2018 18:30:04 +0800
Subject: [PATCH v2] mm, page_alloc: find movable zone after kernel text

In find_zone_movable_pfns_for_nodes(), when try to find the starting
PFN movable zone begins in each node, kernel text position is not
considered. KASLR may put kernel after which movable zone begins.

Fix it by finding movable zone after kernel text.

Signed-off-by: Baoquan He 
---
 mm/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1521100..17584cc 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6678,6 +6678,8 @@ static void __init find_zone_movable_pfns_for_nodes(void)
unsigned long size_pages;
 
start_pfn = max(start_pfn, zone_movable_pfn[nid]);
+   /* KASLR may put kernel after 'start_pfn', start after 
kernel */
+   start_pfn = max(start_pfn, PFN_UP(_etext));
if (start_pfn >= end_pfn)
continue;
 
-- 
2.1.0



Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Baoquan He
On 07/11/18 at 05:42pm, Chao Fan wrote:
> Hi all,
> 
> I found there is a BUG about KASLR and ZONE_MOVABLE.
> 
> When users use 'kernelcore=' parameter without 'movable_node',
> movable memory is evenly distributed to all nodes. The size of
> ZONE_MOVABLE depends on the kernel parameter 'kernelcore=' and
> 'movablecore='.
> But sometiomes, KASLR may put the uncompressed kernel to the
> tail position of a node, which will cause the kernel memory
> set as ZONE_MOVABLE. This region can not be offlined.
> 
> Here is a very simple test in my qemu-kvm machine, there is
> only one node:
> 
> The command line:
> [root@localhost ~]# cat /proc/cmdline
> BOOT_IMAGE=/vmlinuz-4.18.0-rc3+ root=/dev/mapper/fedora_localhost--live-root
> ro resume=/dev/mapper/fedora_localhost--live-swap
> rd.lvm.lv=fedora_localhost-live/root rd.lvm.lv=fedora_localhost-live/swap
> console=ttyS0 earlyprintk=ttyS0,115200n8 memblock=debug kernelcore=50%
> 
> I use 'kernelcore=50%' here.
> 
> Here is my early print result, I print the random_addr after KASLR chooses
> physical memory:
> early console in extract_kernel
> input_data: 0x0266b3b1
> input_len: 0x007d8802
> output: 0x0100
> output_len: 0x01e15698
> kernel_total_size: 0x01a8b000
> trampoline_32bit: 0x0009d000
> booted via startup_32()
> Physical KASLR using RDRAND RDTSC...
> random_addr: 0x00012f00
> Virtual KASLR using RDRAND RDTSC...
> 
> The address for kernel is 0x00012f00
> 
> Here is the log of ZONE:
> [0.00] Zone ranges:
> [0.00]   DMA  [mem 0x1000-0x00ff]
> [0.00]   DMA32[mem 0x0100-0x]
> [0.00]   Normal   [mem 0x0001-0x0001f57f]
> [0.00]   Device   empty
> [0.00] Movable zone start for each node
> [0.00]   Node 0: 0x00011b00
> [0.00] Early memory node ranges
> [0.00]   node   0: [mem 0x1000-0x0009efff]
> [0.00]   node   0: [mem 0x0010-0xbffd6fff]
> [0.00]   node   0: [mem 0x0001-0x0001f57f]
> [0.00] Initmem setup node 0 [mem
> 0x1000-0x0001f57f]
> 
> Only one node in my machine, ZONE_MOVABLE begins from 0x00011b00,
> which is lower than 0x00012f00.
> So KASLR put the kernel to the ZONE_MOVABLE.
> Try to solve this problem, I think there should be a new tactic in function
> find_zone_movable_pfns_for_nodes() of mm/page_alloc.c. If kernel is 
> uncompressed
> in a tail position, then just set the memory after the kernel as ZONE_MOVABLE,
> at the same time, memory in other nodes will be set as ZONE_MOVABLE.

Hmm, it's an issue, worth fixing it. Otherwise the size of
movable area will be smaller than we expect when add "kernel_core="
or "movable_core=".

Add a check in find_zone_movable_pfns_for_nodes(), and use min() to get
the starting address of movable area between aligned '_etext'
and start_pfn. It will go to label 'restart' to calculate the 2nd round
if not satisfiled. 

Hi Chao,

Could you check if below patch works for you?


>From ab6e47c6a78d1a4ccb577b995b7b386f3149732f Mon Sep 17 00:00:00 2001
From: Baoquan He 
Date: Wed, 11 Jul 2018 18:30:04 +0800
Subject: [PATCH] mm, page_alloc: find movable zone after kernel text

In find_zone_movable_pfns_for_nodes(), when try to find the starting
PFN movable zone begins in each node, kernel text position is not
considered. KASLR may put kernel after which movable zone begins.

Fix it by finding movable zone after kernel text.

Signed-off-by: Baoquan He 
---
 mm/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1521100..fe346b4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6678,6 +6678,8 @@ static void __init find_zone_movable_pfns_for_nodes(void)
unsigned long size_pages;
 
start_pfn = max(start_pfn, zone_movable_pfn[nid]);
+   /* KASLR may put kernel after 'start_pfn', start after 
kernel */
+   start_pfn = max(start_pfn, PAGE_ALIGN(_etext));
if (start_pfn >= end_pfn)
continue;
 
-- 
2.1.0



Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Chao Fan
More explanation:

If there is a machine with 10 nodes, and memory size in each node is
20G. Then 'kernelcore=100G' will set last 10G memory in each node as
ZONE_MOVABLE.
But if KASLR put kernel to 19G position of first node, the regions
can not be offlined. So we should set the last 1G of first kernel
and last 11G as ZONE_MOVABLE of other 9 nodes as ZONE_MOVABLE.

Thanks,
Chao Fan

On Wed, Jul 11, 2018 at 05:42:44PM +0800, Chao Fan wrote:
>Hi all,
>
>I found there is a BUG about KASLR and ZONE_MOVABLE.
>
>When users use 'kernelcore=' parameter without 'movable_node',
>movable memory is evenly distributed to all nodes. The size of
>ZONE_MOVABLE depends on the kernel parameter 'kernelcore=' and
>'movablecore='.
>But sometiomes, KASLR may put the uncompressed kernel to the
>tail position of a node, which will cause the kernel memory
>set as ZONE_MOVABLE. This region can not be offlined.
>
>Here is a very simple test in my qemu-kvm machine, there is
>only one node:
>
>The command line:
>[root@localhost ~]# cat /proc/cmdline
>BOOT_IMAGE=/vmlinuz-4.18.0-rc3+ root=/dev/mapper/fedora_localhost--live-root
>ro resume=/dev/mapper/fedora_localhost--live-swap
>rd.lvm.lv=fedora_localhost-live/root rd.lvm.lv=fedora_localhost-live/swap
>console=ttyS0 earlyprintk=ttyS0,115200n8 memblock=debug kernelcore=50%
>
>I use 'kernelcore=50%' here.
>
>Here is my early print result, I print the random_addr after KASLR chooses
>physical memory:
>early console in extract_kernel
>input_data: 0x0266b3b1
>input_len: 0x007d8802
>output: 0x0100
>output_len: 0x01e15698
>kernel_total_size: 0x01a8b000
>trampoline_32bit: 0x0009d000
>booted via startup_32()
>Physical KASLR using RDRAND RDTSC...
>random_addr: 0x00012f00
>Virtual KASLR using RDRAND RDTSC...
>
>The address for kernel is 0x00012f00
>
>Here is the log of ZONE:
>[0.00] Zone ranges:
>[0.00]   DMA  [mem 0x1000-0x00ff]
>[0.00]   DMA32[mem 0x0100-0x]
>[0.00]   Normal   [mem 0x0001-0x0001f57f]
>[0.00]   Device   empty
>[0.00] Movable zone start for each node
>[0.00]   Node 0: 0x00011b00
>[0.00] Early memory node ranges
>[0.00]   node   0: [mem 0x1000-0x0009efff]
>[0.00]   node   0: [mem 0x0010-0xbffd6fff]
>[0.00]   node   0: [mem 0x0001-0x0001f57f]
>[0.00] Initmem setup node 0 [mem
>0x1000-0x0001f57f]
>
>Only one node in my machine, ZONE_MOVABLE begins from 0x00011b00,
>which is lower than 0x00012f00.
>So KASLR put the kernel to the ZONE_MOVABLE.
>Try to solve this problem, I think there should be a new tactic in function
>find_zone_movable_pfns_for_nodes() of mm/page_alloc.c. If kernel is 
>uncompressed
>in a tail position, then just set the memory after the kernel as ZONE_MOVABLE,
>at the same time, memory in other nodes will be set as ZONE_MOVABLE.
>
>If there is something wrong, pleas let me know. And any comments will be 
>welcome.
>
>Thanks,
>Chao Fan




Re: Bug report about KASLR and ZONE_MOVABLE

2018-07-11 Thread Chao Fan
On Wed, Jul 11, 2018 at 05:42:44PM +0800, Chao Fan wrote:
>Hi all,
>
>I found there is a BUG about KASLR and ZONE_MOVABLE.
>
>When users use 'kernelcore=' parameter without 'movable_node',
>movable memory is evenly distributed to all nodes. The size of
>ZONE_MOVABLE depends on the kernel parameter 'kernelcore=' and
>'movablecore='.
>But sometiomes, KASLR may put the uncompressed kernel to the
>tail position of a node, which will cause the kernel memory
>set as ZONE_MOVABLE. This region can not be offlined.
>
>Here is a very simple test in my qemu-kvm machine, there is
>only one node:
>
>The command line:
>[root@localhost ~]# cat /proc/cmdline
>BOOT_IMAGE=/vmlinuz-4.18.0-rc3+ root=/dev/mapper/fedora_localhost--live-root
>ro resume=/dev/mapper/fedora_localhost--live-swap
>rd.lvm.lv=fedora_localhost-live/root rd.lvm.lv=fedora_localhost-live/swap
>console=ttyS0 earlyprintk=ttyS0,115200n8 memblock=debug kernelcore=50%
>
>I use 'kernelcore=50%' here.
>
>Here is my early print result, I print the random_addr after KASLR chooses
>physical memory:
>early console in extract_kernel
>input_data: 0x0266b3b1
>input_len: 0x007d8802
>output: 0x0100
>output_len: 0x01e15698
>kernel_total_size: 0x01a8b000
>trampoline_32bit: 0x0009d000
>booted via startup_32()
>Physical KASLR using RDRAND RDTSC...
>random_addr: 0x00012f00
>Virtual KASLR using RDRAND RDTSC...
>
>The address for kernel is 0x00012f00
>
>Here is the log of ZONE:
>[0.00] Zone ranges:
>[0.00]   DMA  [mem 0x1000-0x00ff]
>[0.00]   DMA32[mem 0x0100-0x]
>[0.00]   Normal   [mem 0x0001-0x0001f57f]
>[0.00]   Device   empty
>[0.00] Movable zone start for each node
>[0.00]   Node 0: 0x00011b00
>[0.00] Early memory node ranges
>[0.00]   node   0: [mem 0x1000-0x0009efff]
>[0.00]   node   0: [mem 0x0010-0xbffd6fff]
>[0.00]   node   0: [mem 0x0001-0x0001f57f]
>[0.00] Initmem setup node 0 [mem
>0x1000-0x0001f57f]
>
>Only one node in my machine, ZONE_MOVABLE begins from 0x00011b00,
>which is lower than 0x00012f00.
>So KASLR put the kernel to the ZONE_MOVABLE.
>Try to solve this problem, I think there should be a new tactic in function
>find_zone_movable_pfns_for_nodes() of mm/page_alloc.c. If kernel is 
>uncompressed
>in a tail position, then just set the memory after the kernel as ZONE_MOVABLE,
>at the same time, memory in other nodes will be set as ZONE_MOVABLE.

Sorry, mistake here. It should be:
"more memory in other nodes will be set as ZONE_MOVABLE".

Thanks,
Chao Fan

>
>If there is something wrong, pleas let me know. And any comments will be 
>welcome.
>
>Thanks,
>Chao Fan