From a9e8b391eb50bc107c79551596bd8b68710ead90 Mon Sep 17 00:00:00 2001 From: Jakub Jermar Date: Wed, 26 Apr 2006 17:56:23 +0000 Subject: [PATCH] Prevent race in as_area_send() by allowing the address space area to be created with AS_AREA_ATTR_PARTIAL attribute. --- generic/include/mm/as.h | 11 ++++-- generic/src/ddi/ddi.c | 2 +- generic/src/lib/elf.c | 2 +- generic/src/mm/as.c | 101 +++++++++++++++++++++++++++--------------------- generic/src/proc/task.c | 3 +- 5 files changed, 69 insertions(+), 50 deletions(-) diff --git a/generic/include/mm/as.h b/generic/include/mm/as.h index c63d101c2..ba2612cd5 100644 --- a/generic/include/mm/as.h +++ b/generic/include/mm/as.h @@ -59,6 +59,10 @@ #define AS_AREA_EXEC 4 #define AS_AREA_DEVICE 8 +/** Address space area attributes. */ +#define AS_AREA_ATTR_NONE 0 +#define AS_AREA_ATTR_PARTIAL 1 /* Not fully initialized area. */ + /** Address space area structure. * * Each as_area_t structure describes one contiguous area of virtual memory. @@ -66,7 +70,8 @@ */ struct as_area { SPINLOCK_DECLARE(lock); - int flags; + int flags; /**< Flags related to the memory represented by the address space area. */ + int attributes; /**< Attributes related to the address space area itself. */ count_t pages; /**< Size of this area in multiples of PAGE_SIZE. */ __address base; /**< Base address of this area. */ }; @@ -112,9 +117,9 @@ extern link_t inactive_as_with_asid_head; extern void as_init(void); extern as_t *as_create(int flags); -extern as_area_t *as_area_create(as_t *as, int flags, size_t size, __address base); +extern as_area_t *as_area_create(as_t *as, int flags, size_t size, __address base, int attrs); extern __address as_area_resize(as_t *as, __address address, size_t size, int flags); -int as_area_send(task_id_t id, __address base); +int as_area_send(task_id_t dst_id, __address base); extern void as_set_mapping(as_t *as, __address page, __address frame); extern int as_page_fault(__address page); extern void as_switch(as_t *old, as_t *new); diff --git a/generic/src/ddi/ddi.c b/generic/src/ddi/ddi.c index 0adf679f3..448c52600 100644 --- a/generic/src/ddi/ddi.c +++ b/generic/src/ddi/ddi.c @@ -92,7 +92,7 @@ static int ddi_physmem_map(task_id_t id, __address pf, __address vp, count_t pag flags = AS_AREA_DEVICE | AS_AREA_READ; if (writable) flags |= AS_AREA_WRITE; - if (!as_area_create(t->as, flags, pages * PAGE_SIZE, vp)) { + if (!as_area_create(t->as, flags, pages * PAGE_SIZE, vp, AS_AREA_ATTR_NONE)) { /* * The address space area could not have been created. * We report it using ENOMEM. diff --git a/generic/src/lib/elf.c b/generic/src/lib/elf.c index 83ae1b2a2..3f1f8df20 100644 --- a/generic/src/lib/elf.c +++ b/generic/src/lib/elf.c @@ -186,7 +186,7 @@ int load_segment(elf_segment_header_t *entry, elf_header_t *elf, as_t *as) } else /* Map identically original data */ segment = ((void *) elf) + entry->p_offset; - a = as_area_create(as, flags, entry->p_memsz, entry->p_vaddr); + a = as_area_create(as, flags, entry->p_memsz, entry->p_vaddr, AS_AREA_ATTR_NONE); if (!a) return EE_MEMORY; diff --git a/generic/src/mm/as.c b/generic/src/mm/as.c index 0ab3820b2..f3aac3d26 100644 --- a/generic/src/mm/as.c +++ b/generic/src/mm/as.c @@ -126,13 +126,14 @@ void as_free(as_t *as) * The created address space area is added to the target address space. * * @param as Target address space. - * @param flags Flags of the area. + * @param flags Flags of the area memory. * @param size Size of area. * @param base Base address of area. + * @param attrs Attributes of the area. * * @return Address space area on success or NULL on failure. */ -as_area_t *as_area_create(as_t *as, int flags, size_t size, __address base) +as_area_t *as_area_create(as_t *as, int flags, size_t size, __address base, int attrs) { ipl_t ipl; as_area_t *a; @@ -161,6 +162,7 @@ as_area_t *as_area_create(as_t *as, int flags, size_t size, __address base) spinlock_initialize(&a->lock, "as_area_lock"); a->flags = flags; + a->attributes = attrs; a->pages = SIZE2FRAMES(size); a->base = base; @@ -289,8 +291,8 @@ __address as_area_resize(as_t *as, __address address, size_t size, int flags) * for sharing group of pages. The source address * space area and any associated mapping is preserved. * - * @param id Task ID of the accepting task. - * @param base Base address of the source address space area. + * @param dst_id Task ID of the accepting task. + * @param src_base Base address of the source address space area. * * @return 0 on success or ENOENT if there is no such task or * if there is no such address space area, @@ -298,21 +300,21 @@ __address as_area_resize(as_t *as, __address address, size_t size, int flags) * ENOMEM if there was a problem in allocating destination * address space area. */ -int as_area_send(task_id_t id, __address base) +int as_area_send(task_id_t dst_id, __address src_base) { ipl_t ipl; task_t *t; count_t i; - as_t *as; + as_t *dst_as; __address dst_base; - int flags; - size_t size; - as_area_t *area; + int src_flags; + size_t src_size; + as_area_t *src_area, *dst_area; ipl = interrupts_disable(); spinlock_lock(&tasks_lock); - t = task_find_by_id(id); + t = task_find_by_id(dst_id); if (!NULL) { spinlock_unlock(&tasks_lock); interrupts_restore(ipl); @@ -322,10 +324,10 @@ int as_area_send(task_id_t id, __address base) spinlock_lock(&t->lock); spinlock_unlock(&tasks_lock); - as = t->as; + dst_as = t->as; dst_base = (__address) t->accept_arg.base; - if (as == AS) { + if (dst_as == AS) { /* * The two tasks share the entire address space. * Return error since there is no point in continuing. @@ -336,8 +338,8 @@ int as_area_send(task_id_t id, __address base) } spinlock_lock(&AS->lock); - area = find_area_and_lock(AS, base); - if (!area) { + src_area = find_area_and_lock(AS, src_base); + if (!src_area) { /* * Could not find the source address space area. */ @@ -346,13 +348,13 @@ int as_area_send(task_id_t id, __address base) interrupts_restore(ipl); return ENOENT; } - size = area->pages * PAGE_SIZE; - flags = area->flags; - spinlock_unlock(&area->lock); + src_size = src_area->pages * PAGE_SIZE; + src_flags = src_area->flags; + spinlock_unlock(&src_area->lock); spinlock_unlock(&AS->lock); - if ((t->accept_arg.task_id != TASK->taskid) || (t->accept_arg.size != size) || - (t->accept_arg.flags != flags)) { + if ((t->accept_arg.task_id != TASK->taskid) || (t->accept_arg.size != src_size) || + (t->accept_arg.flags != src_flags)) { /* * Discrepancy in either task ID, size or flags. */ @@ -362,9 +364,13 @@ int as_area_send(task_id_t id, __address base) } /* - * Create copy of the address space area. + * Create copy of the source address space area. + * The destination area is created with AS_AREA_ATTR_PARTIAL + * attribute set which prevents race condition with + * preliminary as_page_fault() calls. */ - if (!as_area_create(as, flags, size, dst_base)) { + dst_area = as_area_create(dst_as, src_flags, src_size, dst_base, AS_AREA_ATTR_PARTIAL); + if (!dst_area) { /* * Destination address space area could not be created. */ @@ -373,42 +379,30 @@ int as_area_send(task_id_t id, __address base) return ENOMEM; } - /* - * NOTE: we have just introduced a race condition. - * The destination task can try to attempt the newly - * created area before its mapping is copied from - * the source address space area. In result, frames - * can get lost. - * - * Currently, this race is not solved, but one of the - * possible solutions would be to sleep in as_page_fault() - * when this situation is detected. - */ - memsetb((__address) &t->accept_arg, sizeof(as_area_acptsnd_arg_t), 0); spinlock_unlock(&t->lock); /* * Avoid deadlock by first locking the address space with lower address. */ - if (as < AS) { - spinlock_lock(&as->lock); + if (dst_as < AS) { + spinlock_lock(&dst_as->lock); spinlock_lock(&AS->lock); } else { spinlock_lock(&AS->lock); - spinlock_lock(&as->lock); + spinlock_lock(&dst_as->lock); } - for (i = 0; i < SIZE2FRAMES(size); i++) { + for (i = 0; i < SIZE2FRAMES(src_size); i++) { pte_t *pte; __address frame; page_table_lock(AS, false); - pte = page_mapping_find(AS, base + i*PAGE_SIZE); + pte = page_mapping_find(AS, src_base + i*PAGE_SIZE); if (pte && PTE_VALID(pte)) { ASSERT(PTE_PRESENT(pte)); frame = PTE_GET_FRAME(pte); - if (!(flags & AS_AREA_DEVICE)) + if (!(src_flags & AS_AREA_DEVICE)) frame_reference_add(ADDR2PFN(frame)); page_table_unlock(AS, false); } else { @@ -416,13 +410,22 @@ int as_area_send(task_id_t id, __address base) continue; } - page_table_lock(as, false); - page_mapping_insert(as, dst_base + i*PAGE_SIZE, frame, area_flags_to_page_flags(flags)); - page_table_unlock(as, false); + page_table_lock(dst_as, false); + page_mapping_insert(dst_as, dst_base + i*PAGE_SIZE, frame, area_flags_to_page_flags(src_flags)); + page_table_unlock(dst_as, false); } + + /* + * Now the destination address space area has been + * fully initialized. Clear the AS_AREA_ATTR_PARTIAL + * attribute. + */ + spinlock_lock(&dst_area->lock); + dst_area->attributes &= ~AS_AREA_ATTR_PARTIAL; + spinlock_unlock(&dst_area->lock); spinlock_unlock(&AS->lock); - spinlock_unlock(&as->lock); + spinlock_unlock(&dst_as->lock); interrupts_restore(ipl); return 0; @@ -486,6 +489,16 @@ int as_page_fault(__address page) return 0; } + if (area->attributes & AS_AREA_ATTR_PARTIAL) { + /* + * The address space area is not fully initialized. + * Avoid possible race by returning error. + */ + spinlock_unlock(&area->lock); + spinlock_unlock(&AS->lock); + return 0; + } + ASSERT(!(area->flags & AS_AREA_DEVICE)); page_table_lock(AS, false); @@ -839,7 +852,7 @@ bool check_area_conflicts(as_t *as, __address va, size_t size, as_area_t *avoid_ /** Wrapper for as_area_create(). */ __native sys_as_area_create(__address address, size_t size, int flags) { - if (as_area_create(AS, flags, size, address)) + if (as_area_create(AS, flags, size, address, AS_AREA_ATTR_NONE)) return (__native) address; else return (__native) -1; diff --git a/generic/src/proc/task.c b/generic/src/proc/task.c index 6cdcd2e65..1329805d8 100644 --- a/generic/src/proc/task.c +++ b/generic/src/proc/task.c @@ -151,7 +151,8 @@ task_t * task_run_program(void *program_addr, char *name) /* * Create the data as_area. */ - a = as_area_create(as, AS_AREA_READ | AS_AREA_WRITE, LOADED_PROG_STACK_PAGES_NO*PAGE_SIZE, USTACK_ADDRESS); + a = as_area_create(as, AS_AREA_READ | AS_AREA_WRITE, LOADED_PROG_STACK_PAGES_NO*PAGE_SIZE, + USTACK_ADDRESS, AS_AREA_ATTR_NONE); t = thread_create(uinit, kernel_uarg, task, 0, "uinit"); ASSERT(t); -- 2.11.4.GIT