Re: make PROT_WRITE imply PROT_READ

Re: make PROT_WRITE imply PROT_READ

[Robert Hancock]

Jason Baron wrote:
> Currently, if i mmap() a file PROT_WRITE only and then first read from it 
> and then write to it, i get a SEGV. However, if i write to it first and 
> then read from it, i get no SEGV. This seems rather inconsistent.
> 
> The current implementation seems to be to make PROT_WRITE imply PROT_READ, 
> however it does not quite work correctly. The patch below resolves this 
> issue, by explicitly setting the PROT_READ flag when PROT_WRITE is 
> requested.

I would disagree.. the kernel is enforcing the permissions specified 
where the CPU architecture allows it. There is no sense in breaking this 
everywhere just because we can't always enforce it. By that logic we 
should be making PROT_READ imply PROT_EXEC because not all CPUs can 
enforce them separately, which makes no sense at all.

> 
> This might appear at first as a possible permissions subversion, as i 
> could get PROT_READ on a file that i only have write permission 
> to...however, the mmap implementation requires that the file be opened 
> with at least read access already. Thus, i don't believe there is any 
> issue with regards to permissions.
> 
> Another consequenece of this patch is that it forces PROT_READ even for 
> architectures that might be able to support it, (I know that x86, x86_64 
> and ia64 do not) but i think this is best for portability.

That makes little sense to me.. if you want portability, and you're 
reading from the file, you better request PROT_READ. Any app that 
doesn't do that is inherently broken and non-portable regardless of what 
you do to the kernel.

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: make PROT_WRITE imply PROT_READ

[Jason Baron]

On Thu, 22 Jun 2006, Robert Hancock wrote:

> Jason Baron wrote:
> > Currently, if i mmap() a file PROT_WRITE only and then first read from it
> > and then write to it, i get a SEGV. However, if i write to it first and then
> > read from it, i get no SEGV. This seems rather inconsistent.
> > 
> > The current implementation seems to be to make PROT_WRITE imply PROT_READ,
> > however it does not quite work correctly. The patch below resolves this
> > issue, by explicitly setting the PROT_READ flag when PROT_WRITE is
> > requested.
> 
> I would disagree.. the kernel is enforcing the permissions specified where the
> CPU architecture allows it. There is no sense in breaking this everywhere just
> because we can't always enforce it. By that logic we should be making
> PROT_READ imply PROT_EXEC because not all CPUs can enforce them separately,
> which makes no sense at all.
> 
> > 
> > This might appear at first as a possible permissions subversion, as i could
> > get PROT_READ on a file that i only have write permission to...however, the
> > mmap implementation requires that the file be opened with at least read
> > access already. Thus, i don't believe there is any issue with regards to
> > permissions.
> > 
> > Another consequenece of this patch is that it forces PROT_READ even for
> > architectures that might be able to support it, (I know that x86, x86_64 and
> > ia64 do not) but i think this is best for portability.
> 
> That makes little sense to me.. if you want portability, and you're reading
> from the file, you better request PROT_READ. Any app that doesn't do that is
> inherently broken and non-portable regardless of what you do to the kernel.
> 
> 


hi,

So if i create a PROT_WRITE only mapping and then read from first and then 
writte to it a get a SEGV. However, if i write to it first and then read 
from it, i don't get a SEGV...Why should the read/write ordering matter? 

thanks,

-Jason 

Re: make PROT_WRITE imply PROT_READ

[Arjan van de Ven]

> 
> hi,
> 
> So if i create a PROT_WRITE only mapping and then read from first and then 
> writte to it a get a SEGV. However, if i write to it first and then read 
> from it, i don't get a SEGV...Why should the read/write ordering matter? 

it matters only on those cpus that don't have explicit/separate read
permissions (just like PROT_EXEC is implied on cpus that don't have a
special permission bit for that)...

Re: make PROT_WRITE imply PROT_READ

[Jason Baron]

On Fri, 23 Jun 2006, Arjan van de Ven wrote:

