[PATCH] x86: Add warning when memmap=nn!ss and CONFIG_RANDOMIZE_BASE enabled

[PATCH] x86: Add warning when memmap=nn!ss and CONFIG_RANDOMIZE_BASE enabled

[Dave Jiang]

CONFIG_RANDOMIZE_BASE can place the kernel anywhere. This causes a problem
for when memmap=nn!ss is used. This information is not known until after
the kernel starts executing and the decision for where the randomized
base goes happens before the kernel is uncompressed. memmap=nn!ss is not
reliable in the presence of CONFIG_RANDOMIZE_BASE.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/kernel-parameters.txt |    5 ++++-
 arch/x86/kernel/e820.c              |    2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 37babf9..4bf32ab 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2290,7 +2290,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			[KNL,X86] Mark specific memory as protected.
 			Region of memory to be used, from ss to ss+nn.
 			The memory region may be marked as e820 type 12 (0xc)
-			and is NVDIMM or ADR memory.
+			and is NVDIMM or ADR memory. If CONFIG_RANDOMIZE_BASE
+			is enabled the kernel image may collide overwrite the
+			pmem range on subsequent boots. memmap=nn!ss is not
+			reliable in the presence CONFIG_RANDOMIZE_BASE.
 
 	memory_corruption_check=0/1 [X86]
 			Some BIOSes seem to corrupt the first 64k of
diff --git a/arch/x86/kernel/e820.c b/arch/x86/kernel/e820.c
index b85fe5f..d85be72 100644
--- a/arch/x86/kernel/e820.c
+++ b/arch/x86/kernel/e820.c
@@ -904,6 +904,8 @@ static int __init parse_memmap_one(char *p)
 	} else if (*p == '!') {
 		start_at = memparse(p+1, &p);
 		e820_add_region(start_at, mem_size, E820_PRAM);
+		if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
+			pr_warn("e820: CONFIG_RANDOMIZE_BASE enabled, kernel image may collide/overwrite the pmem range on subsequent boots!\n");
 	} else
 		e820_remove_range(mem_size, ULLONG_MAX - mem_size, E820_RAM, 1);
 

Re: [PATCH] x86: Add warning when memmap=nn!ss and CONFIG_RANDOMIZE_BASE enabled

[Thomas Gleixner]

On Thu, 17 Nov 2016, Dave Jiang wrote:
> CONFIG_RANDOMIZE_BASE can place the kernel anywhere. This causes a problem
> for when memmap=nn!ss is used. This information is not known until after
> the kernel starts executing and the decision for where the randomized
> base goes happens before the kernel is uncompressed. memmap=nn!ss is not
> reliable in the presence of CONFIG_RANDOMIZE_BASE.

So this is a description of a problem. Now what's missing is a useful
explanation why you think that adding a warning will make things better.

IMNSHO adding that warning is just a pointless exercise. 

Why aren't you addressing the real issue and make the boot code parse that
option and prevent that region from being used for kernel placement?
 
The same issue exists for other memmap options as well, not just for that
PMEM thingy.

Thanks,

	tglx

Re: [PATCH] x86: Add warning when memmap=nn!ss and CONFIG_RANDOMIZE_BASE enabled

[Dave Jiang]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 11/18/2016 04:33 AM, Thomas Gleixner wrote:
> On Thu, 17 Nov 2016, Dave Jiang wrote:
>> CONFIG_RANDOMIZE_BASE can place the kernel anywhere. This causes
>> a problem for when memmap=nn!ss is used. This information is not
>> known until after the kernel starts executing and the decision
>> for where the randomized base goes happens before the kernel is
>> uncompressed. memmap=nn!ss is not reliable in the presence of
>> CONFIG_RANDOMIZE_BASE.
> 
> So this is a description of a problem. Now what's missing is a
> useful explanation why you think that adding a warning will make
> things better.
> 
> IMNSHO adding that warning is just a pointless exercise.
> 
> Why aren't you addressing the real issue and make the boot code
> parse that option and prevent that region from being used for
> kernel placement?
> 
> The same issue exists for other memmap options as well, not just
> for that PMEM thingy.
> 
> Thanks,
> 
> tglx
> 

