[PATCH] make PROT_WRITE imply PROT_READ
authorJason Baron <jbaron@redhat.com>
Fri, 29 Sep 2006 08:58:58 +0000 (01:58 -0700)
committerLinus Torvalds <torvalds@g5.osdl.org>
Fri, 29 Sep 2006 16:18:05 +0000 (09:18 -0700)
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.

Additional discussion:

Several architectures, including x86, can not support write-only mappings.
The pte for x86 reserves a single bit for protection and its two states are
read only or read/write.  Thus, write only is not supported in h/w.

Currently, if i 'mmap' a page write-only, the first read attempt on that page
creates a page fault and will SEGV.  That check is enforced in
arch/blah/mm/fault.c.  However, if i first write that page it will fault in
and the pte will be set to read/write.  Thus, any subsequent reads to the page
will succeed.  It is this inconsistency in behavior that this patch is
attempting to address.  Furthermore, if the page is swapped out, and then
brought back the first read will also cause a SEGV.  Thus, any arbitrary read
on a page can potentially result in a SEGV.

According to the SuSv3 spec, "if the application requests only PROT_WRITE, the
implementation may also allow read access." Also as mentioned, some
archtectures, such as alpha, shown above already take the approach that i am
suggesting.

The counter-argument to this raised by Arjan, is that the kernel is enforcing
the write only mapping the best it can given the h/w limitations.  This is
true, however Alan Cox, and myself would argue that the inconsitency in
behavior, that is applications can sometimes work/sometimes fails is highly
undesireable.  If you read through the thread, i think people, came to an
agreement on the last patch i posted, as nobody has objected to it...

Signed-off-by: Jason Baron <jbaron@redhat.com>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: "Luck, Tony" <tony.luck@intel.com>
Cc: Hugh Dickins <hugh@veritas.com>
Cc: Roman Zippel <zippel@linux-m68k.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Acked-by: Andi Kleen <ak@muc.de>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Acked-by: Paul Mundt <lethal@linux-sh.org>
Cc: Kazumoto Kojima <kkojima@rr.iij4u.or.jp>
Cc: Ian Molton <spyro@f2s.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
arch/arm/mm/fault.c
arch/arm26/mm/fault.c
arch/i386/mm/fault.c
arch/ia64/mm/fault.c
arch/m68k/mm/fault.c
arch/powerpc/mm/fault.c
arch/ppc/mm/fault.c
arch/sh/mm/fault.c
arch/x86_64/mm/fault.c

index f0943d160ffeccc8d02ece3f82d023bc7c68970a..a5b33ff3924edc35c216f93232031f802651c4ad 100644 (file)
@@ -171,7 +171,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))
index 761938b56679453d350adb663187b4d9b52399ac..a7c4cc922095a184f8dc95f85f7332f857f26b12 100644 (file)
@@ -155,7 +155,7 @@ __do_page_fault(struct mm_struct *mm, unsigned long addr, unsigned int fsr,
         */
 good_area:
        if (READ_FAULT(fsr)) /* read? */
-               mask = VM_READ|VM_EXEC;
+               mask = VM_READ|VM_EXEC|VM_WRITE;
        else
                mask = VM_WRITE;
 
index 5e17a3f43b41ba89c15b17e7390f04dc730ada64..50d8617391dd09cfd86bde16882580c9cc8cebb1 100644 (file)
@@ -440,7 +440,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;
        }
 
index 14ef7cceb208bbd45031036cecadfda5e5527487..d8b1b4ac7f2609f5a4a1a5294d3721c53baae885 100644 (file)
@@ -146,9 +146,11 @@ ia64_do_page_fault (unsigned long address, unsigned long isr, struct pt_regs *re
 #              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;
index aec15270d334b74795fbfdf7ec214434f35a923f..5e2d87c10c872bbdf42d4dc6bcce1ccd7b72b579 100644 (file)
@@ -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;
        }
 
index 78a0d59903ee69d2b8ac96de20e18c17fd5f9206..77953f41d75406c200085e0bca8151857df9742f 100644 (file)
@@ -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;
        }
 
index 5cdfb71fcb078ca50f56059a907e0104ab452194..bc776beb3136f28bced9d3a4ab08d4efc039e5f8 100644 (file)
@@ -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;
        }
 
index c69fd603226aa80a20ba084aebd545e65c5142c3..507f28914706ba4c8e4768c06df5f79a5bc6f998 100644 (file)
@@ -69,7 +69,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;
        }
 
index 1a17b0733ab5d38ad81ccdf4263e2d58571d9e24..9ba54cc2b5f676e0cfe0ed4a6f6fee53daa7ddeb 100644 (file)
@@ -464,7 +464,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;
        }