[PATCH 0/2] RIXI fixes.

[PATCH 0/2] RIXI fixes.

[Ralf Baechle]

As part of unrelated work I ran into issus with RIXI support which allows
the creation of pages that are executable but not readable.  Problems
arise with instruction emulation on such pages, for example when a load
or store instruction is taking an unaligned exception, is an FPU
instruction that requires emulation, a trap or break instruction of
which the break code needs to bread and more.

A better solution would be to create a temporary mapping using
kmap_coherent() but that's more complex so left for a later stage.

These patches have been sitting in a shady git tree for years but seem to
work for me.  Nevertheless Comments appreciated.

Cheers,

  Ralf

Ralf Baechle (2):
  MIPS: Handle page faults of executable but unreadable pages correctly.
  MIPS: Partially disable RIXI support.

 arch/mips/mm/cache.c | 8 ++++----
 arch/mips/mm/fault.c | 3 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.4.3

[PATCH 1/2] MIPS: Handle page faults of executable but unreadable pages correctly.

[Ralf Baechle]

Without this we end taking execeptions in an endless loop hanging the
thread.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
---
 arch/mips/mm/fault.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
index 36c0f26..852a41c 100644
--- a/arch/mips/mm/fault.c
+++ b/arch/mips/mm/fault.c
@@ -133,7 +133,8 @@ good_area:
 #endif
 				goto bad_area;
 			}
-			if (!(vma->vm_flags & VM_READ)) {
+			if (!(vma->vm_flags & VM_READ) &&
+			    exception_epc(regs) != address) {
 #if 0
 				pr_notice("Cpu%d[%s:%d:%0*lx:%ld:%0*lx] RI violation\n",
 					  raw_smp_processor_id(),
-- 
2.4.3

Re: [PATCH 1/2] MIPS: Handle page faults of executable but unreadable pages correctly.

[James Hogan]

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

On 23/07/15 10:10, Ralf Baechle wrote:
> Without this we end taking execeptions in an endless loop hanging the
> thread.

A little more explanation would be nice. Under what situations does this
occur? Does this mean any VM_EXEC and !VM_READ page can't actually be
faulted in without it being treated as an RI violation, or does it only
affect when read from kernel emulation code?

Cheers
James

> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> ---
>  arch/mips/mm/fault.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
> index 36c0f26..852a41c 100644
> --- a/arch/mips/mm/fault.c
> +++ b/arch/mips/mm/fault.c
> @@ -133,7 +133,8 @@ good_area:
>  #endif
>  				goto bad_area;
>  			}
> -			if (!(vma->vm_flags & VM_READ)) {
> +			if (!(vma->vm_flags & VM_READ) &&
> +			    exception_epc(regs) != address) {
>  #if 0
>  				pr_notice("Cpu%d[%s:%d:%0*lx:%ld:%0*lx] RI violation\n",
>  					  raw_smp_processor_id(),
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] MIPS: Handle page faults of executable but unreadable pages correctly.

[James Hogan]

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

On 23/07/15 10:10, Ralf Baechle wrote:
> Without this we end taking execeptions in an endless loop hanging the
> thread.

A little more explanation would be nice. Under what situations does this
occur? Does this mean any VM_EXEC and !VM_READ page can't actually be
faulted in without it being treated as an RI violation, or does it only
affect when read from kernel emulation code?

Cheers
James

> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> ---
>  arch/mips/mm/fault.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/mm/fault.c b/arch/mips/mm/fault.c
> index 36c0f26..852a41c 100644
> --- a/arch/mips/mm/fault.c
> +++ b/arch/mips/mm/fault.c
> @@ -133,7 +133,8 @@ good_area:
>  #endif
>  				goto bad_area;
>  			}
> -			if (!(vma->vm_flags & VM_READ)) {
> +			if (!(vma->vm_flags & VM_READ) &&
> +			    exception_epc(regs) != address) {
>  #if 0
>  				pr_notice("Cpu%d[%s:%d:%0*lx:%ld:%0*lx] RI violation\n",
>  					  raw_smp_processor_id(),
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] MIPS: Handle page faults of executable but unreadable pages correctly.

[Ralf Baechle]

On Thu, Jul 23, 2015 at 11:15:15AM +0100, James Hogan wrote:

> On 23/07/15 10:10, Ralf Baechle wrote:
> > Without this we end taking execeptions in an endless loop hanging the
> > thread.
> 
> A little more explanation would be nice. Under what situations does this
> occur? Does this mean any VM_EXEC and !VM_READ page can't actually be
> faulted in without it being treated as an RI violation, or does it only
> affect when read from kernel emulation code?

> > -			if (!(vma->vm_flags & VM_READ)) {
> > +			if (!(vma->vm_flags & VM_READ) &&
> > +			    exception_epc(regs) != address) {
> >  #if 0
> >  				pr_notice("Cpu%d[%s:%d:%0*lx:%ld:%0*lx] RI violation\n",
> >  					  raw_smp_processor_id(),
> > 
> 

The general idea is the change the code to treat loads of an instruction
just like an instruction fetch.  Which is achieved by adding the second
condition "exception_epc(regs) != address" to the if above.
exception_epc(regs) == address means

It would all be easier if Linux was enabling the separate exception codes
for read and execution failure but short of that, a test like above must
provide if a fault was an attempted instruction fetch or happend fetching
data.

  Ralf

[PATCH 2/2] MIPS: Partially disable RIXI support.

[Ralf Baechle]

Execution of break instruction, trap instructions, emulation of unaligned
loads or floating point instructions - anything that tries to read the
instruction's opcode from userspace - needs read access to a page.

RIXI (Read Inhibit / Execute Inhibit) support however allows the creation of
pags that are executable but not readable.  On such a mapping the attempted
load of the opcode by the kernel is going to cause an endless loop of
page faults.

The quick workaround for this is to disable the combinations that the kernel
currently isn't able to handle which are executable mappings.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
---
 arch/mips/mm/cache.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
index 77d96db..aab218c 100644
--- a/arch/mips/mm/cache.c
+++ b/arch/mips/mm/cache.c
@@ -160,18 +160,18 @@ static inline void setup_protection_map(void)
 		protection_map[1]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
 		protection_map[2]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_NO_READ);
 		protection_map[3]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
-		protection_map[4]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
+		protection_map[4]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
 		protection_map[5]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
-		protection_map[6]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
+		protection_map[6]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
 		protection_map[7]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
 
 		protection_map[8]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_NO_READ);
 		protection_map[9]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
 		protection_map[10] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_WRITE | _PAGE_NO_READ);
 		protection_map[11] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_WRITE);
-		protection_map[12] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
+		protection_map[12] = __pgprot(_page_cachable_default | _PAGE_PRESENT);
 		protection_map[13] = __pgprot(_page_cachable_default | _PAGE_PRESENT);
-		protection_map[14] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE  | _PAGE_NO_READ);
+		protection_map[14] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE);
 		protection_map[15] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE);
 
 	} else {
-- 
2.4.3

Re: [PATCH 2/2] MIPS: Partially disable RIXI support.

[James Hogan]

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

On 23/07/15 10:10, Ralf Baechle wrote:
> Execution of break instruction, trap instructions, emulation of unaligned
> loads or floating point instructions - anything that tries to read the
> instruction's opcode from userspace - needs read access to a page.
> 
> RIXI (Read Inhibit / Execute Inhibit) support however allows the creation of
> pags that are executable but not readable.  On such a mapping the attempted
> load of the opcode by the kernel is going to cause an endless loop of
> page faults.
> 
> The quick workaround for this is to disable the combinations that the kernel
> currently isn't able to handle which are executable mappings.
> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
>  arch/mips/mm/cache.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
> index 77d96db..aab218c 100644
> --- a/arch/mips/mm/cache.c
> +++ b/arch/mips/mm/cache.c
> @@ -160,18 +160,18 @@ static inline void setup_protection_map(void)
>  		protection_map[1]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
>  		protection_map[2]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_NO_READ);
>  		protection_map[3]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
> -		protection_map[4]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
> +		protection_map[4]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  		protection_map[5]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
> -		protection_map[6]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
> +		protection_map[6]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  		protection_map[7]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  
>  		protection_map[8]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_NO_READ);
>  		protection_map[9]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
>  		protection_map[10] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_WRITE | _PAGE_NO_READ);
>  		protection_map[11] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_WRITE);
> -		protection_map[12] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
> +		protection_map[12] = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  		protection_map[13] = __pgprot(_page_cachable_default | _PAGE_PRESENT);
> -		protection_map[14] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE  | _PAGE_NO_READ);
> +		protection_map[14] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE);
>  		protection_map[15] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE);
>  
>  	} else {
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2] MIPS: Partially disable RIXI support.

