Re: [PATCH 0/4] kaslr: extend movable_node to movable_node=nn[KMG]@ss[KMG]

[Chao Fan]

On Fri, Oct 20, 2017 at 11:37:11AM +0800, Dou Liyang wrote:
>Hi Chao,
>
>At 10/20/2017 10:53 AM, Chao Fan wrote:
>> On Fri, Oct 20, 2017 at 10:37:52AM +0800, Dou Liyang wrote:
>> > Hi Chao,
>> > 
>> Hi Dou-san,
>> 
>> > Cheer! I have some concerns below.
>> 
>> Thanks for your reply.
>> 
>> > 
>> > At 10/19/2017 06:02 PM, Chao Fan wrote:
>> > > Here is a problem:
>> > > Here is a machine with several NUMA nodes and some of them are hot-pluggable.
>> > > It's not good for kernel to be extracted in the memory region of movable node.
>> > > But in current code, I print the address choosen by kaslr and found it may be
>> > > placed in movable node sometimes. To solve this problem, it's better to limit
>> > > the memory region choosen by kaslr to immovable node in kaslr.c. But the memory
>> > > infomation about if it's hot-pluggable is stored in ACPI SRAT table, which is
>> > > parsed after kernel is extracted. So we can't get the detail memory infomation
>> > > before extracting kernel.
>> > > 
>> > > So extend the movable_node to movable_node=nn@ss, in which nn means
>> > > the size of memory in *immovable* node, and ss means the start position of
>> > > this memory region. Then limit kaslr choose memory in these regions.
>> > 
>> > Yes, great. Here we should remember that the situation of
>> > 'movable_node=nn@ss' is rare, normal situation is 'movable_node=nn'.
>> > 
>> > So, we should consider our code tendencies for normal situation. ;-)
>> 
>> Yes, it's normal. But you can not make sure the special situation will
>> never happen,. If it happens, we can make sure codes work well, right?
>> 
>> We can not make sure that the movable nodes are continuous, or even if
>> the movable nodes are continuous, we can not make sure the memory
>> address are continuous.
>> 
>> It is easy to avoid the memory region in movable node.
>> But if we can handle more special situations, and at the same time,
>> make kernel more safe, why not?
>
>You misunderstand my opinions, I means that
>when we code, we need to know the problem clearly and which part of
>problem will often be executed.
>
>Make our code more suitable for the normal situation without affecting the
>function of the problem.
>Just like:
>
>likely() and unlikely()
Hi Dou-san,

Thanks for that. I think likely() is suitable for another place.

Thanks,
Chao Fan

>
>Here I guess you don't consider that. so I said that.
>
>> 
>> > 
>> > > 
>> > > There are two policies:
>> > > 1. Specify the memory region in *movable* node to avoid:
>> > >    Then we can use the existing mem_avoid to handle. But if the memory
>> > >    one movable node was separated by memory hole or different movable nodes
>> > >    are discontinuous, we don't know how many regions need to avoid.
>> > 
>> > It is not a problem.
>> > 
>> > As you said, we should provide an interface for users later, like that:
>> > 
>> > # cat /sys/device/system/memory/movable_node
>> > nn@ss
>> > 
>> 
>> Both are OK. I think outputing the memory region in movable_node or
>> immovable_node are both reasonable. So the interface of both methods
>> will be useful. And after we decided which policy used in kaslr, then
>> add the interface of /sys.
>> 
>
>Actually, I prefer the first one, are you ready to post the patches
>for the first policy?
>
>Thanks,
>	dou.
>> Thanks,
>> Chao Fan
>> 
>> > 
>> > Thanks,
>> > 	dou.
>> > >    OTOH, we must avoid all of the movable memory, otherwise, kaslr may
>> > >    choose the wrong place.
>> > > 2. Specify the memory region in "immovable* node to select:
>> > >    Only support 4 regions in this parameter. Then user can use two nodes
>> > >    at least for kaslr to choose, it's enough for the kernel to extract.
>> > >    At the same time, because we need only 4 new mem_vector, the usage
>> > >    of memory here is not too big.
>> > > 
>> > > PATCH 1/4 parse the extended movable_node=nn[KMG]@ss[KMG], then
>> > > 	  store the memory regions.
>> > > PATCH 2/4 selects the memory region in immovable node when process
>> > > 	  memmap.
>> > > PATCH 3/4 is the change of document.
>> > > PATCH 4/4 cleans up some little problems.
>> > > 
>> > > Chao Fan (4):
>> > >   kaslr: parse the extended movable_node=nn[KMG]@ss[KMG]
>> > >   kaslr: select the memory region in immovable node to process
>> > >   document: change the document for the extended movable_node
>> > >   kaslr: clean up a useless variable and some usless space
>> > > 
>> > >  Documentation/admin-guide/kernel-parameters.txt |   9 ++
>> > >  arch/x86/boot/compressed/kaslr.c                | 140 +++++++++++++++++++++---
>> > >  2 files changed, 131 insertions(+), 18 deletions(-)
>> > > 
>>