Re: Bug report about KASLR and ZONE_MOVABLE

Re: Bug report about KASLR and ZONE_MOVABLE

[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: 0x000000000266b3b1
> input_len: 0x00000000007d8802
> output: 0x0000000001000000
> output_len: 0x0000000001e15698
> kernel_total_size: 0x0000000001a8b000
> trampoline_32bit: 0x000000000009d000
> booted via startup_32()
> Physical KASLR using RDRAND RDTSC...
> random_addr: 0x000000012f000000
> Virtual KASLR using RDRAND RDTSC...
> 
> The address for kernel is 0x000000012f000000
> 
> Here is the log of ZONE:
> [    0.000000] Zone ranges:
> [    0.000000]   DMA      [mem 0x0000000000001000-0x0000000000ffffff]
> [    0.000000]   DMA32    [mem 0x0000000001000000-0x00000000ffffffff]
> [    0.000000]   Normal   [mem 0x0000000100000000-0x00000001f57fffff]
> [    0.000000]   Device   empty
> [    0.000000] Movable zone start for each node
> [    0.000000]   Node 0: 0x000000011b000000
> [    0.000000] Early memory node ranges
> [    0.000000]   node   0: [mem 0x0000000000001000-0x000000000009efff]
> [    0.000000]   node   0: [mem 0x0000000000100000-0x00000000bffd6fff]
> [    0.000000]   node   0: [mem 0x0000000100000000-0x00000001f57fffff]
> [    0.000000] Initmem setup node 0 [mem
> 0x0000000000001000-0x00000001f57fffff]
> 
> Only one node in my machine, ZONE_MOVABLE begins from 0x000000011b000000,
> which is lower than 0x000000012f000000.
> 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?

Re: Bug report about KASLR and ZONE_MOVABLE

[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 <bhe@redhat.com>
> 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 <bhe@redhat.com>
> ---
>  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:

Re: Bug report about KASLR and ZONE_MOVABLE

[Baoquan He]

Please try this v3 patch:

Re: [PATCH v3] mm, page_alloc: find movable zone after kernel text

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 14720 bytes --]

Hi Baoquan,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.18-rc4 next-20180711]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Baoquan-He/mm-page_alloc-find-movable-zone-after-kernel-text/20180711-234359
config: x86_64-randconfig-x015-201827 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:18:0,
                    from arch/x86/include/asm/bug.h:83,
                    from include/linux/bug.h:5,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:9,
                    from mm/page_alloc.c:18:
   mm/page_alloc.c: In function 'find_zone_movable_pfns_for_nodes':
