public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [BUGFIX] x86_64: NX bit handling in change_page_attr
@ 2007-08-17  5:28 Huang, Ying
  2007-09-12  3:23 ` Andrew Morton
  0 siblings, 1 reply; 5+ messages in thread
From: Huang, Ying @ 2007-08-17  5:28 UTC (permalink / raw)
  To: Andi Kleen; +Cc: linux-kernel

This patch fixes a bug of change_page_attr/change_page_attr_addr on
Intel x86_64 CPU. After changing page attribute to be executable with
these functions, the page remains un-executable on Intel x86_64
CPU. Because on Intel x86_64 CPU, only if the "NX" bits of all four
level page tables are cleared, the corresponding page is executable
(refer to section 4.13.2 of Intel 64 and IA-32 Architectures Software
Developer's Manual). So, the bug is fixed through clearing the "NX"
bit of PMD when splitting the huge PMD.

Signed-off-by: Huang Ying <ying.huang@intel.com>

---

Index: linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c
===================================================================
--- linux-2.6.23-rc2-mm2.orig/arch/x86_64/mm/pageattr.c	2007-08-17 12:50:25.000000000 +0800
+++ linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c	2007-08-17 12:50:48.000000000 +0800
@@ -147,6 +147,7 @@
 			split = split_large_page(address, prot, ref_prot2);
 			if (!split)
 				return -ENOMEM;
+			pgprot_val(ref_prot2) &= ~_PAGE_NX;
 			set_pte(kpte, mk_pte(split, ref_prot2));
 			kpte_page = split;
 		}

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUGFIX] x86_64: NX bit handling in change_page_attr
  2007-08-17  5:28 [BUGFIX] x86_64: NX bit handling in change_page_attr Huang, Ying
@ 2007-09-12  3:23 ` Andrew Morton
  2007-09-12  6:18   ` Huang, Ying
  2007-09-12 13:35   ` Andi Kleen
  0 siblings, 2 replies; 5+ messages in thread
From: Andrew Morton @ 2007-09-12  3:23 UTC (permalink / raw)
  To: Huang, Ying; +Cc: Andi Kleen, linux-kernel

On Fri, 17 Aug 2007 13:28:38 +0800 "Huang, Ying" <ying.huang@intel.com> wrote:

> This patch fixes a bug of change_page_attr/change_page_attr_addr on
> Intel x86_64 CPU. After changing page attribute to be executable with
> these functions, the page remains un-executable on Intel x86_64
> CPU. Because on Intel x86_64 CPU, only if the "NX" bits of all four
> level page tables are cleared, the corresponding page is executable
> (refer to section 4.13.2 of Intel 64 and IA-32 Architectures Software
> Developer's Manual). So, the bug is fixed through clearing the "NX"
> bit of PMD when splitting the huge PMD.
> 
> Signed-off-by: Huang Ying <ying.huang@intel.com>
> 
> ---
> 
> Index: linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c
> ===================================================================
> --- linux-2.6.23-rc2-mm2.orig/arch/x86_64/mm/pageattr.c	2007-08-17 12:50:25.000000000 +0800
> +++ linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c	2007-08-17 12:50:48.000000000 +0800
> @@ -147,6 +147,7 @@
>  			split = split_large_page(address, prot, ref_prot2);
>  			if (!split)
>  				return -ENOMEM;
> +			pgprot_val(ref_prot2) &= ~_PAGE_NX;
>  			set_pte(kpte, mk_pte(split, ref_prot2));
>  			kpte_page = split;
>  		}

