| Message ID | 1407003407-31219-2-git-send-email-nathan_lynch@mentor.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Aug 02, 2014 at 07:16:38PM +0100, Nathan Lynch wrote: > _install_special_mapping allows the VMA to be identifed in > /proc/pid/maps without the use of arch_vma_name, providing a > slight net reduction in object size: > > text data bss dec hex filename > 2996 96 144 3236 ca4 arch/arm/kernel/process.o (before) > 2956 104 144 3204 c84 arch/arm/kernel/process.o (after) > > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > --- > arch/arm/kernel/process.c | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > index 81ef686a91ca..46fbbb3701a0 100644 > --- a/arch/arm/kernel/process.c > +++ b/arch/arm/kernel/process.c > @@ -472,19 +472,23 @@ int in_gate_area_no_mm(unsigned long addr) > > const char *arch_vma_name(struct vm_area_struct *vma) > { > - return is_gate_vma(vma) ? "[vectors]" : > - (vma->vm_mm && vma->vm_start == vma->vm_mm->context.sigpage) ? > - "[sigpage]" : NULL; > + return is_gate_vma(vma) ? "[vectors]" : NULL; > } Why do you need this function? I just removed it for arm64 and I think x86 has done the same. Will
On 08/04/2014 07:46 AM, Will Deacon wrote: > On Sat, Aug 02, 2014 at 07:16:38PM +0100, Nathan Lynch wrote: >> _install_special_mapping allows the VMA to be identifed in >> /proc/pid/maps without the use of arch_vma_name, providing a >> slight net reduction in object size: >> >> text data bss dec hex filename >> 2996 96 144 3236 ca4 arch/arm/kernel/process.o (before) >> 2956 104 144 3204 c84 arch/arm/kernel/process.o (after) >> >> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> >> Reviewed-by: Kees Cook <keescook@chromium.org> >> --- >> arch/arm/kernel/process.c | 24 ++++++++++++++++-------- >> 1 file changed, 16 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c >> index 81ef686a91ca..46fbbb3701a0 100644 >> --- a/arch/arm/kernel/process.c >> +++ b/arch/arm/kernel/process.c >> @@ -472,19 +472,23 @@ int in_gate_area_no_mm(unsigned long addr) >> >> const char *arch_vma_name(struct vm_area_struct *vma) >> { >> - return is_gate_vma(vma) ? "[vectors]" : >> - (vma->vm_mm && vma->vm_start == vma->vm_mm->context.sigpage) ? >> - "[sigpage]" : NULL; >> + return is_gate_vma(vma) ? "[vectors]" : NULL; >> } > > Why do you need this function? I just removed it for arm64 and I think x86 > has done the same. I think arch_vma_name is still needed for arm as long as the vectors page is not installed using _install_special_mapping. I'm honestly not sure whether moving the vectors page to that API would be appropriate, but it would be a larger undertaking.
On 08/04/2014 08:12 AM, Nathan Lynch wrote: > On 08/04/2014 07:46 AM, Will Deacon wrote: >> On Sat, Aug 02, 2014 at 07:16:38PM +0100, Nathan Lynch wrote: >>> _install_special_mapping allows the VMA to be identifed in >>> /proc/pid/maps without the use of arch_vma_name, providing a >>> slight net reduction in object size: >>> >>> text data bss dec hex filename >>> 2996 96 144 3236 ca4 arch/arm/kernel/process.o (before) >>> 2956 104 144 3204 c84 arch/arm/kernel/process.o (after) >>> >>> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> >>> Reviewed-by: Kees Cook <keescook@chromium.org> >>> --- >>> arch/arm/kernel/process.c | 24 ++++++++++++++++-------- >>> 1 file changed, 16 insertions(+), 8 deletions(-) >>> >>> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c >>> index 81ef686a91ca..46fbbb3701a0 100644 >>> --- a/arch/arm/kernel/process.c >>> +++ b/arch/arm/kernel/process.c >>> @@ -472,19 +472,23 @@ int in_gate_area_no_mm(unsigned long addr) >>> >>> const char *arch_vma_name(struct vm_area_struct *vma) >>> { >>> - return is_gate_vma(vma) ? "[vectors]" : >>> - (vma->vm_mm && vma->vm_start == vma->vm_mm->context.sigpage) ? >>> - "[sigpage]" : NULL; >>> + return is_gate_vma(vma) ? "[vectors]" : NULL; >>> } >> >> Why do you need this function? I just removed it for arm64 and I think x86 >> has done the same. > > I think arch_vma_name is still needed for arm as long as the vectors > page is not installed using _install_special_mapping. Actually... if we give arm's gate_vma a vm_ops with a ->name() routine that returns "[vectors]" we could get rid of arch_vma_name, I think. This seems to be what x86_64 does.
On Mon, Aug 04, 2014 at 03:11:29PM +0100, Nathan Lynch wrote: > On 08/04/2014 08:12 AM, Nathan Lynch wrote: > > On 08/04/2014 07:46 AM, Will Deacon wrote: > >> On Sat, Aug 02, 2014 at 07:16:38PM +0100, Nathan Lynch wrote: > >>> _install_special_mapping allows the VMA to be identifed in > >>> /proc/pid/maps without the use of arch_vma_name, providing a > >>> slight net reduction in object size: > >>> > >>> text data bss dec hex filename > >>> 2996 96 144 3236 ca4 arch/arm/kernel/process.o (before) > >>> 2956 104 144 3204 c84 arch/arm/kernel/process.o (after) > >>> > >>> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > >>> Reviewed-by: Kees Cook <keescook@chromium.org> > >>> --- > >>> arch/arm/kernel/process.c | 24 ++++++++++++++++-------- > >>> 1 file changed, 16 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > >>> index 81ef686a91ca..46fbbb3701a0 100644 > >>> --- a/arch/arm/kernel/process.c > >>> +++ b/arch/arm/kernel/process.c > >>> @@ -472,19 +472,23 @@ int in_gate_area_no_mm(unsigned long addr) > >>> > >>> const char *arch_vma_name(struct vm_area_struct *vma) > >>> { > >>> - return is_gate_vma(vma) ? "[vectors]" : > >>> - (vma->vm_mm && vma->vm_start == vma->vm_mm->context.sigpage) ? > >>> - "[sigpage]" : NULL; > >>> + return is_gate_vma(vma) ? "[vectors]" : NULL; > >>> } > >> > >> Why do you need this function? I just removed it for arm64 and I think x86 > >> has done the same. > > > > I think arch_vma_name is still needed for arm as long as the vectors > > page is not installed using _install_special_mapping. > > Actually... if we give arm's gate_vma a vm_ops with a ->name() routine > that returns "[vectors]" we could get rid of arch_vma_name, I think. > This seems to be what x86_64 does. To be honest, I forgot about the vectors page, but giving it a name sounds like a sensible idea. Will
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 81ef686a91ca..46fbbb3701a0 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -472,19 +472,23 @@ int in_gate_area_no_mm(unsigned long addr) const char *arch_vma_name(struct vm_area_struct *vma) { - return is_gate_vma(vma) ? "[vectors]" : - (vma->vm_mm && vma->vm_start == vma->vm_mm->context.sigpage) ? - "[sigpage]" : NULL; + return is_gate_vma(vma) ? "[vectors]" : NULL; } static struct page *signal_page; extern struct page *get_signal_page(void); +static const struct vm_special_mapping sigpage_mapping = { + .name = "[sigpage]", + .pages = &signal_page, +}; + int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) { struct mm_struct *mm = current->mm; + struct vm_area_struct *vma; unsigned long addr; - int ret; + int ret = 0; if (!signal_page) signal_page = get_signal_page(); @@ -498,12 +502,16 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) goto up_fail; } - ret = install_special_mapping(mm, addr, PAGE_SIZE, + vma = _install_special_mapping(mm, addr, PAGE_SIZE, VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC, - &signal_page); + &sigpage_mapping); + + if (IS_ERR(vma)) { + ret = PTR_ERR(vma); + goto up_fail; + } - if (ret == 0) - mm->context.sigpage = addr; + mm->context.sigpage = addr; up_fail: up_write(&mm->mmap_sem);