[James Hogan]

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

On 23/07/15 10:10, Ralf Baechle wrote:
> Execution of break instruction, trap instructions, emulation of unaligned
> loads or floating point instructions - anything that tries to read the
> instruction's opcode from userspace - needs read access to a page.
> 
> RIXI (Read Inhibit / Execute Inhibit) support however allows the creation of
> pags that are executable but not readable.  On such a mapping the attempted
> load of the opcode by the kernel is going to cause an endless loop of
> page faults.
> 
> The quick workaround for this is to disable the combinations that the kernel
> currently isn't able to handle which are executable mappings.
> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
>  arch/mips/mm/cache.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/mips/mm/cache.c b/arch/mips/mm/cache.c
> index 77d96db..aab218c 100644
> --- a/arch/mips/mm/cache.c
> +++ b/arch/mips/mm/cache.c
> @@ -160,18 +160,18 @@ static inline void setup_protection_map(void)
>  		protection_map[1]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
>  		protection_map[2]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_NO_READ);
>  		protection_map[3]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
> -		protection_map[4]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
> +		protection_map[4]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  		protection_map[5]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
> -		protection_map[6]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
> +		protection_map[6]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  		protection_map[7]  = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  
>  		protection_map[8]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_NO_READ);
>  		protection_map[9]  = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC);
>  		protection_map[10] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_WRITE | _PAGE_NO_READ);
>  		protection_map[11] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_EXEC | _PAGE_WRITE);
> -		protection_map[12] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_NO_READ);
> +		protection_map[12] = __pgprot(_page_cachable_default | _PAGE_PRESENT);
>  		protection_map[13] = __pgprot(_page_cachable_default | _PAGE_PRESENT);
> -		protection_map[14] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE  | _PAGE_NO_READ);
> +		protection_map[14] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE);
>  		protection_map[15] = __pgprot(_page_cachable_default | _PAGE_PRESENT | _PAGE_WRITE);
>  
>  	} else {
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]