Re: [PATCH 24/25] arm64:ilp32: add vdso-ilp32 and use for signal return
From: Yury Norov
Date: Wed Apr 13 2016 - 11:56:14 EST
Hi Bamvor,
On Wed, Apr 13, 2016 at 05:19:28PM +0800, Zhangjian (Bamvor) wrote:
> Hi, Yury and Philipp
>
> There is a small fix for this patch. Othervise our tools of living
> patch could not work.
>
> Regards
>
> Bamvor
>
> From e05770efca9f040e0039a4a9c4e0d7d3b2bd13e8 Mon Sep 17 00:00:00 2001
> From: Bamvor Jian Zhang <bamvor.zhangjian@xxxxxxxxxx>
> Date: Wed, 13 Apr 2016 15:46:28 +0800
> Subject: [PATCH] arm64: ilp32: do not check vdso-ilp32-offsets.h when ILP32
> disabled
>
> vdso-ilp32-offsets.h is the dependency of all the arm64-obj-y. And
> it does not exist when CONFIG_ARM64_ILP32 is disable which lead to
> all the arm64-obj-y are re-built unnecessarily.
>
> Such rebuild may confuse the sofware(e.g. tools of living patch)
> which need to know exactly which file(s) is(are) updated.
>
> This patch fix this issue by adding the config checker.
It looks reasonable and correct.
I'll apply your patch in next submission (if it will be needed),
otherwise I think, Arnd may apply it.
Though, I don't understand much, what 'tools of living patch' means.
Could you explain in details what you do, and what goes wrong?
Nevertheless, thank you for your attention to this patchset.
Yury.
>
> Signed-off-by: Bamvor Jian Zhang <bamvor.zhangjian@xxxxxxxxxx>
> ---
> arch/arm64/kernel/Makefile | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index b43ff12..0f27a10 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -60,5 +60,7 @@ $(call objectify,$(arm64-obj-y)): $(obj)/vdso/vdso-offsets.h
> $(obj)/vdso/vdso-offsets.h: $(obj)/vdso
>
> # vDSO - this must be built first to generate the symbol offsets
> +ifeq ($(CONFIG_ARM64_ILP32),y)
> $(call objectify,$(arm64-obj-y)): $(obj)/vdso-ilp32/vdso-ilp32-offsets.h
> $(obj)/vdso-ilp32/vdso-ilp32-offsets.h: $(obj)/vdso-ilp32
> +endif
> --
> 1.8.4.5
>
> On 2016/4/6 6:08, Yury Norov wrote:
> >From: Philipp Tomsich <philipp.tomsich@xxxxxxxxxxxxxxxxxxxxx>
> >
> >ILP32 VDSO exports next symbols:
> > __kernel_rt_sigreturn;
> > __kernel_gettimeofday;
> > __kernel_clock_gettime;
> > __kernel_clock_getres;
> >
> >What shared object to use, kernel selects depending on result of
> >is_ilp32_compat_task() in arch/arm64/kernel/vdso.c, so it substitutes
> >correct pages and spec.
> >
> >Adjusted to move the move data page before code pages in sync with
> >commit 601255ae3c98fdeeee3a8bb4696425e4f868b4f1
> >
> >Signed-off-by: Philipp Tomsich <philipp.tomsich@xxxxxxxxxxxxxxxxxxxxx>
> >Signed-off-by: Christoph Muellner <christoph.muellner@xxxxxxxxxxxxxxxxxxxxx>
> >Signed-off-by: Yury Norov <ynorov@xxxxxxxxxxxxxxxxxx>
> >---
> > arch/arm64/include/asm/vdso.h | 6 ++
> > arch/arm64/kernel/Makefile | 5 ++
> > arch/arm64/kernel/signal.c | 2 +
> > arch/arm64/kernel/vdso-ilp32/.gitignore | 2 +
> > arch/arm64/kernel/vdso-ilp32/Makefile | 72 ++++++++++++++++++++
> > arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S | 33 ++++++++++
> > arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S | 95 +++++++++++++++++++++++++++
> > arch/arm64/kernel/vdso.c | 61 ++++++++++++++---
> > 8 files changed, 266 insertions(+), 10 deletions(-)
> > create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore
> > create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile
> > create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
> > create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
> >
> >diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h
> >index 839ce00..649a9a4 100644
> >--- a/arch/arm64/include/asm/vdso.h
> >+++ b/arch/arm64/include/asm/vdso.h
> >@@ -29,6 +29,12 @@
> >
> > #include <generated/vdso-offsets.h>
> >
> >+#ifdef CONFIG_ARM64_ILP32
> >+#include <generated/vdso-ilp32-offsets.h>
> >+#else
> >+#define vdso_offset_sigtramp_ilp32
> >+#endif
> >+
> > #define VDSO_SYMBOL(base, name) \
> > ({ \
> > (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \
> >diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> >index 09e4373..b43ff12 100644
> >--- a/arch/arm64/kernel/Makefile
> >+++ b/arch/arm64/kernel/Makefile
> >@@ -50,6 +50,7 @@ arm64-obj-$(CONFIG_PARAVIRT) += paravirt.o
> > arm64-obj-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
> >
> > obj-y += $(arm64-obj-y) vdso/
> >+obj-$(CONFIG_ARM64_ILP32) += vdso-ilp32/
> > obj-m += $(arm64-obj-m)
> > head-y := head.o
> > extra-y += $(head-y) vmlinux.lds
> >@@ -57,3 +58,7 @@ extra-y += $(head-y) vmlinux.lds
> > # vDSO - this must be built first to generate the symbol offsets
> > $(call objectify,$(arm64-obj-y)): $(obj)/vdso/vdso-offsets.h
> > $(obj)/vdso/vdso-offsets.h: $(obj)/vdso
> >+
> >+# vDSO - this must be built first to generate the symbol offsets
> >+$(call objectify,$(arm64-obj-y)): $(obj)/vdso-ilp32/vdso-ilp32-offsets.h
> >+$(obj)/vdso-ilp32/vdso-ilp32-offsets.h: $(obj)/vdso-ilp32
> >diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> >index 45bcd96..933cdcf 100644
> >--- a/arch/arm64/kernel/signal.c
> >+++ b/arch/arm64/kernel/signal.c
> >@@ -264,6 +264,8 @@ void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> >
> > if (ka->sa.sa_flags & SA_RESTORER)
> > sigtramp = ka->sa.sa_restorer;
> >+ else if (is_ilp32_compat_task())
> >+ sigtramp = VDSO_SYMBOL(current->mm->context.vdso, sigtramp_ilp32);
> > else
> > sigtramp = VDSO_SYMBOL(current->mm->context.vdso, sigtramp);
> >
> >diff --git a/arch/arm64/kernel/vdso-ilp32/.gitignore b/arch/arm64/kernel/vdso-ilp32/.gitignore
> >new file mode 100644
> >index 0000000..61806c3
> >--- /dev/null
> >+++ b/arch/arm64/kernel/vdso-ilp32/.gitignore
> >@@ -0,0 +1,2 @@
> >+vdso-ilp32.lds
> >+vdso-ilp32-offsets.h
> >diff --git a/arch/arm64/kernel/vdso-ilp32/Makefile b/arch/arm64/kernel/vdso-ilp32/Makefile
> >new file mode 100644
> >index 0000000..c8f5472
> >--- /dev/null
> >+++ b/arch/arm64/kernel/vdso-ilp32/Makefile
> >@@ -0,0 +1,72 @@
> >+#
> >+# Building a vDSO image for AArch64.
> >+#
> >+# Author: Will Deacon <will.deacon@xxxxxxx>
> >+# Heavily based on the vDSO Makefiles for other archs.
> >+#
> >+
> >+obj-ilp32-vdso := gettimeofday-ilp32.o note-ilp32.o sigreturn-ilp32.o
> >+
> >+# Build rules
> >+targets := $(obj-ilp32-vdso) vdso-ilp32.so vdso-ilp32.so.dbg
> >+obj-ilp32-vdso := $(addprefix $(obj)/, $(obj-ilp32-vdso))
> >+
> >+ccflags-y := -shared -fno-common -fno-builtin
> >+ccflags-y += -nostdlib -Wl,-soname=linux-ilp32-vdso.so.1 \
> >+ $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)
> >+
> >+obj-y += vdso-ilp32.o
> >+extra-y += vdso-ilp32.lds vdso-ilp32-offsets.h
> >+CPPFLAGS_vdso-ilp32.lds += -P -C -U$(ARCH) -mabi=ilp32
> >+
> >+# Force dependency (incbin is bad)
> >+$(obj)/vdso-ilp32.o : $(obj)/vdso-ilp32.so
> >+
> >+# Link rule for the .so file, .lds has to be first
> >+$(obj)/vdso-ilp32.so.dbg: $(src)/vdso-ilp32.lds $(obj-ilp32-vdso)
> >+ $(call if_changed,vdso-ilp32ld)
> >+
> >+# Strip rule for the .so file
> >+$(obj)/%.so: OBJCOPYFLAGS := -S
> >+$(obj)/%.so: $(obj)/%.so.dbg FORCE
> >+ $(call if_changed,objcopy)
> >+
> >+# Generate VDSO offsets using helper script
> >+gen-vdsosym := $(srctree)/$(src)/../vdso/gen_vdso_offsets.sh
> >+quiet_cmd_vdsosym = VDSOSYM $@
> >+define cmd_vdsosym
> >+ $(NM) $< | $(gen-vdsosym) | LC_ALL=C sort > $@ && \
> >+ cp $@ include/generated/
> >+endef
> >+
> >+$(obj)/vdso-ilp32-offsets.h: $(obj)/vdso-ilp32.so.dbg FORCE
> >+ $(call if_changed,vdsosym)
> >+
> >+# Assembly rules for the .S files
> >+#$(obj-ilp32-vdso): %.o: $(src)/../vdso/$(subst -ilp32,,%.S)
> >+# $(call if_changed_dep,vdso-ilp32as)
> >+
> >+$(obj)/gettimeofday-ilp32.o: $(src)/../vdso/gettimeofday.S
> >+ $(call if_changed_dep,vdso-ilp32as)
> >+
> >+$(obj)/note-ilp32.o: $(src)/../vdso/note.S
> >+ $(call if_changed_dep,vdso-ilp32as)
> >+
> >+$(obj)/sigreturn-ilp32.o: $(src)/../vdso/sigreturn.S
> >+ $(call if_changed_dep,vdso-ilp32as)
> >+
> >+# Actual build commands
> >+quiet_cmd_vdso-ilp32ld = VDSOILP32L $@
> >+ cmd_vdso-ilp32ld = $(CC) $(c_flags) -mabi=ilp32 -Wl,-n -Wl,-T $^ -o $@
> >+quiet_cmd_vdso-ilp32as = VDSOILP32A $@
> >+ cmd_vdso-ilp32as = $(CC) $(a_flags) -mabi=ilp32 -c -o $@ $<
> >+
> >+# Install commands for the unstripped file
> >+quiet_cmd_vdso_install = INSTALL $@
> >+ cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
> >+
> >+vdso-ilp32.so: $(obj)/vdso-ilp32.so.dbg
> >+ @mkdir -p $(MODLIB)/vdso
> >+ $(call cmd,vdso_install)
> >+
> >+vdso_install: vdso-ilp32.so
> >diff --git a/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
> >new file mode 100644
> >index 0000000..46ac072
> >--- /dev/null
> >+++ b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
> >@@ -0,0 +1,33 @@
> >+/*
> >+ * Copyright (C) 2012 ARM Limited
> >+ *
> >+ * This program is free software; you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License version 2 as
> >+ * published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >+ * GNU General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
> >+ *
> >+ * Author: Will Deacon <will.deacon@xxxxxxx>
> >+ */
> >+
> >+#include <linux/init.h>
> >+#include <linux/linkage.h>
> >+#include <linux/const.h>
> >+#include <asm/page.h>
> >+
> >+ __PAGE_ALIGNED_DATA
> >+
> >+ .globl vdso_ilp32_start, vdso_ilp32_end
> >+ .balign PAGE_SIZE
> >+vdso_ilp32_start:
> >+ .incbin "arch/arm64/kernel/vdso-ilp32/vdso-ilp32.so"
> >+ .balign PAGE_SIZE
> >+vdso_ilp32_end:
> >+
> >+ .previous
> >diff --git a/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
> >new file mode 100644
> >index 0000000..ddc63fd
> >--- /dev/null
> >+++ b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
> >@@ -0,0 +1,95 @@
> >+/*
> >+ * GNU linker script for the VDSO library.
> >+ *
> >+ * Copyright (C) 2012 ARM Limited
> >+ *
> >+ * This program is free software; you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License version 2 as
> >+ * published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >+ * GNU General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
> >+ *
> >+ * Author: Will Deacon <will.deacon@xxxxxxx>
> >+ * Heavily based on the vDSO linker scripts for other archs.
> >+ */
> >+
> >+#include <linux/const.h>
> >+#include <asm/page.h>
> >+#include <asm/vdso.h>
> >+
> >+SECTIONS
> >+{
> >+ PROVIDE(_vdso_data = . - PAGE_SIZE);
> >+ . = VDSO_LBASE + SIZEOF_HEADERS;
> >+
> >+ .hash : { *(.hash) } :text
> >+ .gnu.hash : { *(.gnu.hash) }
> >+ .dynsym : { *(.dynsym) }
> >+ .dynstr : { *(.dynstr) }
> >+ .gnu.version : { *(.gnu.version) }
> >+ .gnu.version_d : { *(.gnu.version_d) }
> >+ .gnu.version_r : { *(.gnu.version_r) }
> >+
> >+ .note : { *(.note.*) } :text :note
> >+
> >+ . = ALIGN(16);
> >+
> >+ .text : { *(.text*) } :text =0xd503201f
> >+ PROVIDE (__etext = .);
> >+ PROVIDE (_etext = .);
> >+ PROVIDE (etext = .);
> >+
> >+ .eh_frame_hdr : { *(.eh_frame_hdr) } :text :eh_frame_hdr
> >+ .eh_frame : { KEEP (*(.eh_frame)) } :text
> >+
> >+ .dynamic : { *(.dynamic) } :text :dynamic
> >+
> >+ .rodata : { *(.rodata*) } :text
> >+
> >+ _end = .;
> >+ PROVIDE(end = .);
> >+
> >+ /DISCARD/ : {
> >+ *(.note.GNU-stack)
> >+ *(.data .data.* .gnu.linkonce.d.* .sdata*)
> >+ *(.bss .sbss .dynbss .dynsbss)
> >+ }
> >+}
> >+
> >+/*
> >+ * We must supply the ELF program headers explicitly to get just one
> >+ * PT_LOAD segment, and set the flags explicitly to make segments read-only.
> >+ */
> >+PHDRS
> >+{
> >+ text PT_LOAD FLAGS(5) FILEHDR PHDRS; /* PF_R|PF_X */
> >+ dynamic PT_DYNAMIC FLAGS(4); /* PF_R */
> >+ note PT_NOTE FLAGS(4); /* PF_R */
> >+ eh_frame_hdr PT_GNU_EH_FRAME;
> >+}
> >+
> >+/*
> >+ * This controls what symbols we export from the DSO.
> >+ */
> >+VERSION
> >+{
> >+ LINUX_2.6 {
> >+ global:
> >+ __kernel_rt_sigreturn;
> >+ __kernel_gettimeofday;
> >+ __kernel_clock_gettime;
> >+ __kernel_clock_getres;
> >+ local: *;
> >+ };
> >+}
> >+
> >+/*
> >+ * Make the sigreturn code visible to the kernel.
> >+ */
> >+VDSO_sigtramp_ilp32 = __kernel_rt_sigreturn;
> >diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
> >index 26352a6..521a8e4 100644
> >--- a/arch/arm64/kernel/vdso.c
> >+++ b/arch/arm64/kernel/vdso.c
> >@@ -40,6 +40,12 @@ extern char vdso_start, vdso_end;
> > static unsigned long vdso_pages;
> > static struct page **vdso_pagelist;
> >
> >+#ifdef CONFIG_ARM64_ILP32
> >+extern char vdso_ilp32_start, vdso_ilp32_end;
> >+static unsigned long vdso_ilp32_pages;
> >+static struct page **vdso_ilp32_pagelist;
> >+#endif
> >+
> > /*
> > * The vDSO data page.
> > */
> >@@ -109,24 +115,29 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
> > }
> > #endif /* CONFIG_AARCH32_EL0 */
> >
> >-static struct vm_special_mapping vdso_spec[2];
> >-
> >-static int __init vdso_init(void)
> >+static int __init vdso_init_common(char *vdso_start, char *vdso_end,
> >+ unsigned long *vdso_pagesp,
> >+ struct page ***vdso_pagelistp,
> >+ struct vm_special_mapping* vdso_spec)
> > {
> > int i;
> >+ unsigned long vdso_pages;
> >+ struct page **vdso_pagelist;
> >
> >- if (memcmp(&vdso_start, "\177ELF", 4)) {
> >+ if (memcmp(vdso_start, "\177ELF", 4)) {
> > pr_err("vDSO is not a valid ELF object!\n");
> > return -EINVAL;
> > }
> >
> >- vdso_pages = (&vdso_end - &vdso_start) >> PAGE_SHIFT;
> >+ vdso_pages = (vdso_end - vdso_start) >> PAGE_SHIFT;
> >+ *vdso_pagesp = vdso_pages;
> > pr_info("vdso: %ld pages (%ld code @ %p, %ld data @ %p)\n",
> >- vdso_pages + 1, vdso_pages, &vdso_start, 1L, vdso_data);
> >+ vdso_pages + 1, vdso_pages, vdso_start, 1L, vdso_data);
> >
> > /* Allocate the vDSO pagelist, plus a page for the data. */
> > vdso_pagelist = kcalloc(vdso_pages + 1, sizeof(struct page *),
> > GFP_KERNEL);
> >+ *vdso_pagelistp = vdso_pagelist;
> > if (vdso_pagelist == NULL)
> > return -ENOMEM;
> >
> >@@ -135,7 +146,7 @@ static int __init vdso_init(void)
> >
> > /* Grab the vDSO code pages. */
> > for (i = 0; i < vdso_pages; i++)
> >- vdso_pagelist[i + 1] = virt_to_page(&vdso_start + i * PAGE_SIZE);
> >+ vdso_pagelist[i + 1] = virt_to_page(vdso_start + i * PAGE_SIZE);
> >
> > /* Populate the special mapping structures */
> > vdso_spec[0] = (struct vm_special_mapping) {
> >@@ -150,16 +161,46 @@ static int __init vdso_init(void)
> >
> > return 0;
> > }
> >+
> >+static struct vm_special_mapping vdso_spec[2];
> >+
> >+static int __init vdso_init(void)
> >+{
> >+ return vdso_init_common(&vdso_start, &vdso_end,
> >+ &vdso_pages, &vdso_pagelist,
> >+ vdso_spec);
> >+}
> > arch_initcall(vdso_init);
> >
> >+#ifdef CONFIG_ARM64_ILP32
> >+static struct vm_special_mapping vdso_ilp32_spec[2];
> >+
> >+static int __init vdso_ilp32_init(void)
> >+{
> >+ return vdso_init_common(&vdso_ilp32_start, &vdso_ilp32_end,
> >+ &vdso_ilp32_pages, &vdso_ilp32_pagelist,
> >+ vdso_ilp32_spec);
> >+}
> >+arch_initcall(vdso_ilp32_init);
> >+#endif
> >+
> > int arch_setup_additional_pages(struct linux_binprm *bprm,
> > int uses_interp)
> > {
> > struct mm_struct *mm = current->mm;
> > unsigned long vdso_base, vdso_text_len, vdso_mapping_len;
> > void *ret;
> >+ unsigned long pages = vdso_pages;
> >+ struct vm_special_mapping *spec = vdso_spec;
> >+
> >+#ifdef CONFIG_ARM64_ILP32
> >+ if (is_ilp32_compat_task()) {
> >+ pages = vdso_ilp32_pages;
> >+ spec = vdso_ilp32_spec;
> >+ }
> >+#endif
> >
> >- vdso_text_len = vdso_pages << PAGE_SHIFT;
> >+ vdso_text_len = pages << PAGE_SHIFT;
> > /* Be sure to map the data page */
> > vdso_mapping_len = vdso_text_len + PAGE_SIZE;
> >
> >@@ -171,7 +212,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
> > }
> > ret = _install_special_mapping(mm, vdso_base, PAGE_SIZE,
> > VM_READ|VM_MAYREAD,
> >- &vdso_spec[0]);
> >+ &spec[0]);
> > if (IS_ERR(ret))
> > goto up_fail;
> >
> >@@ -180,7 +221,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm,
> > ret = _install_special_mapping(mm, vdso_base, vdso_text_len,
> > VM_READ|VM_EXEC|
> > VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
> >- &vdso_spec[1]);
> >+ &spec[1]);
> > if (IS_ERR(ret))
> > goto up_fail;
> >
> >