> 
> > 
> > hi,
> > 
> > So if i create a PROT_WRITE only mapping and then read from first and then 
> > writte to it a get a SEGV. However, if i write to it first and then read 
> > from it, i don't get a SEGV...Why should the read/write ordering matter? 
> 
> it matters only on those cpus that don't have explicit/separate read
> permissions (just like PROT_EXEC is implied on cpus that don't have a
> special permission bit for that)...
> 
> 
> 

i understand that, but i'd like to see it changed so that i don't get a 
SEGV when i read first on those cpus. The current behavior, besides being 
inconsistent, is rather confusing...in addition, if ptes are copied 
instead of faulted i get yet another type of behavior...

-Jason

Re: make PROT_WRITE imply PROT_READ

[Arjan van de Ven]

On Fri, 2006-06-23 at 10:05 -0400, Jason Baron wrote:
> On Fri, 23 Jun 2006, Arjan van de Ven wrote:
> 
> > 
> > > 
> > > hi,
> > > 
> > > So if i create a PROT_WRITE only mapping and then read from first and then 
> > > writte to it a get a SEGV. However, if i write to it first and then read 
> > > from it, i don't get a SEGV...Why should the read/write ordering matter? 
> > 
> > it matters only on those cpus that don't have explicit/separate read
> > permissions (just like PROT_EXEC is implied on cpus that don't have a
> > special permission bit for that)...
> > 
> > 
> > 
> 
> i understand that, but i'd like to see it changed so that i don't get a 
> SEGV when i read first on those cpus. ?

why? You asked for this you get it in all cases we can deliver what you
asked...

> The current behavior, besides being 
> inconsistent, is rather confusing...in addition, if ptes are copied 
> instead of faulted i get yet another type of behavior...

same for prefault I suppose, but still.

you ask for it, and the kernel is supposed to deliver the best behavior
it can.

Re: make PROT_WRITE imply PROT_READ

[Ulrich Drepper]

On 6/23/06, Arjan van de Ven <arjan@infradead.org> wrote:
> you ask for it, and the kernel is supposed to deliver the best behavior
> it can.

The kernel should provide

- a stable, reliable interface

- a consistent interface at least accross architectures, maybe even platforms


Providing write-only support for memory falls into none of these
categories.  When Jason and I discussed this my position actually was
to disallow PROT_WRITE without PROT_READ completely, making it an
error of mmap and mprotect.  That's perfectly legal according to POSIX
and it will teach those who write broken code like this.

Jason's proposal to implicitly set PROT_READ is the second best
solution.  It gets rid of the error cases which a developer might not
notice, either because of lucky circumstances or details of the
architecture.

The reality is that almost no platform can really implement write-only
memory.  RISC architectures implement sub-word write as word reads,
modify, word write.  And even for CISC archs the compiler, for
instance, might decide to read memory, be it for prefetching or
explicit write-combining.

I'm strongly in favor of adding of adding one of the two possible
patches (forbidding, implicitly setting PROT_READ).  This will help
preventing these kinds of programming mistakes from spreading and we
already _know_ that there are such programs out there.

Re: make PROT_WRITE imply PROT_READ

[Pavel Machek]

Hi!

> >you ask for it, and the kernel is supposed to deliver the best behavior
> >it can.
> 
> The kernel should provide
> 
> - a stable, reliable interface
> 
> - a consistent interface at least accross architectures, maybe even 
> platforms
> 
> 
> Providing write-only support for memory falls into none of these
> categories.  When Jason and I discussed this my position actually was
> to disallow PROT_WRITE without PROT_READ completely, making it an
> error of mmap and mprotect.  That's perfectly legal according to POSIX
> and it will teach those who write broken code like this.

Well, some hardware can probably support write-only, and such support
can be useful for "weird" applications, such as just-in-time
compilers, etc.

Usability for "normal" C applications is probably not too high... so
why not work around it in glibc, if at all?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: make PROT_WRITE imply PROT_READ

[Arjan van de Ven]

> Well, some hardware can probably support write-only, and such support
> can be useful for "weird" applications, such as just-in-time
> compilers, etc.

it also makes sense for PCI MMIO mappings..

Re: make PROT_WRITE imply PROT_READ

[Ulrich Drepper]

