From d39ffc6acdc8a29b1729a317bd98f918874b66fc Mon Sep 17 00:00:00 2001 From: Bahadir Balban Date: Fri, 9 Oct 2009 16:45:10 +0300 Subject: [PATCH] Fixed utcb updating issue that was a significant burden. Any thread that touches a utcb inside the kernel now properly checks whether the utcb is mapped on its owner, and whether the mapped physical address matches that of the current thread's tables. If not the tables are updated. This way, even though page tables become incoherent on utcb address change situations (such as fork() exit(), execve()) they get updated as they are referenced. Since mappings are added only conditionally, caches are flushed only when an update is necessary. --- conts/posix/test0/src/fileio.c | 3 ++ include/l4/generic/space.h | 2 ++ include/l4/glue/arm/syscall.h | 4 +++ src/arch/arm/exception.c | 3 +- src/generic/space.c | 33 +++++++++++++++------- src/generic/tcb.c | 50 +++++++++++++++++++++++++++++++++- 6 files changed, 83 insertions(+), 12 deletions(-) diff --git a/conts/posix/test0/src/fileio.c b/conts/posix/test0/src/fileio.c index 7d9c3c0..4e5579e 100644 --- a/conts/posix/test0/src/fileio.c +++ b/conts/posix/test0/src/fileio.c @@ -44,6 +44,9 @@ int fileio(void) if ((int)(cnt = read(fd, buf, strlen(str))) < 0) { test_printf("READ: %d", errno); goto out_err; + } else if (cnt != strlen(str)) { + test_printf("%d: Read: %d bytes from file.\n", tid, cnt); + goto out_err; } test_printf("%d: Read: %d bytes from file.\n", tid, cnt); diff --git a/include/l4/generic/space.h b/include/l4/generic/space.h index 51c7efa..73d9c7f 100644 --- a/include/l4/generic/space.h +++ b/include/l4/generic/space.h @@ -57,6 +57,8 @@ void address_space_reference_unlock(); void init_address_space_list(struct address_space_list *space_list); int check_access(unsigned long vaddr, unsigned long size, unsigned int flags, int page_in); +int check_access_task(unsigned long vaddr, unsigned long size, + unsigned int flags, int page_in, struct ktcb *task); #endif #endif /* __SPACE_H__ */ diff --git a/include/l4/glue/arm/syscall.h b/include/l4/glue/arm/syscall.h index 169d7f1..9d16fd9 100644 --- a/include/l4/glue/arm/syscall.h +++ b/include/l4/glue/arm/syscall.h @@ -56,6 +56,10 @@ typedef struct msg_regs { * These references are valid only when they have been explicitly set * by a kernel entry point, e.g. a system call, a data abort handler * that imitates a page fault ipc etc. + * + * Second note: + * _If_ these refer to real utcb's in the future, make sure to have + * utcb_map_lazily() check so that they're safe accesses. */ #define KTCB_REF_ARG0(ktcb) (&(ktcb)->syscall_regs->r0) #define KTCB_REF_MR0(ktcb) (&(ktcb)->syscall_regs->MR0_REGISTER) diff --git a/src/arch/arm/exception.c b/src/arch/arm/exception.c index 700bcc4..c8b15b9 100644 --- a/src/arch/arm/exception.c +++ b/src/arch/arm/exception.c @@ -112,7 +112,8 @@ void fault_ipc_to_pager(u32 faulty_pc, u32 fsr, u32 far) * on mapping the buffer. Remember that if a task maps its own user buffer to itself * this way, the kernel can access it, since it shares that task's page table. */ -int pager_pagein_request(unsigned long addr, unsigned long size, unsigned int flags) +int pager_pagein_request(unsigned long addr, unsigned long size, + unsigned int flags) { u32 abort = 0; unsigned long npages = __pfn(align_up(size, PAGE_SIZE)); diff --git a/src/generic/space.c b/src/generic/space.c index ed81dff..ebf739e 100644 --- a/src/generic/space.c +++ b/src/generic/space.c @@ -141,13 +141,8 @@ struct address_space *address_space_create(struct address_space *orig) * check if a pte is locked before going forward with a request. */ -/* - * Checks whether the given user address is a valid userspace address. - * If so, whether it is currently mapped into its own address space. - * If its not mapped-in, it generates a page-in request to the thread's - * pager. If fault hasn't cleared, aborts. - */ -int check_access(unsigned long vaddr, unsigned long size, unsigned int flags, int page_in) +int check_access_task(unsigned long vaddr, unsigned long size, + unsigned int flags, int page_in, struct ktcb *task) { int err; unsigned long start, end, mapsize; @@ -162,9 +157,15 @@ int check_access(unsigned long vaddr, unsigned long size, unsigned int flags, in mapsize = end - start; /* Check if the address is mapped with given flags */ - if (!check_mapping(start, mapsize, flags)) { - /* Is a page in requested? */ - if (page_in) { + if (!check_mapping_pgd(start, mapsize, flags, TASK_PGD(task))) { + /* + * Is a page in requested? + * + * Only allow page-in from current task, + * since on-behalf-of type of ipc is + * complicated and we don't do it. + */ + if (page_in && task == current) { /* Ask pager if paging in is possible */ if((err = pager_pagein_request(start, mapsize, flags)) < 0) @@ -176,3 +177,15 @@ int check_access(unsigned long vaddr, unsigned long size, unsigned int flags, in return 0; } +/* + * Checks whether the given user address is a valid userspace address. + * If so, whether it is currently mapped into its own address space. + * If its not mapped-in, it generates a page-in request to the thread's + * pager. If fault hasn't cleared, aborts. + */ +int check_access(unsigned long vaddr, unsigned long size, + unsigned int flags, int page_in) +{ + return check_access_task(vaddr, size, flags, page_in, current); +} + diff --git a/src/generic/tcb.c b/src/generic/tcb.c index 44eaca7..67ed17d 100644 --- a/src/generic/tcb.c +++ b/src/generic/tcb.c @@ -1,7 +1,7 @@ /* * Some ktcb related data * - * Copyright (C) 2007 Bahadir Balban + * Copyright (C) 2007 - 2009 Bahadir Balban */ #include #include @@ -179,6 +179,53 @@ int tcb_check_and_lazy_map_utcb(struct ktcb *task) BUG_ON(!task->utcb_address); + /* + * There you go: + * + * if task == current && not mapped, page-in, if not return -EFAULT + * if task != current && not mapped, page-in to task, return -EFAULT + * if task != current && task mapped, but mapped != current mapped, map it, return 0 + * if task != current && task mapped, but mapped == current mapped, return 0 + */ + + if (current == task) { + /* Check own utcb, if not there, page it in */ + if ((ret = check_access(task->utcb_address, UTCB_SIZE, + MAP_SVC_RW_FLAGS, 1)) < 0) + return -EFAULT; + else + return 0; + } else { + /* Check another's utcb, but don't try to map in */ + if ((ret = check_access_task(task->utcb_address, + UTCB_SIZE, + MAP_SVC_RW_FLAGS, 0, + task)) < 0) { + return -EFAULT; + } else { + /* + * Task has it mapped, map it to self + * unless they're identical + */ + if ((phys = + virt_to_phys_by_pgd(task->utcb_address, + TASK_PGD(task))) != + virt_to_phys_by_pgd(task->utcb_address, + TASK_PGD(current))) + /* + * We have none or an old reference. + * Update it with privileged flags, + * so that only kernel can access. + */ + printk("%s: Caught old utcb mapping.\n", __FUNCTION__); + add_mapping_pgd(phys, task->utcb_address, + page_align_up(UTCB_SIZE), + MAP_SVC_RW_FLAGS, + TASK_PGD(current)); + BUG_ON(!phys); + } + } +#if 0 /* * FIXME: * @@ -209,6 +256,7 @@ int tcb_check_and_lazy_map_utcb(struct ktcb *task) } } } +#endif return 0; }