Re: make PROT_WRITE imply PROT_READ

From: Jason Baron
Date: Thu Jul 06 2006 - 22:13:25 EST



On Thu, 29 Jun 2006, Jason Baron wrote:

>
> On Thu, 29 Jun 2006, Pavel Machek wrote:
>
> > Hi!
> >
> > > > > PROT_READ to be used or implicitly adding it. Don't confuse people
> > > > > with wrong statement like yours.
> > > >
> > > > Can you quote part of POSIX where it says that PROT_WRITE must imply
> > > > PROT_READ?
> > >
> > > I don't believe POSIX cares either way
> > >
> > > "An implementation may permit accesses other than those specified by
> > > prot; however, if the Memory Protection option is supported, the
> > > implementation shall not permit a write to succeed where PROT_WRITE has
> > > not been set or shall not permit any access where PROT_NONE alone has
> > > been set."
> > >
> > > However the current behaviour of "write to map read might work some days
> > > depending on the execution order of instructions" (and in some cases the
> > > order that the specific CPU does its tests for access rights) is not
> > > sane, not conducive to application stability and not good practice.
> >
> > Well, some architectures may have working PROT_WRITE without
> > PROT_READ. If you are careful and code your userland application in
> > assembly, it should work okay.
> >
> > On processors where that combination depends randomly depending on
> > phase of moon (i386, apparently), I guess change is okay. But the
> > patch disabled PROT_WRITE w/o PROT_READ on _all_ processors.
> >
> > Pavel
> >
>
>
> ok, the following patch should make x86 self-consistent, making PROT_WRITE
> imply PROT_READ.
>
> i can produce patches for other architectures, if people agree with this
> approach.
>
> thanks,
>
> -Jason
>
>
> --- linux-2.6/arch/i386/mm/fault.c.bak 2006-06-29 16:48:25.000000000 -0400
> +++ linux-2.6/arch/i386/mm/fault.c 2006-06-29 16:49:51.000000000 -0400
> @@ -449,7 +449,7 @@
> case 1: /* read, present */
> goto bad_area;
> case 0: /* read, not present */
> - if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
> + if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
> goto bad_area;
> }
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>


hearing no objections.....below is a patch to make PROT_WRITE imply
PROT_READ for a number of architectures which don't support write only
in hardware.

While looking at this, I noticed that some architectures which do not
support write only mappings already take the exact same approach. For
example, in arch/alpha/mm/fault.c:

"
if (cause < 0) {
if (!(vma->vm_flags & VM_EXEC))
goto bad_area;
} else if (!cause) {
/* Allow reads even for write-only mappings */
if (!(vma->vm_flags & (VM_READ | VM_WRITE)))
goto bad_area;
} else {
if (!(vma->vm_flags & VM_WRITE))
goto bad_area;
}
"

Thus, this patch brings other architectures which do not support write
only mappings in-line and consistent with the rest. I've verified the
patch on ia64, x86_64 and x86.

I've 'cc relevant architecture maintainers for comments.

thanks,

-Jason

Signed-off-by: Jason Baron <jbaron@xxxxxxxxxx>

--- linux-2.6/arch/sh/mm/fault.c.bak 2006-07-06 13:24:16.000000000 -0400
+++ linux-2.6/arch/sh/mm/fault.c 2006-07-06 13:24:37.000000000 -0400
@@ -80,7 +80,7 @@ good_area:
if (!(vma->vm_flags & VM_WRITE))
goto bad_area;
} else {
- if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
+ if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
goto bad_area;
}

--- linux-2.6/arch/arm26/mm/fault.c.bak 2006-07-06 13:22:20.000000000 -0400
+++ linux-2.6/arch/arm26/mm/fault.c 2006-07-06 13:21:56.000000000 -0400
@@ -155,7 +155,7 @@ __do_page_fault(struct mm_struct *mm, un
*/
good_area:
if (READ_FAULT(fsr)) /* read? */
- mask = VM_READ|VM_EXEC;
+ mask = VM_READ|VM_EXEC|VM_WRITE;
else
mask = VM_WRITE;

--- linux-2.6/arch/powerpc/mm/fault.c.bak 2006-07-06 13:15:04.000000000 -0400
+++ linux-2.6/arch/powerpc/mm/fault.c 2006-07-06 13:15:17.000000000 -0400
@@ -333,7 +333,7 @@ good_area:
/* protection fault */
if (error_code & 0x08000000)
goto bad_area;
- if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
+ if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
goto bad_area;
}

--- linux-2.6/arch/ia64/mm/fault.c.bak 2006-07-06 12:56:24.000000000 -0400
+++ linux-2.6/arch/ia64/mm/fault.c 2006-07-06 13:04:08.000000000 -0400
@@ -146,9 +146,11 @@ ia64_do_page_fault (unsigned long addres
# error File is out of sync with <linux/mm.h>. Please update.
# endif

+ if (((isr >> IA64_ISR_R_BIT) & 1UL) && (!(vma->vm_flags & (VM_READ | VM_WRITE))))
+ goto bad_area;
+
mask = ( (((isr >> IA64_ISR_X_BIT) & 1UL) << VM_EXEC_BIT)
- | (((isr >> IA64_ISR_W_BIT) & 1UL) << VM_WRITE_BIT)
- | (((isr >> IA64_ISR_R_BIT) & 1UL) << VM_READ_BIT));
+ | (((isr >> IA64_ISR_W_BIT) & 1UL) << VM_WRITE_BIT));

if ((vma->vm_flags & mask) != mask)
goto bad_area;
--- linux-2.6/arch/ppc/mm/fault.c.bak 2006-07-06 13:23:09.000000000 -0400
+++ linux-2.6/arch/ppc/mm/fault.c 2006-07-06 13:23:57.000000000 -0400
@@ -239,7 +239,7 @@ good_area:
/* protection fault */
if (error_code & 0x08000000)
goto bad_area;
- if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
+ if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
goto bad_area;
}

--- linux-2.6/arch/x86_64/mm/fault.c.bak 2006-07-06 13:25:12.000000000 -0400
+++ linux-2.6/arch/x86_64/mm/fault.c 2006-07-06 13:25:32.000000000 -0400
@@ -470,7 +470,7 @@ good_area:
case PF_PROT: /* read, present */
goto bad_area;
case 0: /* read, not present */
- if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
+ if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
goto bad_area;
}

--- linux-2.6/arch/arm/mm/fault.c.bak 2006-07-06 13:26:07.000000000 -0400
+++ linux-2.6/arch/arm/mm/fault.c 2006-07-06 13:26:14.000000000 -0400
@@ -170,7 +170,7 @@ good_area:
if (fsr & (1 << 11)) /* write? */
mask = VM_WRITE;
else
- mask = VM_READ|VM_EXEC;
+ mask = VM_READ|VM_EXEC|VM_WRITE;

fault = VM_FAULT_BADACCESS;
if (!(vma->vm_flags & mask))
--- linux-2.6/arch/m68k/mm/fault.c.bak 2006-07-06 13:06:42.000000000 -0400
+++ linux-2.6/arch/m68k/mm/fault.c 2006-07-06 13:07:17.000000000 -0400
@@ -144,7 +144,7 @@ good_area:
case 1: /* read, present */
goto acc_err;
case 0: /* read, not present */
- if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
+ if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
goto acc_err;
}

--- linux-2.6/arch/i386/mm/fault.c.bak 2006-06-29 16:48:25.000000000 -0400
+++ linux-2.6/arch/i386/mm/fault.c 2006-07-06 11:13:09.000000000 -0400
@@ -449,7 +449,7 @@ good_area:
case 1: /* read, present */
goto bad_area;
case 0: /* read, not present */
- if (!(vma->vm_flags & (VM_READ | VM_EXEC)))
+ if (!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)))
goto bad_area;
}


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/