On 6/27/06, Pavel Machek <pavel@ucw.cz> wrote:
> Usability for "normal" C applications is probably not too high... so
> why not work around it in glibc, if at all?

Because it wouldn't affect all b inaries.  Existing code could still
cause the problem.  Also, there are other callers of the syscalls
(direct, other libcs, etc).  The only reliable way to get rid of this
problem is to enforce it in the kernel.  Since the kernel cannot make
sense of this setting in all situations it is IMO even necessary since
you really don't want to have anything as unstable as this code.

Re: make PROT_WRITE imply PROT_READ

[Pavel Machek]

On Wed 2006-06-28 09:43:22, Ulrich Drepper wrote:
> On 6/27/06, Pavel Machek <pavel@ucw.cz> wrote:
> >Usability for "normal" C applications is probably not too high... so
> >why not work around it in glibc, if at all?
> 
> Because it wouldn't affect all b inaries.  Existing code could still
> cause the problem.  Also, there are other callers of the syscalls

_There is no problem_. 

mmap() behaviour always was platform-specific, and it happens to be
quite strange on i386. So what.

> (direct, other libcs, etc).  The only reliable way to get rid of this
> problem is to enforce it in the kernel.  Since the kernel cannot make
> sense of this setting in all situations it is IMO even necessary since
> you really don't want to have anything as unstable as this code.

Current kernel behaviour is useful for specialized apps. If you do not
want to see that weirdness in regular c application, work around it in
glibc.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: make PROT_WRITE imply PROT_READ

[Chase Venters]

On Wed, 28 Jun 2006, Pavel Machek wrote:

> On Wed 2006-06-28 09:43:22, Ulrich Drepper wrote:
>> On 6/27/06, Pavel Machek <pavel@ucw.cz> wrote:
>>> Usability for "normal" C applications is probably not too high... so
>>> why not work around it in glibc, if at all?
>>
>> Because it wouldn't affect all b inaries.  Existing code could still
>> cause the problem.  Also, there are other callers of the syscalls
>
> _There is no problem_.
>
> mmap() behaviour always was platform-specific, and it happens to be
> quite strange on i386. So what.

Hell, IIRC, on ConvexOS 11, the second argument to mmap() is a /pointer/ 
to the length.

>> (direct, other libcs, etc).  The only reliable way to get rid of this
>> problem is to enforce it in the kernel.  Since the kernel cannot make
>> sense of this setting in all situations it is IMO even necessary since
>> you really don't want to have anything as unstable as this code.
>
> Current kernel behaviour is useful for specialized apps. If you do not
> want to see that weirdness in regular c application, work around it in
> glibc.
> 									Pavel
>

Thanks,
Chase

Re: make PROT_WRITE imply PROT_READ

[Ulrich Drepper]

On 6/28/06, Pavel Machek <pavel@ucw.cz> wrote:
> mmap() behaviour always was platform-specific, and it happens to be
> quite strange on i386. So what.

Nonsense.  The mmap semantics is specified in POSIX.  If something
doesn't work as requested it is a bug.  For the specific issue hurting
x86 and likely others the standard explicitly allows requiring
PROT_READ to be used or implicitly adding it.  Don't confuse people
with wrong statement like yours.

Re: make PROT_WRITE imply PROT_READ

[Pavel Machek]

On Wed 2006-06-28 16:47:00, Ulrich Drepper wrote:
> On 6/28/06, Pavel Machek <pavel@ucw.cz> wrote:
> >mmap() behaviour always was platform-specific, and it happens to be
> >quite strange on i386. So what.
> 
> Nonsense.  The mmap semantics is specified in POSIX.  If something
> doesn't work as requested it is a bug.  For the specific issue hurting
> x86 and likely others the standard explicitly allows requiring
> 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?
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: make PROT_WRITE imply PROT_READ

[Alan Cox]

Ar Iau, 2006-06-29 am 09:30 +0200, ysgrifennodd Pavel Machek:
> > 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.

Alan

Re: make PROT_WRITE imply PROT_READ

[Pavel Machek]

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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: make PROT_WRITE imply PROT_READ

[Jason Baron]

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;
 	}
 

Re: make PROT_WRITE imply PROT_READ