What happened with this?  Still valid?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUGFIX] x86_64: NX bit handling in change_page_attr
  2007-09-12  3:23 ` Andrew Morton
@ 2007-09-12  6:18   ` Huang, Ying
  2007-09-12 13:35   ` Andi Kleen
  1 sibling, 0 replies; 5+ messages in thread
From: Huang, Ying @ 2007-09-12  6:18 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andi Kleen, linux-kernel

On Tue, 2007-09-11 at 20:23 -0700, Andrew Morton wrote:
> On Fri, 17 Aug 2007 13:28:38 +0800 "Huang, Ying" <ying.huang@intel.com> wrote:
> 
> > This patch fixes a bug of change_page_attr/change_page_attr_addr on
> > Intel x86_64 CPU. After changing page attribute to be executable with
> > these functions, the page remains un-executable on Intel x86_64
> > CPU. Because on Intel x86_64 CPU, only if the "NX" bits of all four
> > level page tables are cleared, the corresponding page is executable
> > (refer to section 4.13.2 of Intel 64 and IA-32 Architectures Software
> > Developer's Manual). So, the bug is fixed through clearing the "NX"
> > bit of PMD when splitting the huge PMD.
> > 
> > Signed-off-by: Huang Ying <ying.huang@intel.com>
> > 
> > ---
> > 
> > Index: linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c
> > ===================================================================
> > --- linux-2.6.23-rc2-mm2.orig/arch/x86_64/mm/pageattr.c	2007-08-17 12:50:25.000000000 +0800
> > +++ linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c	2007-08-17 12:50:48.000000000 +0800
> > @@ -147,6 +147,7 @@
> >  			split = split_large_page(address, prot, ref_prot2);
> >  			if (!split)
> >  				return -ENOMEM;
> > +			pgprot_val(ref_prot2) &= ~_PAGE_NX;
> >  			set_pte(kpte, mk_pte(split, ref_prot2));
> >  			kpte_page = split;
> >  		}
> 
> What happened with this?  Still valid?

I am waiting for reviewing or merging. And I think it is still valid.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUGFIX] x86_64: NX bit handling in change_page_attr
  2007-09-12  3:23 ` Andrew Morton
  2007-09-12  6:18   ` Huang, Ying
@ 2007-09-12 13:35   ` Andi Kleen
  2007-09-13  1:12     ` Huang, Ying
  1 sibling, 1 reply; 5+ messages in thread
From: Andi Kleen @ 2007-09-12 13:35 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Huang, Ying, linux-kernel


> > Index: linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c
> > ===================================================================
> > --- linux-2.6.23-rc2-mm2.orig/arch/x86_64/mm/pageattr.c	2007-08-17
> > 12:50:25.000000000 +0800 +++
> > linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c	2007-08-17
> > 12:50:48.000000000 +0800 @@ -147,6 +147,7 @@
> >  			split = split_large_page(address, prot, ref_prot2);
> >  			if (!split)
> >  				return -ENOMEM;
> > +			pgprot_val(ref_prot2) &= ~_PAGE_NX;
> >  			set_pte(kpte, mk_pte(split, ref_prot2));
> >  			kpte_page = split;
> >  		}
>
> What happened with this?  Still valid?

The bug is probably latent there, but I don't think it can affect anything
in the kernel because nothing in the kernel should change NX status
as far as I know.

Where did you see it? 

Anyways I would prefer to only clear the PMD NX when NX status actually 
changes on the PTE.Can you do that change? 

Anyways; it's really not very important.

-Andi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [BUGFIX] x86_64: NX bit handling in change_page_attr
  2007-09-12 13:35   ` Andi Kleen
@ 2007-09-13  1:12     ` Huang, Ying
  0 siblings, 0 replies; 5+ messages in thread
From: Huang, Ying @ 2007-09-13  1:12 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andrew Morton, linux-kernel

On Wed, 2007-09-12 at 15:35 +0200, Andi Kleen wrote:
> > > Index: linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c
> > > ===================================================================
> > > --- linux-2.6.23-rc2-mm2.orig/arch/x86_64/mm/pageattr.c	2007-08-17
> > > 12:50:25.000000000 +0800 +++
> > > linux-2.6.23-rc2-mm2/arch/x86_64/mm/pageattr.c	2007-08-17
> > > 12:50:48.000000000 +0800 @@ -147,6 +147,7 @@
> > >  			split = split_large_page(address, prot, ref_prot2);
> > >  			if (!split)
> > >  				return -ENOMEM;
> > > +			pgprot_val(ref_prot2) &= ~_PAGE_NX;
> > >  			set_pte(kpte, mk_pte(split, ref_prot2));
> > >  			kpte_page = split;
> > >  		}
> >
> > What happened with this?  Still valid?
> 
> The bug is probably latent there, but I don't think it can affect anything
> in the kernel because nothing in the kernel should change NX status
> as far as I know.
> 
> Where did you see it? 

I found the problem for EFI runtime service supporting. Where the EFI
runtime code (from firmware) need to be mapped without NX bit set.

> Anyways I would prefer to only clear the PMD NX when NX status actually 
> changes on the PTE.Can you do that change? 

This change is sufficient for Intel CPU. Because the NX bit of PTE is
still there, no page will be made executable if not been set explicitly
through PTE. For AMD CPU, will the page be made executable if the NX bit
of PMD is cleared and the NX bit of PTE is set? If so, I will do the
change as you said.

> Anyways; it's really not very important.

It is needed by EFI runtime service supporting.

Best Regards,
Huang Ying

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-09-13  1:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-17  5:28 [BUGFIX] x86_64: NX bit handling in change_page_attr Huang, Ying
2007-09-12  3:23 ` Andrew Morton
2007-09-12  6:18   ` Huang, Ying
2007-09-12 13:35   ` Andi Kleen
2007-09-13  1:12     ` Huang, Ying

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox