Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
On 03/15/17 at 02:13pm, Baoquan He wrote: > PING! > > Is there any suggestion for this code bug fix? > > Boris added comment in patch 1/2 thread that it can also be fixed by > swapping the naming - EFI_VA_START and EFI_VA_END. As he said the > swapping can remove the confusion about the naming, while the con is > changing it now could confuse more people who have the current > mental picture of the mapping direction. If swapping the naming is suggested, I can post v2 to change efi code. Both of them is fine to me. > > And there's also a well known similar use case, stack, like stack_end > naming in arch/x86/boot/main.c which is the low addr boundary of stack > region. > > Any idea? > > Thanks > Baoquan > > On 03/08/17 at 03:47pm, Baoquan He wrote: > > EFI allocates runtime services regions top-down, starting from EFI_VA_START > > to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of > > EFI region. The upper boundary of memory regions randomized by KASLR should > > be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. > > > > Correct it in this patch. > > > > Signed-off-by: Baoquan He> > --- > > arch/x86/mm/kaslr.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c > > index 887e571..aed2064 100644 > > --- a/arch/x86/mm/kaslr.c > > +++ b/arch/x86/mm/kaslr.c > > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = > > __PAGE_OFFSET_BASE; > > #if defined(CONFIG_X86_ESPFIX64) > > static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; > > #elif defined(CONFIG_EFI) > > -static const unsigned long vaddr_end = EFI_VA_START; > > +static const unsigned long vaddr_end = EFI_VA_END; > > #else > > static const unsigned long vaddr_end = __START_KERNEL_map; > > #endif > > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) > > */ > > BUILD_BUG_ON(vaddr_start >= vaddr_end); > > BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && > > -vaddr_end >= EFI_VA_START); > > +vaddr_end >= EFI_VA_END); > > BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || > > IS_ENABLED(CONFIG_EFI)) && > > vaddr_end >= __START_KERNEL_map); > > -- > > 2.5.5 > >
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
On 03/15/17 at 02:13pm, Baoquan He wrote: > PING! > > Is there any suggestion for this code bug fix? > > Boris added comment in patch 1/2 thread that it can also be fixed by > swapping the naming - EFI_VA_START and EFI_VA_END. As he said the > swapping can remove the confusion about the naming, while the con is > changing it now could confuse more people who have the current > mental picture of the mapping direction. If swapping the naming is suggested, I can post v2 to change efi code. Both of them is fine to me. > > And there's also a well known similar use case, stack, like stack_end > naming in arch/x86/boot/main.c which is the low addr boundary of stack > region. > > Any idea? > > Thanks > Baoquan > > On 03/08/17 at 03:47pm, Baoquan He wrote: > > EFI allocates runtime services regions top-down, starting from EFI_VA_START > > to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of > > EFI region. The upper boundary of memory regions randomized by KASLR should > > be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. > > > > Correct it in this patch. > > > > Signed-off-by: Baoquan He > > --- > > arch/x86/mm/kaslr.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c > > index 887e571..aed2064 100644 > > --- a/arch/x86/mm/kaslr.c > > +++ b/arch/x86/mm/kaslr.c > > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = > > __PAGE_OFFSET_BASE; > > #if defined(CONFIG_X86_ESPFIX64) > > static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; > > #elif defined(CONFIG_EFI) > > -static const unsigned long vaddr_end = EFI_VA_START; > > +static const unsigned long vaddr_end = EFI_VA_END; > > #else > > static const unsigned long vaddr_end = __START_KERNEL_map; > > #endif > > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) > > */ > > BUILD_BUG_ON(vaddr_start >= vaddr_end); > > BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && > > -vaddr_end >= EFI_VA_START); > > +vaddr_end >= EFI_VA_END); > > BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || > > IS_ENABLED(CONFIG_EFI)) && > > vaddr_end >= __START_KERNEL_map); > > -- > > 2.5.5 > >
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
PING! Is there any suggestion for this code bug fix? Boris added comment in patch 1/2 thread that it can also be fixed by swapping the naming - EFI_VA_START and EFI_VA_END. As he said the swapping can remove the confusion about the naming, while the con is changing it now could confuse more people who have the current mental picture of the mapping direction. And there's also a well known similar use case, stack, like stack_end naming in arch/x86/boot/main.c which is the low addr boundary of stack region. Any idea? Thanks Baoquan On 03/08/17 at 03:47pm, Baoquan He wrote: > EFI allocates runtime services regions top-down, starting from EFI_VA_START > to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of > EFI region. The upper boundary of memory regions randomized by KASLR should > be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. > > Correct it in this patch. > > Signed-off-by: Baoquan He> --- > arch/x86/mm/kaslr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c > index 887e571..aed2064 100644 > --- a/arch/x86/mm/kaslr.c > +++ b/arch/x86/mm/kaslr.c > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE; > #if defined(CONFIG_X86_ESPFIX64) > static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; > #elif defined(CONFIG_EFI) > -static const unsigned long vaddr_end = EFI_VA_START; > +static const unsigned long vaddr_end = EFI_VA_END; > #else > static const unsigned long vaddr_end = __START_KERNEL_map; > #endif > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >*/ > BUILD_BUG_ON(vaddr_start >= vaddr_end); > BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && > - vaddr_end >= EFI_VA_START); > + vaddr_end >= EFI_VA_END); > BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || > IS_ENABLED(CONFIG_EFI)) && >vaddr_end >= __START_KERNEL_map); > -- > 2.5.5 >
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
PING! Is there any suggestion for this code bug fix? Boris added comment in patch 1/2 thread that it can also be fixed by swapping the naming - EFI_VA_START and EFI_VA_END. As he said the swapping can remove the confusion about the naming, while the con is changing it now could confuse more people who have the current mental picture of the mapping direction. And there's also a well known similar use case, stack, like stack_end naming in arch/x86/boot/main.c which is the low addr boundary of stack region. Any idea? Thanks Baoquan On 03/08/17 at 03:47pm, Baoquan He wrote: > EFI allocates runtime services regions top-down, starting from EFI_VA_START > to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of > EFI region. The upper boundary of memory regions randomized by KASLR should > be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. > > Correct it in this patch. > > Signed-off-by: Baoquan He > --- > arch/x86/mm/kaslr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c > index 887e571..aed2064 100644 > --- a/arch/x86/mm/kaslr.c > +++ b/arch/x86/mm/kaslr.c > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE; > #if defined(CONFIG_X86_ESPFIX64) > static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; > #elif defined(CONFIG_EFI) > -static const unsigned long vaddr_end = EFI_VA_START; > +static const unsigned long vaddr_end = EFI_VA_END; > #else > static const unsigned long vaddr_end = __START_KERNEL_map; > #endif > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >*/ > BUILD_BUG_ON(vaddr_start >= vaddr_end); > BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && > - vaddr_end >= EFI_VA_START); > + vaddr_end >= EFI_VA_END); > BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || > IS_ENABLED(CONFIG_EFI)) && >vaddr_end >= __START_KERNEL_map); > -- > 2.5.5 >
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
Thanks for the change. Acked-by: Thomas GarnierOn Wed, Mar 8, 2017 at 12:35 AM, Bhupesh Sharma wrote: > On Wed, Mar 8, 2017 at 1:48 PM, Dave Young wrote: >> On 03/08/17 at 03:47pm, Baoquan He wrote: >>> EFI allocates runtime services regions top-down, starting from EFI_VA_START >>> to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of >>> EFI region. The upper boundary of memory regions randomized by KASLR should >>> be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. >>> >>> Correct it in this patch. >>> >>> Signed-off-by: Baoquan He >>> --- >>> arch/x86/mm/kaslr.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c >>> index 887e571..aed2064 100644 >>> --- a/arch/x86/mm/kaslr.c >>> +++ b/arch/x86/mm/kaslr.c >>> @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = >>> __PAGE_OFFSET_BASE; >>> #if defined(CONFIG_X86_ESPFIX64) >>> static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; >>> #elif defined(CONFIG_EFI) >>> -static const unsigned long vaddr_end = EFI_VA_START; >>> +static const unsigned long vaddr_end = EFI_VA_END; >>> #else >>> static const unsigned long vaddr_end = __START_KERNEL_map; >>> #endif >>> @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >>>*/ >>> BUILD_BUG_ON(vaddr_start >= vaddr_end); >>> BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && >>> - vaddr_end >= EFI_VA_START); >>> + vaddr_end >= EFI_VA_END); >>> BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || >>> IS_ENABLED(CONFIG_EFI)) && >>>vaddr_end >= __START_KERNEL_map); >>> -- >>> 2.5.5 >>> >> >> Acked-by: Dave Young >> > > Thanks Bao for this fix. This makes the KASLR code consistent with > Address space markers hints in [1] > > [1] http://lxr.free-electrons.com/source/arch/x86/mm/dump_pagetables.c#L82 > > Reviewed-by: Bhupesh Sharma > > Regards, > Bhupesh -- Thomas
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
Thanks for the change. Acked-by: Thomas Garnier On Wed, Mar 8, 2017 at 12:35 AM, Bhupesh Sharma wrote: > On Wed, Mar 8, 2017 at 1:48 PM, Dave Young wrote: >> On 03/08/17 at 03:47pm, Baoquan He wrote: >>> EFI allocates runtime services regions top-down, starting from EFI_VA_START >>> to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of >>> EFI region. The upper boundary of memory regions randomized by KASLR should >>> be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. >>> >>> Correct it in this patch. >>> >>> Signed-off-by: Baoquan He >>> --- >>> arch/x86/mm/kaslr.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c >>> index 887e571..aed2064 100644 >>> --- a/arch/x86/mm/kaslr.c >>> +++ b/arch/x86/mm/kaslr.c >>> @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = >>> __PAGE_OFFSET_BASE; >>> #if defined(CONFIG_X86_ESPFIX64) >>> static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; >>> #elif defined(CONFIG_EFI) >>> -static const unsigned long vaddr_end = EFI_VA_START; >>> +static const unsigned long vaddr_end = EFI_VA_END; >>> #else >>> static const unsigned long vaddr_end = __START_KERNEL_map; >>> #endif >>> @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >>>*/ >>> BUILD_BUG_ON(vaddr_start >= vaddr_end); >>> BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && >>> - vaddr_end >= EFI_VA_START); >>> + vaddr_end >= EFI_VA_END); >>> BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || >>> IS_ENABLED(CONFIG_EFI)) && >>>vaddr_end >= __START_KERNEL_map); >>> -- >>> 2.5.5 >>> >> >> Acked-by: Dave Young >> > > Thanks Bao for this fix. This makes the KASLR code consistent with > Address space markers hints in [1] > > [1] http://lxr.free-electrons.com/source/arch/x86/mm/dump_pagetables.c#L82 > > Reviewed-by: Bhupesh Sharma > > Regards, > Bhupesh -- Thomas
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
On Wed, Mar 8, 2017 at 1:48 PM, Dave Youngwrote: > On 03/08/17 at 03:47pm, Baoquan He wrote: >> EFI allocates runtime services regions top-down, starting from EFI_VA_START >> to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of >> EFI region. The upper boundary of memory regions randomized by KASLR should >> be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. >> >> Correct it in this patch. >> >> Signed-off-by: Baoquan He >> --- >> arch/x86/mm/kaslr.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c >> index 887e571..aed2064 100644 >> --- a/arch/x86/mm/kaslr.c >> +++ b/arch/x86/mm/kaslr.c >> @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = >> __PAGE_OFFSET_BASE; >> #if defined(CONFIG_X86_ESPFIX64) >> static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; >> #elif defined(CONFIG_EFI) >> -static const unsigned long vaddr_end = EFI_VA_START; >> +static const unsigned long vaddr_end = EFI_VA_END; >> #else >> static const unsigned long vaddr_end = __START_KERNEL_map; >> #endif >> @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >>*/ >> BUILD_BUG_ON(vaddr_start >= vaddr_end); >> BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && >> - vaddr_end >= EFI_VA_START); >> + vaddr_end >= EFI_VA_END); >> BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || >> IS_ENABLED(CONFIG_EFI)) && >>vaddr_end >= __START_KERNEL_map); >> -- >> 2.5.5 >> > > Acked-by: Dave Young > Thanks Bao for this fix. This makes the KASLR code consistent with Address space markers hints in [1] [1] http://lxr.free-electrons.com/source/arch/x86/mm/dump_pagetables.c#L82 Reviewed-by: Bhupesh Sharma Regards, Bhupesh
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
On Wed, Mar 8, 2017 at 1:48 PM, Dave Young wrote: > On 03/08/17 at 03:47pm, Baoquan He wrote: >> EFI allocates runtime services regions top-down, starting from EFI_VA_START >> to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of >> EFI region. The upper boundary of memory regions randomized by KASLR should >> be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. >> >> Correct it in this patch. >> >> Signed-off-by: Baoquan He >> --- >> arch/x86/mm/kaslr.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c >> index 887e571..aed2064 100644 >> --- a/arch/x86/mm/kaslr.c >> +++ b/arch/x86/mm/kaslr.c >> @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = >> __PAGE_OFFSET_BASE; >> #if defined(CONFIG_X86_ESPFIX64) >> static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; >> #elif defined(CONFIG_EFI) >> -static const unsigned long vaddr_end = EFI_VA_START; >> +static const unsigned long vaddr_end = EFI_VA_END; >> #else >> static const unsigned long vaddr_end = __START_KERNEL_map; >> #endif >> @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >>*/ >> BUILD_BUG_ON(vaddr_start >= vaddr_end); >> BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && >> - vaddr_end >= EFI_VA_START); >> + vaddr_end >= EFI_VA_END); >> BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || >> IS_ENABLED(CONFIG_EFI)) && >>vaddr_end >= __START_KERNEL_map); >> -- >> 2.5.5 >> > > Acked-by: Dave Young > Thanks Bao for this fix. This makes the KASLR code consistent with Address space markers hints in [1] [1] http://lxr.free-electrons.com/source/arch/x86/mm/dump_pagetables.c#L82 Reviewed-by: Bhupesh Sharma Regards, Bhupesh
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
On 03/08/17 at 03:47pm, Baoquan He wrote: > EFI allocates runtime services regions top-down, starting from EFI_VA_START > to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of > EFI region. The upper boundary of memory regions randomized by KASLR should > be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. > > Correct it in this patch. > > Signed-off-by: Baoquan He> --- > arch/x86/mm/kaslr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c > index 887e571..aed2064 100644 > --- a/arch/x86/mm/kaslr.c > +++ b/arch/x86/mm/kaslr.c > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE; > #if defined(CONFIG_X86_ESPFIX64) > static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; > #elif defined(CONFIG_EFI) > -static const unsigned long vaddr_end = EFI_VA_START; > +static const unsigned long vaddr_end = EFI_VA_END; > #else > static const unsigned long vaddr_end = __START_KERNEL_map; > #endif > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >*/ > BUILD_BUG_ON(vaddr_start >= vaddr_end); > BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && > - vaddr_end >= EFI_VA_START); > + vaddr_end >= EFI_VA_END); > BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || > IS_ENABLED(CONFIG_EFI)) && >vaddr_end >= __START_KERNEL_map); > -- > 2.5.5 > Acked-by: Dave Young Thanks Dave
Re: [PATCH 2/2] x86/mm/KASLR: Correct the upper boundary of KALSR mm regions if adjacent to EFI
On 03/08/17 at 03:47pm, Baoquan He wrote: > EFI allocates runtime services regions top-down, starting from EFI_VA_START > to EFI_VA_END. So EFI_VA_START is bigger than EFI_VA_END and is the end of > EFI region. The upper boundary of memory regions randomized by KASLR should > be EFI_VA_END if it's adjacent to EFI region, but not EFI_VA_START. > > Correct it in this patch. > > Signed-off-by: Baoquan He > --- > arch/x86/mm/kaslr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c > index 887e571..aed2064 100644 > --- a/arch/x86/mm/kaslr.c > +++ b/arch/x86/mm/kaslr.c > @@ -48,7 +48,7 @@ static const unsigned long vaddr_start = __PAGE_OFFSET_BASE; > #if defined(CONFIG_X86_ESPFIX64) > static const unsigned long vaddr_end = ESPFIX_BASE_ADDR; > #elif defined(CONFIG_EFI) > -static const unsigned long vaddr_end = EFI_VA_START; > +static const unsigned long vaddr_end = EFI_VA_END; > #else > static const unsigned long vaddr_end = __START_KERNEL_map; > #endif > @@ -105,7 +105,7 @@ void __init kernel_randomize_memory(void) >*/ > BUILD_BUG_ON(vaddr_start >= vaddr_end); > BUILD_BUG_ON(IS_ENABLED(CONFIG_X86_ESPFIX64) && > - vaddr_end >= EFI_VA_START); > + vaddr_end >= EFI_VA_END); > BUILD_BUG_ON((IS_ENABLED(CONFIG_X86_ESPFIX64) || > IS_ENABLED(CONFIG_EFI)) && >vaddr_end >= __START_KERNEL_map); > -- > 2.5.5 > Acked-by: Dave Young Thanks Dave