[Jason Baron]

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@vger.kernel.org
> 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@redhat.com>

--- 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;
 	}
 
 

Re: make PROT_WRITE imply PROT_READ

[Ulrich Drepper]

On 6/29/06, Pavel Machek <pavel@suse.cz> wrote:
> Can you quote part of POSIX where it says that PROT_WRITE must imply
> PROT_READ?

I never wrote that.  What POSIX does specify is that PROT_READ can
optionally be implicitly set.

Re: make PROT_WRITE imply PROT_READ

[Arjan van de Ven]

On Wed, 2006-06-28 at 09:43 -0700, Ulrich Drepper wrote:
> On 6/27/06, Pavel Machek <pavel@ucw.cz> wrote:
> > Usability for "normal" C applications is probably not too high... so
> > why not work around it in glibc, if at all?
> 
> Because it wouldn't affect all b inaries.  Existing code could still
> cause the problem.  Also, there are other callers of the syscalls
> (direct, other libcs, etc).  The only reliable way to get rid of this
> problem is to enforce it in the kernel.  Since the kernel cannot make
> sense of this setting in all situations it is IMO even necessary since
> you really don't want to have anything as unstable as this code.

the thing is.. you can say EXACTLY the same about PROT_EXEC.. not all
processors support enforcing that.. so should we just always imply
PROT_EXEC as well?

Re: make PROT_WRITE imply PROT_READ

[Ulrich Drepper]

On 6/29/06, Arjan van de Ven <arjan@infradead.org> wrote:
> the thing is.. you can say EXACTLY the same about PROT_EXEC.. not all
> processors support enforcing that.. so should we just always imply
> PROT_EXEC as well?

There is a fundamental difference: not setting PROT_EXEC has no
negative side effects.  You might be able to execute code and it just
works.

With PROT_READ this is not the case, there _are_ side effects which are visible.

Re: make PROT_WRITE imply PROT_READ

[Arjan van de Ven]

On Thu, 2006-06-29 at 20:48 -0700, Ulrich Drepper wrote:
> On 6/29/06, Arjan van de Ven <arjan@infradead.org> wrote:
> > the thing is.. you can say EXACTLY the same about PROT_EXEC.. not all
> > processors support enforcing that.. so should we just always imply
> > PROT_EXEC as well?
> 
> There is a fundamental difference: not setting PROT_EXEC has no
> negative side effects.  You might be able to execute code and it just
> works.
> 
> With PROT_READ this is not the case, there _are_ side effects which are visible.

there are side effects which are visible with PROT_EXEC too, and even
the same kind...

with PROT_READ you may read even if you didn't specify it
with PROT_EXEC you may execute even if you didn't specifiy it

apps like JVM's forgot PROT_EXEC and break when the hardware enforces it
apps that forget PROT_READ break when the kernel/hardware enforce it

not too much difference....

Re: make PROT_WRITE imply PROT_READ

[Alan Cox]

Ar Gwe, 2006-06-30 am 10:35 +0200, ysgrifennodd Arjan van de Ven:
> apps like JVM's forgot PROT_EXEC and break when the hardware enforces it
> apps that forget PROT_READ break when the kernel/hardware enforce it
> 
> not too much difference....

There is quite a difference. The _EXEC case behaves predictably for the
platforms that support it. At least I am not aware of cases that is not
true. The _READ case without the fault handling patch behaves
unpredictably depending on the precise ordering of events on a clean
page. 

Alan

Re: make PROT_WRITE imply PROT_READ

[Bodo Eggert]

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> Ar Gwe, 2006-06-30 am 10:35 +0200, ysgrifennodd Arjan van de Ven:

>> apps like JVM's forgot PROT_EXEC and break when the hardware enforces it
>> apps that forget PROT_READ break when the kernel/hardware enforce it
>> 
>> not too much difference....
> 
> There is quite a difference. The _EXEC case behaves predictably for the
> platforms that support it. At least I am not aware of cases that is not
> true. The _READ case without the fault handling patch behaves

predictable on platforms that support it, but since you're testing on a
platform where it isn't fully supported, it behaves

> unpredictably depending on the precise ordering of events on a clean
> page.

