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

Chao Fan
2017-10-20T03:46:48Z

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(-)
>> > >
>>