From 94daebd0c5625bb375a69228e4cee0434a42a8ce Mon Sep 17 00:00:00 2001 From: Bahadir Balban Date: Sat, 8 Nov 2008 10:18:35 +0200 Subject: [PATCH] Mapping precision fixes on l4_unmap and do_munmap() l4_unmap now returns -1 if given range was only partially unmapped. do_munmap() now only unmaps address ranges that have correspondence in the unmapped vmas. Trying to unmap regions with no correspondent vmas causes problems in corner cases, e.g. mm0 that tries to mmap its own address space during initialisation would unmap its whole address space and fail to execute. --- include/l4/glue/arm/memory.h | 4 ++-- src/api/space.c | 30 +++++++++++++----------- src/arch/arm/v5/mm.c | 34 +++++++++++++++++----------- tasks/mm0/include/mmap.h | 2 +- tasks/mm0/src/mmap.c | 5 ++-- tasks/mm0/src/munmap.c | 44 +++++++++++++++++++++++++++++++----- tasks/mm0/src/shm.c | 2 +- 7 files changed, 82 insertions(+), 39 deletions(-) diff --git a/include/l4/glue/arm/memory.h b/include/l4/glue/arm/memory.h index 0d8d389..4c27416 100644 --- a/include/l4/glue/arm/memory.h +++ b/include/l4/glue/arm/memory.h @@ -111,8 +111,8 @@ void add_mapping_pgd(unsigned int paddr, unsigned int vaddr, pgd_table_t *pgd); void add_mapping(unsigned int paddr, unsigned int vaddr, unsigned int size, unsigned int flags); -void remove_mapping(unsigned long vaddr); -void remove_mapping_pgd(unsigned long vaddr, pgd_table_t *pgd); +int remove_mapping(unsigned long vaddr); +int remove_mapping_pgd(unsigned long vaddr, pgd_table_t *pgd); void prealloc_phys_pagedesc(void); int check_mapping_pgd(unsigned long vaddr, unsigned long size, diff --git a/src/api/space.c b/src/api/space.c index 7564c21..c3248e7 100644 --- a/src/api/space.c +++ b/src/api/space.c @@ -40,26 +40,30 @@ found: return 0; } - +/* + * Unmaps given range from given task. If the complete range is unmapped + * sucessfully, returns 0. If part of the range was found to be already + * unmapped, returns -1. This is may or may not be an error. + */ int sys_unmap(syscall_context_t *regs) { - unsigned long virt = regs->r0; + unsigned long virtual = regs->r0; unsigned long npages = regs->r1; unsigned int tid = regs->r2; struct ktcb *target; + int ret, retval; - if (tid == current->tid) { /* The easiest case */ + if (tid == current->tid) target = current; - goto found; - } else /* else search the tcb from its hash list */ - if ((target = find_task(tid))) - goto found; - BUG(); - return -EINVAL; -found: - for (int i = 0; i < npages; i++) - remove_mapping_pgd(virt, target->pgd); + else if (!(target = find_task(tid))) + return -ESRCH; - return 0; + for (int i = 0; i < npages; i++) { + ret = remove_mapping_pgd(virtual + i * PAGE_SIZE, target->pgd); + if (ret) + retval = ret; + } + + return retval; } diff --git a/src/arch/arm/v5/mm.c b/src/arch/arm/v5/mm.c index db61a86..765ea31 100644 --- a/src/arch/arm/v5/mm.c +++ b/src/arch/arm/v5/mm.c @@ -274,31 +274,38 @@ int check_mapping(unsigned long vaddr, unsigned long size, } /* FIXME: Empty PMDs should be returned here !!! */ -void __remove_mapping(pmd_table_t *pmd, unsigned long vaddr) +int __remove_mapping(pmd_table_t *pmd, unsigned long vaddr) { pmd_t pmd_i = PMD_INDEX(vaddr); + int ret; switch (pmd->entry[pmd_i] & PMD_TYPE_MASK) { + case PMD_TYPE_FAULT: + ret = -1; + break; case PMD_TYPE_LARGE: pmd->entry[pmd_i] = 0; pmd->entry[pmd_i] |= PMD_TYPE_FAULT; + ret = 0; break; case PMD_TYPE_SMALL: pmd->entry[pmd_i] = 0; pmd->entry[pmd_i] |= PMD_TYPE_FAULT; + ret = 0; break; default: printk("Unknown page mapping in pmd. Assuming bug.\n"); BUG(); } - return; + return ret; } -void remove_mapping_pgd(unsigned long vaddr, pgd_table_t *pgd) +int remove_mapping_pgd(unsigned long vaddr, pgd_table_t *pgd) { pgd_t pgd_i = PGD_INDEX(vaddr); pmd_table_t *pmd; pmd_t pmd_i; + int ret; /* * Clean the cache to main memory before removing the mapping. Otherwise @@ -321,20 +328,19 @@ void remove_mapping_pgd(unsigned long vaddr, pgd_table_t *pgd) phys_to_virt((pgd->entry[pgd_i] & PGD_COARSE_ALIGN_MASK)); pmd_i = PMD_INDEX(vaddr); - __remove_mapping(pmd, vaddr); + ret = __remove_mapping(pmd, vaddr); break; case PGD_TYPE_FAULT: - dprintk("Attempting to remove fault mapping. " - "Assuming bug.\n", vaddr); - BUG(); + ret = -1; break; case PGD_TYPE_SECTION: - printk("Removing section mapping for 0x%lx", - vaddr); - pgd->entry[pgd_i] = 0; - pgd->entry[pgd_i] |= PGD_TYPE_FAULT; + printk("Removing section mapping for 0x%lx", + vaddr); + pgd->entry[pgd_i] = 0; + pgd->entry[pgd_i] |= PGD_TYPE_FAULT; + ret = 0; break; case PGD_TYPE_FINE: @@ -352,11 +358,13 @@ void remove_mapping_pgd(unsigned long vaddr, pgd_table_t *pgd) /* The tlb must be invalidated here because it might have cached the * old translation for this mapping. */ arm_invalidate_tlb(); + + return ret; } -void remove_mapping(unsigned long vaddr) +int remove_mapping(unsigned long vaddr) { - remove_mapping_pgd(vaddr, current->pgd); + return remove_mapping_pgd(vaddr, current->pgd); } /* diff --git a/tasks/mm0/include/mmap.h b/tasks/mm0/include/mmap.h index b8695d0..af663d1 100644 --- a/tasks/mm0/include/mmap.h +++ b/tasks/mm0/include/mmap.h @@ -24,7 +24,7 @@ struct vm_area *vma_new(unsigned long pfn_start, unsigned long npages, unsigned int flags, unsigned long file_offset); -int do_munmap(struct tcb *task, void *vaddr, unsigned long size); +int do_munmap(struct tcb *task, unsigned long vaddr, unsigned long size); void *do_mmap(struct vm_file *mapfile, unsigned long f_offset, struct tcb *t, unsigned long map_address, unsigned int flags, unsigned int pages); diff --git a/tasks/mm0/src/mmap.c b/tasks/mm0/src/mmap.c index 7bd2ac1..aee0380 100644 --- a/tasks/mm0/src/mmap.c +++ b/tasks/mm0/src/mmap.c @@ -146,10 +146,9 @@ void *do_mmap(struct vm_file *mapfile, unsigned long file_offset, struct tcb *task, unsigned long map_address, unsigned int flags, unsigned int npages) { - unsigned long map_pfn = __pfn(map_address); - unsigned long file_npages; - struct vm_area *new, *mapped; struct vm_obj_link *vmo_link, *vmo_link2; + unsigned long file_npages; + struct vm_area *new; int err; /* Set up devzero if none given */ diff --git a/tasks/mm0/src/munmap.c b/tasks/mm0/src/munmap.c index 5f22a3c..18b97cb 100644 --- a/tasks/mm0/src/munmap.c +++ b/tasks/mm0/src/munmap.c @@ -9,12 +9,14 @@ #include #include #include +#include /* This splits a vma, splitter region must be in the *middle* of original vma */ int vma_split(struct vm_area *vma, struct tcb *task, const unsigned long pfn_start, const unsigned long pfn_end) { struct vm_area *new; + unsigned long unmap_start = pfn_start, unmap_end = pfn_end; /* Allocate an uninitialised vma first */ if (!(new = vma_new(0, 0, 0, 0))) @@ -43,6 +45,10 @@ int vma_split(struct vm_area *vma, struct tcb *task, /* Add new one next to original vma */ list_add_tail(&new->list, &vma->list); + /* Unmap the removed portion */ + BUG_ON(l4_unmap((void *)__pfn_to_addr(unmap_start), + unmap_end - unmap_start, task->tid) < 0); + return 0; } @@ -50,20 +56,49 @@ int vma_split(struct vm_area *vma, struct tcb *task, int vma_shrink(struct vm_area *vma, struct tcb *task, const unsigned long pfn_start, const unsigned long pfn_end) { - unsigned long diff; + unsigned long diff, unmap_start, unmap_end; /* Shrink from the end */ if (vma->pfn_start < pfn_start) { BUG_ON(pfn_start >= vma->pfn_end); + unmap_start = pfn_start; + unmap_end = vma->pfn_end; vma->pfn_end = pfn_start; /* Shrink from the beginning */ } else if (vma->pfn_end > pfn_end) { BUG_ON(pfn_end <= vma->pfn_start); + unmap_start = vma->pfn_start; + unmap_end = pfn_end; diff = pfn_end - vma->pfn_start; vma->file_offset += diff; vma->pfn_start = pfn_end; - } else BUG(); + } else + BUG(); + + /* Unmap the shrinked portion */ + BUG_ON(l4_unmap((void *)__pfn_to_addr(unmap_start), + unmap_end - unmap_start, task->tid) < 0); + + return 0; +} + +/* Destroys a single vma from a task and unmaps its range from task space */ +int vma_destroy_single(struct tcb *task, struct vm_area *vma) +{ + int ret; + + /* Release all object links */ + if ((ret = vma_drop_merge_delete_all(vma)) < 0) + return ret; + + /* Unmap the whole vma address range */ + BUG_ON(l4_unmap((void *)__pfn_to_addr(vma->pfn_start), + vma->pfn_end - vma->pfn_start, task->tid) < 0); + + /* Unlink and delete vma */ + list_del(&vma->list); + kfree(vma); return 0; } @@ -84,7 +119,7 @@ int vma_unmap(struct vm_area *vma, struct tcb *task, return vma_shrink(vma, task, pfn_start, pfn_end); /* Destroy needed? */ else if ((vma->pfn_start >= pfn_start) && (vma->pfn_end <= pfn_end)) - return vma_drop_merge_delete_all(vma); + return vma_destroy_single(task, vma); else BUG(); @@ -158,9 +193,6 @@ int do_munmap(struct tcb *task, unsigned long vaddr, unsigned long npages) } } - /* Unmap those pages from the task address space */ - l4_unmap(vaddr, npages, task->tid); - return 0; } diff --git a/tasks/mm0/src/shm.c b/tasks/mm0/src/shm.c index 1ec2b0a..1f05550 100644 --- a/tasks/mm0/src/shm.c +++ b/tasks/mm0/src/shm.c @@ -146,7 +146,7 @@ int do_shmdt(struct tcb *task, struct vm_file *shm) int err; if ((err = do_munmap(task, - shm_file_to_desc(shm)->shm_addr, + (unsigned long)shm_file_to_desc(shm)->shm_addr, shm_file_to_desc(shm)->npages)) < 0) return err;