You asked for a fault, and as long as the hardware supports it, you'll
get one (and you're supposed to). If the hardware doesn't support read
faults on mapped pages, you may not get all the read faults you want. The
proposed patch makes the situation worse by disabeling the _requested_
failures even in situations where it can be done.

-- 
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.

http://david.woodhou.se/why-not-spf.html

Re: make PROT_WRITE imply PROT_READ

[Alan Cox]

Ar Sad, 2006-07-01 am 15:19 +0200, ysgrifennodd Bodo Eggert:
> > unpredictably depending on the precise ordering of events on a clean
> > page.
> 
> You asked for a fault, and as long as the hardware supports it, you'll
> get one (and you're supposed to). If the hardware doesn't support read
> faults on mapped pages, you may not get all the read faults you want. The
> proposed patch makes the situation worse by disabeling the _requested_
> failures even in situations where it can be done.

The later patch as posted has no effect on such platforms because it
does not touch anything but the architecture code. Without that its
random what happens because the CPU cannot enforce write only but the
fault handler tries to. That means if you fault reading because the page
is not present you may get a fault while if you access a page which is
present you won't get a fault.

That gets quite random and has bizarre effects.

Alan

Re: make PROT_WRITE imply PROT_READ

[Bodo Eggert]

On Sun, 2 Jul 2006, Alan Cox wrote:
> Ar Sad, 2006-07-01 am 15:19 +0200, ysgrifennodd Bodo Eggert:

> > > unpredictably depending on the precise ordering of events on a clean
> > > page.
> > 
> > You asked for a fault, and as long as the hardware supports it, you'll
> > get one (and you're supposed to). If the hardware doesn't support read
> > faults on mapped pages, you may not get all the read faults you want. The
> > proposed patch makes the situation worse by disabeling the _requested_
> > failures even in situations where it can be done.
> 
> The later patch as posted has no effect on such platforms

I'm talking about the affected platforms.

> because it
> does not touch anything but the architecture code. Without that its
> random what happens because the CPU cannot enforce write only but the
> fault handler tries to. That means if you fault reading because the page
> is not present you may get a fault while if you access a page which is
> present you won't get a fault.

IMO it's the best we can get, even if the results are ...

> That gets quite random and has bizarre effects.

OTOH, there is not much difference between randomly wrong and consistently 
wrong, so I shall be happy either way (as if it would even matter).
-- 
'Calm down -- it's only ones and zeros.' 

make PROT_WRITE imply PROT_READ

[Jason Baron]

Hi,

Currently, if i mmap() a file PROT_WRITE only and then first read from it 
and then write to it, i get a SEGV. However, if i write to it first and 
then read from it, i get no SEGV. This seems rather inconsistent.

The current implementation seems to be to make PROT_WRITE imply PROT_READ, 
however it does not quite work correctly. The patch below resolves this 
issue, by explicitly setting the PROT_READ flag when PROT_WRITE is 
requested.

This might appear at first as a possible permissions subversion, as i 
could get PROT_READ on a file that i only have write permission 
to...however, the mmap implementation requires that the file be opened 
with at least read access already. Thus, i don't believe there is any 
issue with regards to permissions.

Another consequenece of this patch is that it forces PROT_READ even for 
architectures that might be able to support it, (I know that x86, x86_64 
and ia64 do not) but i think this is best for portability.

This was originally reported by Doug Chapman.

thanks,

-Jason


Signed-off-by: Jason Baron <jbaron@redhat.com>

--- linux-2.6/mm/mmap.c.bak	2006-06-21 17:07:52.000000000 -0400
+++ linux-2.6/mm/mmap.c	2006-06-21 17:22:54.000000000 -0400
@@ -910,6 +910,13 @@
 		if (!(file && (file->f_vfsmnt->mnt_flags & MNT_NOEXEC)))
 			prot |= PROT_EXEC;
 
+	 /* SuSv3: "if the application requests only PROT_WRITE, the 
+	 *          implementation may also allow read access."
+ 	 */
+
+	if (prot & PROT_WRITE)
+		prot |= PROT_READ;
+
 	if (!len)
 		return -EINVAL;