I wasn't planning to fix it because the pmem memmap option is really
only used for testing. Is it possible to parse the kernel commandline
parameters before the kernel is uncompressed?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iF4EAREIAAYFAlgvMIUACgkQZBXE8WajuT5aoAEAmXEJ0git9CwD4RhXV3+A/bmF
CN1iLKoXHOvcpQ0FcDsA/j8UKmobWBHyswb6RCSu2uSqTt6XreMG7uNamT1eMt0J
=hk/4
-----END PGP SIGNATURE-----

Re: [PATCH] x86: Add warning when memmap=nn!ss and CONFIG_RANDOMIZE_BASE enabled

[Dan Williams]

On Fri, Nov 18, 2016 at 8:47 AM, Dave Jiang <dave.jiang@intel.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
>
>
> On 11/18/2016 04:33 AM, Thomas Gleixner wrote:
>> On Thu, 17 Nov 2016, Dave Jiang wrote:
>>> CONFIG_RANDOMIZE_BASE can place the kernel anywhere. This causes
>>> a problem for when memmap=nn!ss is used. This information is not
>>> known until after the kernel starts executing and the decision
>>> for where the randomized base goes happens before the kernel is
>>> uncompressed. memmap=nn!ss is not reliable in the presence of
>>> CONFIG_RANDOMIZE_BASE.
>>
>> So this is a description of a problem. Now what's missing is a
>> useful explanation why you think that adding a warning will make
>> things better.
>>
>> IMNSHO adding that warning is just a pointless exercise.
>>
>> Why aren't you addressing the real issue and make the boot code
>> parse that option and prevent that region from being used for
>> kernel placement?
>>
>> The same issue exists for other memmap options as well, not just
>> for that PMEM thingy.
>>
>> Thanks,
>>
>> tglx
>>
>
> I wasn't planning to fix it because the pmem memmap option is really
> only used for testing. Is it possible to parse the kernel commandline
> parameters before the kernel is uncompressed?

Apologies, this was my mistake.  I missed that we have early boot
command line parsing in addition to the in-kernel cmdline parsing.

Dave, I think we could fix this in:
arch/x86/boot/compressed/kaslr.c::choose_random_location().

Re: [PATCH] x86: Add warning when memmap=nn!ss and CONFIG_RANDOMIZE_BASE enabled

[Dave Jiang]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 11/18/2016 10:07 AM, Dan Williams wrote:
> On Fri, Nov 18, 2016 at 8:47 AM, Dave Jiang <dave.jiang@intel.com>
> wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
>> 
>> 
>> 
>> On 11/18/2016 04:33 AM, Thomas Gleixner wrote:
>>> On Thu, 17 Nov 2016, Dave Jiang wrote:
>>>> CONFIG_RANDOMIZE_BASE can place the kernel anywhere. This
>>>> causes a problem for when memmap=nn!ss is used. This
>>>> information is not known until after the kernel starts
>>>> executing and the decision for where the randomized base goes
>>>> happens before the kernel is uncompressed. memmap=nn!ss is
>>>> not reliable in the presence of CONFIG_RANDOMIZE_BASE.
>>> 
>>> So this is a description of a problem. Now what's missing is a 
>>> useful explanation why you think that adding a warning will
>>> make things better.
>>> 
>>> IMNSHO adding that warning is just a pointless exercise.
>>> 
>>> Why aren't you addressing the real issue and make the boot
>>> code parse that option and prevent that region from being used
>>> for kernel placement?
>>> 
>>> The same issue exists for other memmap options as well, not
>>> just for that PMEM thingy.
>>> 
>>> Thanks,
>>> 
>>> tglx
>>> 
>> 
>> I wasn't planning to fix it because the pmem memmap option is
>> really only used for testing. Is it possible to parse the kernel
>> commandline parameters before the kernel is uncompressed?
> 
> Apologies, this was my mistake.  I missed that we have early boot 
> command line parsing in addition to the in-kernel cmdline parsing.
> 
> Dave, I think we could fix this in: 
> arch/x86/boot/compressed/kaslr.c::choose_random_location().
> 

Actually it was my fault. I was digging around that and somehow
totally missed the early command line parsing bits.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iF4EAREIAAYFAlgvNqEACgkQZBXE8WajuT71LAD9GGV5sL9bAkunWlZZylNGZ+Ug
XRfcnKB1g1+SJ4JFDuYA/RxrHAyBBhteIfSuORl5T+1ACYqwTA0MuHFwiASHXbuQ
=+j9S
-----END PGP SIGNATURE-----