Skip to content

Commit cb22f24

Browse files
danglin44hdeller
authored andcommitted
parisc: Update comments in make_insert_tlb
The following testcase exposed a problem with our read access checks in get_user() and raw_copy_from_user(): #include <stdint.h> #include <stddef.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> #include <errno.h> #include <sys/mman.h> #include <sys/types.h> int main(int argc, char **argv) { unsigned long page_size = sysconf(_SC_PAGESIZE); char *p = malloc(3 * page_size); char *p_aligned; /* initialize memory region. If not initialized, write syscall below will correctly return EFAULT. */ if (1) memset(p, 'X', 3 * page_size); p_aligned = (char *) ((((uintptr_t) p) + (2*page_size - 1)) & ~(page_size - 1)); /* Drop PROT_READ protection. Kernel and userspace should fault when accessing that memory region */ mprotect(p_aligned, page_size, PROT_NONE); /* the following write() should return EFAULT, since PROT_READ was dropped by previous mprotect() */ int ret = write(2, p_aligned, 1); if (!ret || errno != EFAULT) printf("\n FAILURE: write() did not returned expected EFAULT value\n"); return 0; } Because of the way _PAGE_READ is handled, kernel code never generates a read access fault when it access a page as the kernel privilege level is always less than PL1 in the PTE. This patch reworks the comments in the make_insert_tlb macro to try to make this clearer. Signed-off-by: John David Anglin <[email protected]> Signed-off-by: Helge Deller <[email protected]> Cc: [email protected] # v5.12+
1 parent 305ab0a commit cb22f24

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

arch/parisc/kernel/entry.S

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,12 @@
499499
* this happens is quite subtle, read below */
500500
.macro make_insert_tlb spc,pte,prot,tmp
501501
space_to_prot \spc \prot /* create prot id from space */
502+
503+
#if _PAGE_SPECIAL_BIT == _PAGE_DMB_BIT
504+
/* need to drop DMB bit, as it's used as SPECIAL flag */
505+
depi 0,_PAGE_SPECIAL_BIT,1,\pte
506+
#endif
507+
502508
/* The following is the real subtlety. This is depositing
503509
* T <-> _PAGE_REFTRAP
504510
* D <-> _PAGE_DIRTY
@@ -511,17 +517,18 @@
511517
* Finally, _PAGE_READ goes in the top bit of PL1 (so we
512518
* trigger an access rights trap in user space if the user
513519
* tries to read an unreadable page */
514-
#if _PAGE_SPECIAL_BIT == _PAGE_DMB_BIT
515-
/* need to drop DMB bit, as it's used as SPECIAL flag */
516-
depi 0,_PAGE_SPECIAL_BIT,1,\pte
517-
#endif
518520
depd \pte,8,7,\prot
519521

520522
/* PAGE_USER indicates the page can be read with user privileges,
521523
* so deposit X1|11 to PL1|PL2 (remember the upper bit of PL1
522-
* contains _PAGE_READ) */
524+
* contains _PAGE_READ). While the kernel can't directly write
525+
* user pages which have _PAGE_WRITE zero, it can read pages
526+
* which have _PAGE_READ zero (PL <= PL1). Thus, the kernel
527+
* exception fault handler doesn't trigger when reading pages
528+
* that aren't user read accessible */
523529
extrd,u,*= \pte,_PAGE_USER_BIT+32,1,%r0
524530
depdi 7,11,3,\prot
531+
525532
/* If we're a gateway page, drop PL2 back to zero for promotion
526533
* to kernel privilege (so we can execute the page as kernel).
527534
* Any privilege promotion page always denys read and write */

0 commit comments

Comments
 (0)