>> include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:812:40: note: in definition of macro '__typecheck'
      (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                                           ^
   include/linux/kernel.h:836:24: note: in expansion of macro '__safe_cmp'
     __builtin_choose_expr(__safe_cmp(x, y), \
                           ^~~~~~~~~~
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
>> mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
>> mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
>> include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:820:48: note: in definition of macro '__is_constexpr'
     (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
                                                   ^
   include/linux/kernel.h:826:25: note: in expansion of macro '__no_side_effects'
      (__typecheck(x, y) && __no_side_effects(x, y))
                            ^~~~~~~~~~~~~~~~~
   include/linux/kernel.h:836:24: note: in expansion of macro '__safe_cmp'
     __builtin_choose_expr(__safe_cmp(x, y), \
                           ^~~~~~~~~~
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
>> mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
>> mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
>> include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:828:34: note: in definition of macro '__cmp'
    #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                                     ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
>> mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
>> mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
>> include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:828:46: note: in definition of macro '__cmp'
    #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                                                 ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
>> mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
>> mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
>> include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:832:10: note: in definition of macro '__cmp_once'
      typeof(y) unique_y = (y);  \
             ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
>> mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
>> mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
>> include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:832:25: note: in definition of macro '__cmp_once'
      typeof(y) unique_y = (y);  \
                            ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
>> mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
>> mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
>> include/linux/kernel.h:836:2: error: first argument to '__builtin_choose_expr' not a constant
     __builtin_choose_expr(__safe_cmp(x, y), \
     ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
>> mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~

vim +6692 mm/page_alloc.c

  6540	
  6541	/*
  6542	 * Find the PFN the Movable zone begins in each node. Kernel memory
  6543	 * is spread evenly between nodes as long as the nodes have enough
  6544	 * memory. When they don't, some nodes will have more kernelcore than
  6545	 * others
  6546	 */
  6547	static void __init find_zone_movable_pfns_for_nodes(void)
  6548	{
  6549		int i, nid;
  6550		unsigned long usable_startpfn, real_startpfn;
  6551		unsigned long kernelcore_node, kernelcore_remaining;
  6552		/* save the state before borrow the nodemask */
  6553		nodemask_t saved_node_state = node_states[N_MEMORY];
  6554		unsigned long totalpages = early_calculate_totalpages();
  6555		int usable_nodes = nodes_weight(node_states[N_MEMORY]);
  6556		struct memblock_region *r;
  6557	
  6558		/* Need to find movable_zone earlier when movable_node is specified. */
  6559		find_usable_zone_for_movable();
  6560	
  6561		/*
  6562		 * If movable_node is specified, ignore kernelcore and movablecore
  6563		 * options.
  6564		 */
  6565		if (movable_node_is_enabled()) {
  6566			for_each_memblock(memory, r) {
  6567				if (!memblock_is_hotpluggable(r))
  6568					continue;
  6569	
  6570				nid = r->nid;
  6571	
  6572				usable_startpfn = PFN_DOWN(r->base);
  6573				zone_movable_pfn[nid] = zone_movable_pfn[nid] ?
  6574					min(usable_startpfn, zone_movable_pfn[nid]) :
  6575					usable_startpfn;
  6576			}
  6577	
  6578			goto out2;
  6579		}
  6580	
  6581		/*
  6582		 * If kernelcore=mirror is specified, ignore movablecore option
  6583		 */
  6584		if (mirrored_kernelcore) {
  6585			bool mem_below_4gb_not_mirrored = false;
  6586	
  6587			for_each_memblock(memory, r) {
  6588				if (memblock_is_mirror(r))
  6589					continue;
  6590	
  6591				nid = r->nid;
  6592	
  6593				usable_startpfn = memblock_region_memory_base_pfn(r);
  6594	
  6595				if (usable_startpfn < 0x100000) {
  6596					mem_below_4gb_not_mirrored = true;
  6597					continue;
  6598				}
  6599	
  6600				zone_movable_pfn[nid] = zone_movable_pfn[nid] ?
  6601					min(usable_startpfn, zone_movable_pfn[nid]) :
  6602					usable_startpfn;
  6603			}
  6604	
  6605			if (mem_below_4gb_not_mirrored)
  6606				pr_warn("This configuration results in unmirrored kernel memory.");
  6607	
  6608			goto out2;
  6609		}
  6610	
  6611		/*
  6612		 * If kernelcore=nn% or movablecore=nn% was specified, calculate the
  6613		 * amount of necessary memory.
  6614		 */
  6615		if (required_kernelcore_percent)
  6616			required_kernelcore = (totalpages * 100 * required_kernelcore_percent) /
  6617					       10000UL;
  6618		if (required_movablecore_percent)
  6619			required_movablecore = (totalpages * 100 * required_movablecore_percent) /
  6620						10000UL;
  6621	
  6622		/*
  6623		 * If movablecore= was specified, calculate what size of
  6624		 * kernelcore that corresponds so that memory usable for
  6625		 * any allocation type is evenly spread. If both kernelcore
  6626		 * and movablecore are specified, then the value of kernelcore
  6627		 * will be used for required_kernelcore if it's greater than
  6628		 * what movablecore would have allowed.
  6629		 */
  6630		if (required_movablecore) {
  6631			unsigned long corepages;
  6632	
  6633			/*
  6634			 * Round-up so that ZONE_MOVABLE is at least as large as what
  6635			 * was requested by the user
  6636			 */
  6637			required_movablecore =
  6638				roundup(required_movablecore, MAX_ORDER_NR_PAGES);
  6639			required_movablecore = min(totalpages, required_movablecore);
  6640			corepages = totalpages - required_movablecore;
  6641	
  6642			required_kernelcore = max(required_kernelcore, corepages);
  6643		}
  6644	
  6645		/*
  6646		 * If kernelcore was not specified or kernelcore size is larger
  6647		 * than totalpages, there is no ZONE_MOVABLE.
  6648		 */
  6649		if (!required_kernelcore || required_kernelcore >= totalpages)
  6650			goto out;
  6651	
  6652		/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */
  6653		usable_startpfn = arch_zone_lowest_possible_pfn[movable_zone];
  6654	
  6655	restart:
  6656		/* Spread kernelcore memory as evenly as possible throughout nodes */
  6657		kernelcore_node = required_kernelcore / usable_nodes;
  6658		for_each_node_state(nid, N_MEMORY) {
  6659			unsigned long start_pfn, end_pfn;
  6660	
  6661			/*
  6662			 * Recalculate kernelcore_node if the division per node
  6663			 * now exceeds what is necessary to satisfy the requested
  6664			 * amount of memory for the kernel
  6665			 */
  6666			if (required_kernelcore < kernelcore_node)
  6667				kernelcore_node = required_kernelcore / usable_nodes;
  6668	
  6669			/*
  6670			 * As the map is walked, we track how much memory is usable
  6671			 * by the kernel using kernelcore_remaining. When it is
  6672			 * 0, the rest of the node is usable by ZONE_MOVABLE
  6673			 */
  6674			kernelcore_remaining = kernelcore_node;
  6675	
  6676			/* Go through each range of PFNs within this node */
  6677			for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {
  6678				unsigned long size_pages;
  6679	
  6680				start_pfn = max(start_pfn, zone_movable_pfn[nid]);
  6681				if (start_pfn >= end_pfn)
  6682					continue;
  6683	
  6684				/*
  6685				 * KASLR may put kernel near tail of node memory,
  6686				 * start after kernel on that node to find PFN
  6687				 * which zone begins.
  6688				 */
  6689				if (pfn_to_nid(PFN_UP(_etext)) == i)
> 6690					real_startpfn = max(usable_startpfn,
> 6691							PFN_UP(_etext))
> 6692				else
  6693					real_startpfn = usable_startpfn;
  6694				/* Account for what is only usable for kernelcore */
  6695				if (start_pfn < real_startpfn) {
  6696					unsigned long kernel_pages;
  6697					kernel_pages = min(end_pfn, real_startpfn)
  6698									- start_pfn;
  6699	
  6700					kernelcore_remaining -= min(kernel_pages,
  6701								kernelcore_remaining);
  6702					required_kernelcore -= min(kernel_pages,
  6703								required_kernelcore);
  6704	
  6705					/* Continue if range is now fully accounted */
  6706					if (end_pfn <= real_startpfn) {
  6707	
  6708						/*
  6709						 * Push zone_movable_pfn to the end so
  6710						 * that if we have to rebalance
  6711						 * kernelcore across nodes, we will
  6712						 * not double account here
  6713						 */
  6714						zone_movable_pfn[nid] = end_pfn;
  6715						continue;
  6716					}
  6717					start_pfn = real_startpfn;
  6718				}
  6719	
  6720				/*
  6721				 * The usable PFN range for ZONE_MOVABLE is from
  6722				 * start_pfn->end_pfn. Calculate size_pages as the
  6723				 * number of pages used as kernelcore
  6724				 */
  6725				size_pages = end_pfn - start_pfn;
  6726				if (size_pages > kernelcore_remaining)
  6727					size_pages = kernelcore_remaining;
  6728				zone_movable_pfn[nid] = start_pfn + size_pages;
  6729	
  6730				/*
  6731				 * Some kernelcore has been met, update counts and
  6732				 * break if the kernelcore for this node has been
  6733				 * satisfied
  6734				 */
  6735				required_kernelcore -= min(required_kernelcore,
  6736									size_pages);
  6737				kernelcore_remaining -= size_pages;
  6738				if (!kernelcore_remaining)
  6739					break;
  6740			}
  6741		}
  6742	
  6743		/*
  6744		 * If there is still required_kernelcore, we do another pass with one
  6745		 * less node in the count. This will push zone_movable_pfn[nid] further
  6746		 * along on the nodes that still have memory until kernelcore is
  6747		 * satisfied
  6748		 */
  6749		usable_nodes--;
  6750		if (usable_nodes && required_kernelcore > usable_nodes)
  6751			goto restart;
  6752	
  6753	out2:
  6754		/* Align start of ZONE_MOVABLE on all nids to MAX_ORDER_NR_PAGES */
  6755		for (nid = 0; nid < MAX_NUMNODES; nid++)
  6756			zone_movable_pfn[nid] =
  6757				roundup(zone_movable_pfn[nid], MAX_ORDER_NR_PAGES);
  6758	
  6759	out:
  6760		/* restore the node_state */
  6761		node_states[N_MEMORY] = saved_node_state;
  6762	}
  6763	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26781 bytes --]

Re: [PATCH v3] mm, page_alloc: find movable zone after kernel text

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 16398 bytes --]

Hi Baoquan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.18-rc4 next-20180711]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Baoquan-He/mm-page_alloc-find-movable-zone-after-kernel-text/20180711-234359
config: x86_64-randconfig-x007-201827 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:5:0,
                    from arch/x86/include/asm/bug.h:83,
                    from include/linux/bug.h:5,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:9,
                    from mm/page_alloc.c:18:
   mm/page_alloc.c: In function 'find_zone_movable_pfns_for_nodes':
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
   mm/page_alloc.c:6689:4: note: in expansion of macro 'if'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
       ^~
>> mm/page_alloc.c:6689:8: note: in expansion of macro 'pfn_to_nid'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
           ^~~~~~~~~~
   mm/page_alloc.c:6689:19: note: in expansion of macro 'PFN_UP'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
                      ^~~~~~
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
   mm/page_alloc.c:6689:4: note: in expansion of macro 'if'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
       ^~
>> mm/page_alloc.c:6689:8: note: in expansion of macro 'pfn_to_nid'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
           ^~~~~~~~~~
   mm/page_alloc.c:6689:19: note: in expansion of macro 'PFN_UP'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
                      ^~~~~~
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
   mm/page_alloc.c:6689:4: note: in expansion of macro 'if'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
       ^~
>> mm/page_alloc.c:6689:8: note: in expansion of macro 'pfn_to_nid'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
           ^~~~~~~~~~
   mm/page_alloc.c:6689:19: note: in expansion of macro 'PFN_UP'
       if (pfn_to_nid(PFN_UP(_etext)) == i)
                      ^~~~~~
   In file included from include/asm-generic/bug.h:18:0,
                    from arch/x86/include/asm/bug.h:83,
                    from include/linux/bug.h:5,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:9,
                    from mm/page_alloc.c:18:
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:812:40: note: in definition of macro '__typecheck'
      (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                                           ^
   include/linux/kernel.h:836:24: note: in expansion of macro '__safe_cmp'
     __builtin_choose_expr(__safe_cmp(x, y), \
                           ^~~~~~~~~~
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
   mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
   mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:820:48: note: in definition of macro '__is_constexpr'
     (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
                                                   ^
   include/linux/kernel.h:826:25: note: in expansion of macro '__no_side_effects'
      (__typecheck(x, y) && __no_side_effects(x, y))
                            ^~~~~~~~~~~~~~~~~
   include/linux/kernel.h:836:24: note: in expansion of macro '__safe_cmp'
     __builtin_choose_expr(__safe_cmp(x, y), \
                           ^~~~~~~~~~
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
   mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
   mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:828:34: note: in definition of macro '__cmp'
    #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                                     ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
   mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
   mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:828:46: note: in definition of macro '__cmp'
    #define __cmp(x, y, op) ((x) op (y) ? (x) : (y))
                                                 ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
   mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
   mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:832:10: note: in definition of macro '__cmp_once'
      typeof(y) unique_y = (y);  \
             ^
   include/linux/kernel.h:852:19: note: in expansion of macro '__careful_cmp'
    #define max(x, y) __careful_cmp(x, y, >)
                      ^~~~~~~~~~~~~
   mm/page_alloc.c:6690:21: note: in expansion of macro 'max'
        real_startpfn = max(usable_startpfn,
                        ^~~
   mm/page_alloc.c:6691:7: note: in expansion of macro 'PFN_UP'
          PFN_UP(_etext))
          ^~~~~~
   include/linux/pfn.h:19:40: error: invalid operands to binary >> (have 'char *' and 'int')
    #define PFN_UP(x) (((x) + PAGE_SIZE-1) >> PAGE_SHIFT)
                       ~~~~~~~~~~~~~~~~~~~ ^
   include/linux/kernel.h:832:25: note: in definition of macro '__cmp_once'
      typeof(y) unique_y = (y);  \

vim +/pfn_to_nid +6689 mm/page_alloc.c

  6540	
  6541	/*
  6542	 * Find the PFN the Movable zone begins in each node. Kernel memory
  6543	 * is spread evenly between nodes as long as the nodes have enough
  6544	 * memory. When they don't, some nodes will have more kernelcore than
  6545	 * others
  6546	 */
  6547	static void __init find_zone_movable_pfns_for_nodes(void)
  6548	{
  6549		int i, nid;
  6550		unsigned long usable_startpfn, real_startpfn;
  6551		unsigned long kernelcore_node, kernelcore_remaining;
  6552		/* save the state before borrow the nodemask */
  6553		nodemask_t saved_node_state = node_states[N_MEMORY];
  6554		unsigned long totalpages = early_calculate_totalpages();
  6555		int usable_nodes = nodes_weight(node_states[N_MEMORY]);
  6556		struct memblock_region *r;
  6557	
  6558		/* Need to find movable_zone earlier when movable_node is specified. */
  6559		find_usable_zone_for_movable();
  6560	
  6561		/*
  6562		 * If movable_node is specified, ignore kernelcore and movablecore
  6563		 * options.
  6564		 */
  6565		if (movable_node_is_enabled()) {
  6566			for_each_memblock(memory, r) {
  6567				if (!memblock_is_hotpluggable(r))
  6568					continue;
  6569	
  6570				nid = r->nid;
  6571	
  6572				usable_startpfn = PFN_DOWN(r->base);
  6573				zone_movable_pfn[nid] = zone_movable_pfn[nid] ?
  6574					min(usable_startpfn, zone_movable_pfn[nid]) :
  6575					usable_startpfn;
  6576			}
  6577	
  6578			goto out2;
  6579		}
  6580	
  6581		/*
  6582		 * If kernelcore=mirror is specified, ignore movablecore option
  6583		 */
  6584		if (mirrored_kernelcore) {
  6585			bool mem_below_4gb_not_mirrored = false;
  6586	
  6587			for_each_memblock(memory, r) {
  6588				if (memblock_is_mirror(r))
  6589					continue;
  6590	
  6591				nid = r->nid;
  6592	
  6593				usable_startpfn = memblock_region_memory_base_pfn(r);
  6594	
  6595				if (usable_startpfn < 0x100000) {
  6596					mem_below_4gb_not_mirrored = true;
  6597					continue;
  6598				}
  6599	
  6600				zone_movable_pfn[nid] = zone_movable_pfn[nid] ?
  6601					min(usable_startpfn, zone_movable_pfn[nid]) :
  6602					usable_startpfn;
  6603			}
  6604	
  6605			if (mem_below_4gb_not_mirrored)
  6606				pr_warn("This configuration results in unmirrored kernel memory.");
  6607	
  6608			goto out2;
  6609		}
  6610	
  6611		/*
  6612		 * If kernelcore=nn% or movablecore=nn% was specified, calculate the
  6613		 * amount of necessary memory.
  6614		 */
  6615		if (required_kernelcore_percent)
  6616			required_kernelcore = (totalpages * 100 * required_kernelcore_percent) /
  6617					       10000UL;
  6618		if (required_movablecore_percent)
  6619			required_movablecore = (totalpages * 100 * required_movablecore_percent) /
  6620						10000UL;
  6621	
  6622		/*
  6623		 * If movablecore= was specified, calculate what size of
  6624		 * kernelcore that corresponds so that memory usable for
  6625		 * any allocation type is evenly spread. If both kernelcore
  6626		 * and movablecore are specified, then the value of kernelcore
  6627		 * will be used for required_kernelcore if it's greater than
  6628		 * what movablecore would have allowed.
  6629		 */
  6630		if (required_movablecore) {
  6631			unsigned long corepages;
  6632	
  6633			/*
  6634			 * Round-up so that ZONE_MOVABLE is at least as large as what
  6635			 * was requested by the user
  6636			 */
  6637			required_movablecore =
  6638				roundup(required_movablecore, MAX_ORDER_NR_PAGES);
  6639			required_movablecore = min(totalpages, required_movablecore);
  6640			corepages = totalpages - required_movablecore;
  6641	
  6642			required_kernelcore = max(required_kernelcore, corepages);
  6643		}
  6644	
  6645		/*
  6646		 * If kernelcore was not specified or kernelcore size is larger
  6647		 * than totalpages, there is no ZONE_MOVABLE.
  6648		 */
  6649		if (!required_kernelcore || required_kernelcore >= totalpages)
  6650			goto out;
  6651	
  6652		/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */
  6653		usable_startpfn = arch_zone_lowest_possible_pfn[movable_zone];
  6654	
  6655	restart:
  6656		/* Spread kernelcore memory as evenly as possible throughout nodes */
  6657		kernelcore_node = required_kernelcore / usable_nodes;
  6658		for_each_node_state(nid, N_MEMORY) {
  6659			unsigned long start_pfn, end_pfn;
  6660	
  6661			/*
  6662			 * Recalculate kernelcore_node if the division per node
  6663			 * now exceeds what is necessary to satisfy the requested
  6664			 * amount of memory for the kernel
  6665			 */
  6666			if (required_kernelcore < kernelcore_node)
  6667				kernelcore_node = required_kernelcore / usable_nodes;
  6668	
  6669			/*
  6670			 * As the map is walked, we track how much memory is usable
  6671			 * by the kernel using kernelcore_remaining. When it is
  6672			 * 0, the rest of the node is usable by ZONE_MOVABLE
  6673			 */
  6674			kernelcore_remaining = kernelcore_node;
  6675	
  6676			/* Go through each range of PFNs within this node */
  6677			for_each_mem_pfn_range(i, nid, &start_pfn, &end_pfn, NULL) {
  6678				unsigned long size_pages;
  6679	
  6680				start_pfn = max(start_pfn, zone_movable_pfn[nid]);
  6681				if (start_pfn >= end_pfn)
  6682					continue;
  6683	
  6684				/*
  6685				 * KASLR may put kernel near tail of node memory,
  6686				 * start after kernel on that node to find PFN
  6687				 * which zone begins.
  6688				 */
> 6689				if (pfn_to_nid(PFN_UP(_etext)) == i)
  6690					real_startpfn = max(usable_startpfn,
  6691							PFN_UP(_etext))
  6692				else
  6693					real_startpfn = usable_startpfn;
  6694				/* Account for what is only usable for kernelcore */
  6695				if (start_pfn < real_startpfn) {
  6696					unsigned long kernel_pages;
  6697					kernel_pages = min(end_pfn, real_startpfn)
  6698									- start_pfn;
  6699	
  6700					kernelcore_remaining -= min(kernel_pages,
  6701								kernelcore_remaining);
  6702					required_kernelcore -= min(kernel_pages,
  6703								required_kernelcore);
  6704	
  6705					/* Continue if range is now fully accounted */
  6706					if (end_pfn <= real_startpfn) {
  6707	
  6708						/*
  6709						 * Push zone_movable_pfn to the end so
  6710						 * that if we have to rebalance
  6711						 * kernelcore across nodes, we will
  6712						 * not double account here
  6713						 */
  6714						zone_movable_pfn[nid] = end_pfn;
  6715						continue;
  6716					}
  6717					start_pfn = real_startpfn;
  6718				}
  6719	
  6720				/*
  6721				 * The usable PFN range for ZONE_MOVABLE is from
  6722				 * start_pfn->end_pfn. Calculate size_pages as the
  6723				 * number of pages used as kernelcore
  6724				 */
  6725				size_pages = end_pfn - start_pfn;
  6726				if (size_pages > kernelcore_remaining)
  6727					size_pages = kernelcore_remaining;
  6728				zone_movable_pfn[nid] = start_pfn + size_pages;
  6729	
  6730				/*
  6731				 * Some kernelcore has been met, update counts and
  6732				 * break if the kernelcore for this node has been
  6733				 * satisfied
  6734				 */
  6735				required_kernelcore -= min(required_kernelcore,
  6736									size_pages);
  6737				kernelcore_remaining -= size_pages;
  6738				if (!kernelcore_remaining)
  6739					break;
  6740			}
  6741		}
  6742	
  6743		/*
  6744		 * If there is still required_kernelcore, we do another pass with one
  6745		 * less node in the count. This will push zone_movable_pfn[nid] further
  6746		 * along on the nodes that still have memory until kernelcore is
  6747		 * satisfied
  6748		 */
  6749		usable_nodes--;
  6750		if (usable_nodes && required_kernelcore > usable_nodes)
  6751			goto restart;
  6752	
  6753	out2:
  6754		/* Align start of ZONE_MOVABLE on all nids to MAX_ORDER_NR_PAGES */
  6755		for (nid = 0; nid < MAX_NUMNODES; nid++)
  6756			zone_movable_pfn[nid] =
  6757				roundup(zone_movable_pfn[nid], MAX_ORDER_NR_PAGES);
  6758	
  6759	out:
  6760		/* restore the node_state */
  6761		node_states[N_MEMORY] = saved_node_state;
  6762	}
  6763	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26195 bytes --]

Re: Bug report about KASLR and ZONE_MOVABLE

[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 <bhe@redhat.com>
>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 <bhe@redhat.com>
>---
> 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

[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 <bhe@redhat.com>
> >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 <bhe@redhat.com>
> >---
> > 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

[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 <bhe@redhat.com>
> 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 <bhe@redhat.com>


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

[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 <bhe@redhat.com>
>> 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 <bhe@redhat.com>
>
>
>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

[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 <bhe@redhat.com>
> >> 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 <bhe@redhat.com>
> >
> >
> >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

[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 <bhe@redhat.com>
> > >> 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 <bhe@redhat.com>
> > >
> > >
> > >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.

Re: Bug report about KASLR and ZONE_MOVABLE

[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 <bhe@redhat.com>
>> > >> 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 <bhe@redhat.com>
>> > >
>> > >
>> > >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 <bhe@redhat.com>
>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 <bhe@redhat.com>

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);
>+		else
>+		        usable_startpfn = arch_startpfn;
>+
>+		/*
> 		 * Recalculate kernelcore_node if the division per node
> 		 * now exceeds what is necessary to satisfy the requested
> 		 * amount of memory for the kernel
>-- 
>2.13.6
>
>
>

Re: Bug report about KASLR and ZONE_MOVABLE

[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

[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

[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

[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

[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

[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 <bhe@redhat.com>
> 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 <bhe@redhat.com>
> ---
>  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
>