[PATCH v1][CLEANUP] x86: use already defined macros instead of hard-coded values

[PATCH v1][CLEANUP] x86: use already defined macros instead of hard-coded values

[Alexander Kuleshov]

This patch provides following minor fixes:

* Remove non-used L3_PAGE_OFFSET
* Use already defined L3_START_KERNEL and other macros instead of hard-coded
values
* Fix paths in a comments

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 arch/x86/kernel/head_64.S | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 6fd514d9..dbc4ef3 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -1,6 +1,4 @@
 /*
- *  linux/arch/x86_64/kernel/head.S -- start in 32bit and switch to 64bit
- *
  *  Copyright (C) 2000 Andrea Arcangeli <andrea@suse.de> SuSE
  *  Copyright (C) 2000 Pavel Machek <pavel@suse.cz>
  *  Copyright (C) 2000 Karsten Keil <kkeil@suse.de>
@@ -20,6 +18,7 @@
 #include <asm/processor-flags.h>
 #include <asm/percpu.h>
 #include <asm/nops.h>
+#include <asm/cpufeature.h>
 
 #ifdef CONFIG_PARAVIRT
 #include <asm/asm-offsets.h>
@@ -38,7 +37,6 @@
 #define pud_index(x)	(((x) >> PUD_SHIFT) & (PTRS_PER_PUD-1))
 
 L4_PAGE_OFFSET = pgd_index(__PAGE_OFFSET)
-L3_PAGE_OFFSET = pud_index(__PAGE_OFFSET)
 L4_START_KERNEL = pgd_index(__START_KERNEL_map)
 L3_START_KERNEL = pud_index(__START_KERNEL_map)
 
@@ -56,7 +54,7 @@ startup_64:
 	 * %rsi holds a physical pointer to real_mode_data.
 	 *
 	 * We come here either directly from a 64bit bootloader, or from
-	 * arch/x86_64/boot/compressed/head.S.
+	 * arch/x86/boot/compressed/head_64.S.
 	 *
 	 * We only come here initially at boot nothing else comes here.
 	 *
@@ -90,8 +88,8 @@ startup_64:
 	 */
 	addq	%rbp, early_level4_pgt + (L4_START_KERNEL*8)(%rip)
 
-	addq	%rbp, level3_kernel_pgt + (510*8)(%rip)
-	addq	%rbp, level3_kernel_pgt + (511*8)(%rip)
+	addq	%rbp, level3_kernel_pgt + (L3_START_KERNEL*8)(%rip)
+	addq	%rbp, level3_kernel_pgt + (L3_START_KERNEL*8 + 1)(%rip)
 
 	addq	%rbp, level2_fixmap_pgt + (506*8)(%rip)
 
@@ -199,7 +197,7 @@ ENTRY(secondary_startup_64)
 	movl	$MSR_EFER, %ecx
 	rdmsr
 	btsl	$_EFER_SCE, %eax	/* Enable System Call */
-	btl	$20,%edi		/* No Execute supported? */
+	btl	$X86_FEATURE_NX, %edi	/* No Execute supported? */
 	jnc     1f
 	btsl	$_EFER_NX, %eax
 	btsq	$_PAGE_BIT_NX,early_pmd_flags(%rip)
@@ -441,7 +439,7 @@ GLOBAL(name)
 
 	__INITDATA
 NEXT_PAGE(early_level4_pgt)
-	.fill	511,8,0
+	.fill	L4_START_KERNEL,8,0
 	.quad	level3_kernel_pgt - __START_KERNEL_map + _PAGE_TABLE
 
 NEXT_PAGE(early_dynamic_pgts)
-- 
2.3.1.167.g7f4ba4b

Re: [PATCH v1][CLEANUP] x86: use already defined macros instead of hard-coded values

[Borislav Petkov]

On Thu, Mar 05, 2015 at 07:30:51PM +0600, Alexander Kuleshov wrote:
> This patch provides following minor fixes:
> 
> * Remove non-used L3_PAGE_OFFSET
> * Use already defined L3_START_KERNEL and other macros instead of hard-coded
> values
> * Fix paths in a comments
> 
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>

This patch introduces two new bugs. For educational purposes I'm not
going to tell you where they are but let you find them.

:-)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [PATCH v1][CLEANUP] x86: use already defined macros instead of hard-coded values

[Steven Rostedt]

On Thu, Mar 05, 2015 at 06:22:53PM +0100, Borislav Petkov wrote:
> 
> This patch introduces two new bugs. For educational purposes I'm not
> going to tell you where they are but let you find them.

Hmm, for educational purposes, I believe I found three bugs. Was the third
bug for extra credit?

-- Steve

Re: [PATCH v1][CLEANUP] x86: use already defined macros instead of hard-coded values

[Steven Rostedt]

On Thu, 5 Mar 2015 12:52:29 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, Mar 05, 2015 at 06:22:53PM +0100, Borislav Petkov wrote:
> > 
> > This patch introduces two new bugs. For educational purposes I'm not
> > going to tell you where they are but let you find them.
> 
> Hmm, for educational purposes, I believe I found three bugs. Was the third
> bug for extra credit?
> 

Nevermind, there isn't a third bug. It was a bug in my C code that
tested this patch. Used a 32bit version of PGDIR_SHIFT.

;-)

-- Steve

Re: [PATCH v1][CLEANUP] x86: use already defined macros instead of hard-coded values

[Alexander Kuleshov]

Hello Borislav,

>
> This patch introduces two new bugs. For educational purposes I'm not
> going to tell you where they are but let you find them.
>

Ok, i looked on it with gdb, and X86_FEATURE_NX is not correct here.
It's a bug number one.
And L3_START_KERNEL*8 + 1, must be L3_START_KERNEL*8 + 8, as we works
with page entries
or i'm wrong here?

Anyway, thanks for education:)

Re: [PATCH v1][CLEANUP] x86: use already defined macros instead of hard-coded values

[Borislav Petkov]

On Fri, Mar 06, 2015 at 12:12:09AM +0600, Alexander Kuleshov wrote:
> Ok, i looked on it with gdb, and X86_FEATURE_NX is not correct here.
> It's a bug number one.

Yes, X86_FEATURE_NX is not the same bit type as the bits you have in
%edi.

> And L3_START_KERNEL*8 + 1, must be L3_START_KERNEL*8 + 8, as we works
> with page entries
> or i'm wrong here?

Yes.

And look at the code after you've changed it. Do you think someone
looking at it a couple of months from now will know what

L3_START_KERNEL*8 + 8

or 

(L3_START_KERNEL + 1)*8

means?

So having 510 and 511 to mean the last two page table entries is much
more understandable.

Like I told you a couple of days ago, please don't waste your time with
stuff which already works and there are no problems with and go look for
real bugs. Believe me, there are plenty out there.

> Anyway, thanks for education:)

No problem.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--