* Re: make PROT_WRITE imply PROT_READ [not found] <fa.PuMM6IwflUYh1MWILO9rb6z4fvY@ifi.uio.no> @ 2006-06-23 1:24 ` Robert Hancock 2006-06-23 13:39 ` Jason Baron 0 siblings, 1 reply; 26+ messages in thread From: Robert Hancock @ 2006-06-23 1:24 UTC (permalink / raw) To: Jason Baron; +Cc: akpm, linux-kernel 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/ ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-23 1:24 ` make PROT_WRITE imply PROT_READ Robert Hancock @ 2006-06-23 13:39 ` Jason Baron 2006-06-23 14:06 ` Arjan van de Ven 0 siblings, 1 reply; 26+ messages in thread From: Jason Baron @ 2006-06-23 13:39 UTC (permalink / raw) To: Robert Hancock; +Cc: akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-23 13:39 ` Jason Baron @ 2006-06-23 14:06 ` Arjan van de Ven 2006-06-23 14:05 ` Jason Baron 0 siblings, 1 reply; 26+ messages in thread From: Arjan van de Ven @ 2006-06-23 14:06 UTC (permalink / raw) To: Jason Baron; +Cc: Robert Hancock, akpm, linux-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? 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)... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-23 14:06 ` Arjan van de Ven @ 2006-06-23 14:05 ` Jason Baron 2006-06-23 14:18 ` Arjan van de Ven 0 siblings, 1 reply; 26+ messages in thread From: Jason Baron @ 2006-06-23 14:05 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Robert Hancock, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-23 14:05 ` Jason Baron @ 2006-06-23 14:18 ` Arjan van de Ven 2006-06-24 18:45 ` Ulrich Drepper 0 siblings, 1 reply; 26+ messages in thread From: Arjan van de Ven @ 2006-06-23 14:18 UTC (permalink / raw) To: Jason Baron; +Cc: Robert Hancock, akpm, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-23 14:18 ` Arjan van de Ven @ 2006-06-24 18:45 ` Ulrich Drepper 2006-06-27 9:56 ` Pavel Machek 0 siblings, 1 reply; 26+ messages in thread From: Ulrich Drepper @ 2006-06-24 18:45 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Jason Baron, akpm, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-24 18:45 ` Ulrich Drepper @ 2006-06-27 9:56 ` Pavel Machek 2006-06-27 12:18 ` Arjan van de Ven 2006-06-28 16:43 ` Ulrich Drepper 0 siblings, 2 replies; 26+ messages in thread From: Pavel Machek @ 2006-06-27 9:56 UTC (permalink / raw) To: Ulrich Drepper; +Cc: Arjan van de Ven, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-27 9:56 ` Pavel Machek @ 2006-06-27 12:18 ` Arjan van de Ven 2006-06-28 16:43 ` Ulrich Drepper 1 sibling, 0 replies; 26+ messages in thread From: Arjan van de Ven @ 2006-06-27 12:18 UTC (permalink / raw) To: Pavel Machek; +Cc: Ulrich Drepper, Jason Baron, akpm, linux-kernel > 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.. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-27 9:56 ` Pavel Machek 2006-06-27 12:18 ` Arjan van de Ven @ 2006-06-28 16:43 ` Ulrich Drepper 2006-06-28 19:49 ` Pavel Machek 2006-06-29 8:15 ` Arjan van de Ven 1 sibling, 2 replies; 26+ messages in thread From: Ulrich Drepper @ 2006-06-28 16:43 UTC (permalink / raw) To: Pavel Machek; +Cc: Arjan van de Ven, Jason Baron, akpm, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-28 16:43 ` Ulrich Drepper @ 2006-06-28 19:49 ` Pavel Machek 2006-06-28 20:05 ` Chase Venters 2006-06-28 23:47 ` Ulrich Drepper 2006-06-29 8:15 ` Arjan van de Ven 1 sibling, 2 replies; 26+ messages in thread From: Pavel Machek @ 2006-06-28 19:49 UTC (permalink / raw) To: Ulrich Drepper; +Cc: Arjan van de Ven, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-28 19:49 ` Pavel Machek @ 2006-06-28 20:05 ` Chase Venters 2006-06-28 23:47 ` Ulrich Drepper 1 sibling, 0 replies; 26+ messages in thread From: Chase Venters @ 2006-06-28 20:05 UTC (permalink / raw) To: Pavel Machek Cc: Ulrich Drepper, Arjan van de Ven, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-28 19:49 ` Pavel Machek 2006-06-28 20:05 ` Chase Venters @ 2006-06-28 23:47 ` Ulrich Drepper 2006-06-29 7:30 ` Pavel Machek 1 sibling, 1 reply; 26+ messages in thread From: Ulrich Drepper @ 2006-06-28 23:47 UTC (permalink / raw) To: Pavel Machek; +Cc: Arjan van de Ven, Jason Baron, akpm, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-28 23:47 ` Ulrich Drepper @ 2006-06-29 7:30 ` Pavel Machek 2006-06-29 11:58 ` Alan Cox 2006-06-30 3:49 ` Ulrich Drepper 0 siblings, 2 replies; 26+ messages in thread From: Pavel Machek @ 2006-06-29 7:30 UTC (permalink / raw) To: Ulrich Drepper; +Cc: Arjan van de Ven, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-29 7:30 ` Pavel Machek @ 2006-06-29 11:58 ` Alan Cox 2006-06-29 17:20 ` Pavel Machek 2006-06-30 3:49 ` Ulrich Drepper 1 sibling, 1 reply; 26+ messages in thread From: Alan Cox @ 2006-06-29 11:58 UTC (permalink / raw) To: Pavel Machek Cc: Ulrich Drepper, Arjan van de Ven, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-29 11:58 ` Alan Cox @ 2006-06-29 17:20 ` Pavel Machek 2006-06-29 21:00 ` Jason Baron 0 siblings, 1 reply; 26+ messages in thread From: Pavel Machek @ 2006-06-29 17:20 UTC (permalink / raw) To: Alan Cox Cc: Ulrich Drepper, Arjan van de Ven, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-29 17:20 ` Pavel Machek @ 2006-06-29 21:00 ` Jason Baron 2006-07-07 2:05 ` Jason Baron 0 siblings, 1 reply; 26+ messages in thread From: Jason Baron @ 2006-06-29 21:00 UTC (permalink / raw) To: Pavel Machek Cc: Alan Cox, Ulrich Drepper, Arjan van de Ven, akpm, linux-kernel 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; } ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-29 21:00 ` Jason Baron @ 2006-07-07 2:05 ` Jason Baron 0 siblings, 0 replies; 26+ messages in thread From: Jason Baron @ 2006-07-07 2:05 UTC (permalink / raw) To: akpm Cc: Alan Cox, Ulrich Drepper, Arjan van de Ven, linux-kernel, alex, paulus, tony.luck, paulus, ak, geert, zippel 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; } ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-29 7:30 ` Pavel Machek 2006-06-29 11:58 ` Alan Cox @ 2006-06-30 3:49 ` Ulrich Drepper 1 sibling, 0 replies; 26+ messages in thread From: Ulrich Drepper @ 2006-06-30 3:49 UTC (permalink / raw) To: Pavel Machek; +Cc: Arjan van de Ven, Jason Baron, akpm, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-28 16:43 ` Ulrich Drepper 2006-06-28 19:49 ` Pavel Machek @ 2006-06-29 8:15 ` Arjan van de Ven 2006-06-30 3:48 ` Ulrich Drepper 1 sibling, 1 reply; 26+ messages in thread From: Arjan van de Ven @ 2006-06-29 8:15 UTC (permalink / raw) To: Ulrich Drepper; +Cc: Pavel Machek, Jason Baron, akpm, linux-kernel 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? ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-29 8:15 ` Arjan van de Ven @ 2006-06-30 3:48 ` Ulrich Drepper 2006-06-30 8:35 ` Arjan van de Ven 0 siblings, 1 reply; 26+ messages in thread From: Ulrich Drepper @ 2006-06-30 3:48 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Pavel Machek, Jason Baron, akpm, linux-kernel 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. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-30 3:48 ` Ulrich Drepper @ 2006-06-30 8:35 ` Arjan van de Ven 2006-06-30 12:20 ` Alan Cox 0 siblings, 1 reply; 26+ messages in thread From: Arjan van de Ven @ 2006-06-30 8:35 UTC (permalink / raw) To: Ulrich Drepper; +Cc: Pavel Machek, Jason Baron, akpm, linux-kernel 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.... ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-06-30 8:35 ` Arjan van de Ven @ 2006-06-30 12:20 ` Alan Cox 0 siblings, 0 replies; 26+ messages in thread From: Alan Cox @ 2006-06-30 12:20 UTC (permalink / raw) To: Arjan van de Ven Cc: Ulrich Drepper, Pavel Machek, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <6qIEW-1Tx-23@gated-at.bofh.it>]
[parent not found: <6qIEW-1Tx-21@gated-at.bofh.it>]
[parent not found: <6qUwd-2Aq-9@gated-at.bofh.it>]
[parent not found: <6qUwd-2Aq-7@gated-at.bofh.it>]
[parent not found: <6qUFV-2N8-13@gated-at.bofh.it>]
[parent not found: <6qUFY-2N8-33@gated-at.bofh.it>]
[parent not found: <6rlmT-8op-37@gated-at.bofh.it>]
[parent not found: <6siwJ-3dC-5@gated-at.bofh.it>]
[parent not found: <6sLoY-4GV-31@gated-at.bofh.it>]
[parent not found: <6sZUS-V5-19@gated-at.bofh.it>]
[parent not found: <6tib4-2wA-3@gated-at.bofh.it>]
[parent not found: <6tmHL-Oq-5@gated-at.bofh.it>]
[parent not found: <6tpZ7-5Tj-13@gated-at.bofh.it>]
* Re: make PROT_WRITE imply PROT_READ [not found] ` <6tpZ7-5Tj-13@gated-at.bofh.it> @ 2006-07-01 13:19 ` Bodo Eggert 2006-07-02 9:56 ` Alan Cox 0 siblings, 1 reply; 26+ messages in thread From: Bodo Eggert @ 2006-07-01 13:19 UTC (permalink / raw) To: Alan Cox, Arjan van de Ven, Ulrich Drepper, Pavel Machek, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-07-01 13:19 ` Bodo Eggert @ 2006-07-02 9:56 ` Alan Cox 2006-07-02 22:04 ` Bodo Eggert 0 siblings, 1 reply; 26+ messages in thread From: Alan Cox @ 2006-07-02 9:56 UTC (permalink / raw) To: 7eggert Cc: Arjan van de Ven, Ulrich Drepper, Pavel Machek, Jason Baron, akpm, linux-kernel 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 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: make PROT_WRITE imply PROT_READ 2006-07-02 9:56 ` Alan Cox @ 2006-07-02 22:04 ` Bodo Eggert 0 siblings, 0 replies; 26+ messages in thread From: Bodo Eggert @ 2006-07-02 22:04 UTC (permalink / raw) To: Alan Cox Cc: 7eggert, Arjan van de Ven, Ulrich Drepper, Pavel Machek, Jason Baron, akpm, linux-kernel 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.' ^ permalink raw reply [flat|nested] 26+ messages in thread
* make PROT_WRITE imply PROT_READ @ 2006-06-22 17:33 Jason Baron 0 siblings, 0 replies; 26+ messages in thread From: Jason Baron @ 2006-06-22 17:33 UTC (permalink / raw) To: akpm; +Cc: linux-kernel 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; ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2006-07-07 2:14 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <fa.PuMM6IwflUYh1MWILO9rb6z4fvY@ifi.uio.no> 2006-06-23 1:24 ` make PROT_WRITE imply PROT_READ Robert Hancock 2006-06-23 13:39 ` Jason Baron 2006-06-23 14:06 ` Arjan van de Ven 2006-06-23 14:05 ` Jason Baron 2006-06-23 14:18 ` Arjan van de Ven 2006-06-24 18:45 ` Ulrich Drepper 2006-06-27 9:56 ` Pavel Machek 2006-06-27 12:18 ` Arjan van de Ven 2006-06-28 16:43 ` Ulrich Drepper 2006-06-28 19:49 ` Pavel Machek 2006-06-28 20:05 ` Chase Venters 2006-06-28 23:47 ` Ulrich Drepper 2006-06-29 7:30 ` Pavel Machek 2006-06-29 11:58 ` Alan Cox 2006-06-29 17:20 ` Pavel Machek 2006-06-29 21:00 ` Jason Baron 2006-07-07 2:05 ` Jason Baron 2006-06-30 3:49 ` Ulrich Drepper 2006-06-29 8:15 ` Arjan van de Ven 2006-06-30 3:48 ` Ulrich Drepper 2006-06-30 8:35 ` Arjan van de Ven 2006-06-30 12:20 ` Alan Cox [not found] <6qIEW-1Tx-23@gated-at.bofh.it> [not found] ` <6qIEW-1Tx-21@gated-at.bofh.it> [not found] ` <6qUwd-2Aq-9@gated-at.bofh.it> [not found] ` <6qUwd-2Aq-7@gated-at.bofh.it> [not found] ` <6qUFV-2N8-13@gated-at.bofh.it> [not found] ` <6qUFY-2N8-33@gated-at.bofh.it> [not found] ` <6rlmT-8op-37@gated-at.bofh.it> [not found] ` <6siwJ-3dC-5@gated-at.bofh.it> [not found] ` <6sLoY-4GV-31@gated-at.bofh.it> [not found] ` <6sZUS-V5-19@gated-at.bofh.it> [not found] ` <6tib4-2wA-3@gated-at.bofh.it> [not found] ` <6tmHL-Oq-5@gated-at.bofh.it> [not found] ` <6tpZ7-5Tj-13@gated-at.bofh.it> 2006-07-01 13:19 ` Bodo Eggert 2006-07-02 9:56 ` Alan Cox 2006-07-02 22:04 ` Bodo Eggert 2006-06-22 17:33 Jason Baron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).