| Message ID | 20211228143958.3409187-12-guoren@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: compat: Add COMPAT mode support for rv64 | expand |
On Tue, Dec 28, 2021 at 3:39 PM <guoren@kernel.org> wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > Implement necessary type and macro for compat elf. See the code > comment for detail. > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > Signed-off-by: Guo Ren <guoren@kernel.org> > Cc: Arnd Bergmann <arnd@arndb.de> This looks mostly correct, > +/* > + * FIXME: not sure SET_PERSONALITY for compat process is right! > + */ > +#define SET_PERSONALITY(ex) \ > +do { if ((ex).e_ident[EI_CLASS] == ELFCLASS32) \ > + set_thread_flag(TIF_32BIT); \ > + else \ > + clear_thread_flag(TIF_32BIT); \ > + set_personality(PER_LINUX | (current->personality & (~PER_MASK))); \ > +} while (0) This means the personality after exec is always set to PER_LINUX, not PER_LINUX32, which I think is wrong: you want the PER_LINUX32 setting to stick, just like the upper bits do in the default implementation. What the other ones do is: | arch/parisc/include/asm/elf.h- set_personality((current->personality & ~PER_MASK) | PER_LINUX); \ This looks like the same problem you introduce here: always forcing PER_LINUX instead of PER_LINUX32 makes it impossible to use PER_LINUX32. | arch/alpha/include/asm/elf.h:#define SET_PERSONALITY(EX) \ | arch/alpha/include/asm/elf.h- set_personality(((EX).e_flags & EF_ALPHA_32BIT) \ | arch/alpha/include/asm/elf.h- ? PER_LINUX_32BIT : PER_LINUX) | arch/csky/include/asm/elf.h:#define SET_PERSONALITY(ex) set_personality(PER_LINUX) | arch/nds32/include/asm/elf.h:#define SET_PERSONALITY(ex) set_personality(PER_LINUX) These look even worse: instead of forcing the lower bits to PER_LINUX/PER_LINUX32 and leaving the upper bits untouched, these also clear the upper bits unconditionally. | arch/arm64/include/asm/elf.h:#define SET_PERSONALITY(ex) \ | arch/arm64/include/asm/elf.h- current->personality &= ~READ_IMPLIES_EXEC; \ | arch/x86/um/asm/elf.h:#define SET_PERSONALITY(ex) do {} while(0) | arch/x86/include/asm/elf.h:#define set_personality_64bit() do { } while (0) | arch/x86/kernel/process_64.c:static void __set_personality_ia32(void) | current->personality |= force_personality32; Inconsistent: does not enforce PER_LINUX/PER_LINUX32 as the default implementation does, but just leaves the value untouched (other than (re)setting READ_IMPLIES_EXEC). I think this is harmless otherwise, as we generally ignore the lower bits, except for the bit of code that checks for PER_LINUX32 in override_architecture() to mangle the output of sys_newuname() or in /proc/cpuinfo. | arch/s390/include/asm/elf.h- if (personality(current->personality) != PER_LINUX32) \ | arch/s390/include/asm/elf.h- set_personality(PER_LINUX | \ | arch/s390/include/asm/elf.h- (current->personality & ~PER_MASK)); \ | arch/powerpc/include/asm/elf.h- if (personality(current->personality) != PER_LINUX32) \ | arch/powerpc/include/asm/elf.h- set_personality(PER_LINUX | \ | arch/powerpc/include/asm/elf.h- (current->personality & (~PER_MASK))); \ | arch/sparc/include/asm/elf_64.h- if (personality(current->personality) != PER_LINUX32) \ | arch/sparc/include/asm/elf_64.h- set_personality(PER_LINUX | \ | arch/sparc/include/asm/elf_64.h- (current->personality & (~PER_MASK))); \ This is probably the behavior you want to copy. Arnd
On Mon, Jan 10, 2022 at 10:29 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Tue, Dec 28, 2021 at 3:39 PM <guoren@kernel.org> wrote: > > > > From: Guo Ren <guoren@linux.alibaba.com> > > > > Implement necessary type and macro for compat elf. See the code > > comment for detail. > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > Signed-off-by: Guo Ren <guoren@kernel.org> > > Cc: Arnd Bergmann <arnd@arndb.de> > > This looks mostly correct, > > > +/* > > + * FIXME: not sure SET_PERSONALITY for compat process is right! > > + */ > > +#define SET_PERSONALITY(ex) \ > > +do { if ((ex).e_ident[EI_CLASS] == ELFCLASS32) \ > > + set_thread_flag(TIF_32BIT); \ > > + else \ > > + clear_thread_flag(TIF_32BIT); \ > > + set_personality(PER_LINUX | (current->personality & (~PER_MASK))); \ > > +} while (0) > > This means the personality after exec is always set to PER_LINUX, not > PER_LINUX32, which I think is wrong: you want the PER_LINUX32 > setting to stick, just like the upper bits do in the default implementation. > > What the other ones do is: > > | arch/parisc/include/asm/elf.h- > set_personality((current->personality & ~PER_MASK) | PER_LINUX); \ > > This looks like the same problem you introduce here: always forcing PER_LINUX > instead of PER_LINUX32 makes it impossible to use PER_LINUX32. > > | arch/alpha/include/asm/elf.h:#define SET_PERSONALITY(EX) > \ > | arch/alpha/include/asm/elf.h- set_personality(((EX).e_flags & > EF_ALPHA_32BIT) \ > | arch/alpha/include/asm/elf.h- ? PER_LINUX_32BIT : PER_LINUX) > | arch/csky/include/asm/elf.h:#define SET_PERSONALITY(ex) > set_personality(PER_LINUX) > | arch/nds32/include/asm/elf.h:#define SET_PERSONALITY(ex) > set_personality(PER_LINUX) > > These look even worse: instead of forcing the lower bits to > PER_LINUX/PER_LINUX32 and > leaving the upper bits untouched, these also clear the upper bits > unconditionally. > > | arch/arm64/include/asm/elf.h:#define SET_PERSONALITY(ex) > \ > | arch/arm64/include/asm/elf.h- current->personality &= > ~READ_IMPLIES_EXEC; \ > | arch/x86/um/asm/elf.h:#define SET_PERSONALITY(ex) do {} while(0) > | arch/x86/include/asm/elf.h:#define set_personality_64bit() do { > } while (0) > | arch/x86/kernel/process_64.c:static void __set_personality_ia32(void) > | current->personality |= force_personality32; > > Inconsistent: does not enforce PER_LINUX/PER_LINUX32 as the default > implementation > does, but just leaves the value untouched (other than (re)setting > READ_IMPLIES_EXEC). > I think this is harmless otherwise, as we generally ignore the lower > bits, except for the > bit of code that checks for PER_LINUX32 in override_architecture() to mangle the > output of sys_newuname() or in /proc/cpuinfo. > > | arch/s390/include/asm/elf.h- if > (personality(current->personality) != PER_LINUX32) \ > | arch/s390/include/asm/elf.h- set_personality(PER_LINUX | > \ > | arch/s390/include/asm/elf.h- > (current->personality & ~PER_MASK)); \ > | arch/powerpc/include/asm/elf.h- if > (personality(current->personality) != PER_LINUX32) \ > | arch/powerpc/include/asm/elf.h- set_personality(PER_LINUX | > \ > | arch/powerpc/include/asm/elf.h- > (current->personality & (~PER_MASK))); \ > | arch/sparc/include/asm/elf_64.h- if > (personality(current->personality) != PER_LINUX32) \ > | arch/sparc/include/asm/elf_64.h- > set_personality(PER_LINUX | \ > | arch/sparc/include/asm/elf_64.h- > (current->personality & (~PER_MASK))); \ > > This is probably the behavior you want to copy. Thank you very much for your detailed explanation. Here is my modification. +#define SET_PERSONALITY(ex) \ +do { if ((ex).e_ident[EI_CLASS] == ELFCLASS32) \ + set_thread_flag(TIF_32BIT); \ + else \ + clear_thread_flag(TIF_32BIT); \ + if (personality(current->personality) != PER_LINUX32) \ + set_personality(PER_LINUX | \ + (current->personality & (~PER_MASK))); \ +} while (0) > > Arnd -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/
diff --git a/arch/riscv/include/asm/elf.h b/arch/riscv/include/asm/elf.h index f53c40026c7a..91b372d4e13b 100644 --- a/arch/riscv/include/asm/elf.h +++ b/arch/riscv/include/asm/elf.h @@ -8,6 +8,8 @@ #ifndef _ASM_RISCV_ELF_H #define _ASM_RISCV_ELF_H +#include <uapi/linux/elf.h> +#include <linux/compat.h> #include <uapi/asm/elf.h> #include <asm/auxvec.h> #include <asm/byteorder.h> @@ -18,11 +20,13 @@ */ #define ELF_ARCH EM_RISCV +#ifndef ELF_CLASS #ifdef CONFIG_64BIT #define ELF_CLASS ELFCLASS64 #else #define ELF_CLASS ELFCLASS32 #endif +#endif #define ELF_DATA ELFDATA2LSB @@ -31,6 +35,15 @@ */ #define elf_check_arch(x) ((x)->e_machine == EM_RISCV) +#ifdef CONFIG_COMPAT +/* + * Use the same code with elf_check_arch, because elf32_hdr & + * elf64_hdr e_machine's offset are different. The checker is + * a little bit simple compare to other architectures. + */ +#define compat_elf_check_arch(x) ((x)->e_machine == EM_RISCV) +#endif + #define CORE_DUMP_USE_REGSET #define ELF_EXEC_PAGESIZE (PAGE_SIZE) @@ -43,8 +56,14 @@ #define ELF_ET_DYN_BASE ((TASK_SIZE / 3) * 2) #ifdef CONFIG_64BIT +#ifdef CONFIG_COMPAT +#define STACK_RND_MASK (test_thread_flag(TIF_32BIT) ? \ + 0x7ff >> (PAGE_SHIFT - 12) : \ + 0x3ffff >> (PAGE_SHIFT - 12)) +#else #define STACK_RND_MASK (0x3ffff >> (PAGE_SHIFT - 12)) #endif +#endif /* * This yields a mask that user programs can use to figure out what * instruction set this CPU supports. This could be done in user space, @@ -60,11 +79,19 @@ extern unsigned long elf_hwcap; */ #define ELF_PLATFORM (NULL) +#define COMPAT_ELF_PLATFORM (NULL) + #ifdef CONFIG_MMU #define ARCH_DLINFO \ do { \ + /* \ + * Note that we add ulong after elf_addr_t because \ + * casting current->mm->context.vdso triggers a cast \ + * warning of cast from pointer to integer for \ + * COMPAT ELFCLASS32. \ + */ \ NEW_AUX_ENT(AT_SYSINFO_EHDR, \ - (elf_addr_t)current->mm->context.vdso); \ + (elf_addr_t)(ulong)current->mm->context.vdso); \ NEW_AUX_ENT(AT_L1I_CACHESIZE, \ get_cache_size(1, CACHE_TYPE_INST)); \ NEW_AUX_ENT(AT_L1I_CACHEGEOMETRY, \ @@ -90,4 +117,24 @@ do { \ *(struct user_regs_struct *)regs; \ } while (0); +#ifdef CONFIG_COMPAT + +/* + * FIXME: not sure SET_PERSONALITY for compat process is right! + */ +#define SET_PERSONALITY(ex) \ +do { if ((ex).e_ident[EI_CLASS] == ELFCLASS32) \ + set_thread_flag(TIF_32BIT); \ + else \ + clear_thread_flag(TIF_32BIT); \ + set_personality(PER_LINUX | (current->personality & (~PER_MASK))); \ +} while (0) + +#define COMPAT_ELF_ET_DYN_BASE ((TASK_SIZE_32 / 3) * 2) + +/* rv32 registers */ +typedef compat_ulong_t compat_elf_greg_t; +typedef compat_elf_greg_t compat_elf_gregset_t[ELF_NGREG]; + +#endif /* CONFIG_COMPAT */ #endif /* _ASM_RISCV_ELF_H */