Re: [PATCH V2 11/17] riscv: compat: Add elf.h implementation
On Mon, Jan 10, 2022 at 10:29 PM Arnd Bergmann wrote: > > On Tue, Dec 28, 2021 at 3:39 PM wrote: > > > > From: Guo Ren > > > > Implement necessary type and macro for compat elf. See the code > > comment for detail. > > > > Signed-off-by: Guo Ren > > Signed-off-by: Guo Ren > > Cc: Arnd Bergmann > > 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/
Re: [PATCH V2 11/17] riscv: compat: Add elf.h implementation
On Tue, Dec 28, 2021 at 3:39 PM wrote: > > From: Guo Ren > > Implement necessary type and macro for compat elf. See the code > comment for detail. > > Signed-off-by: Guo Ren > Signed-off-by: Guo Ren > Cc: Arnd Bergmann 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
[PATCH V2 11/17] riscv: compat: Add elf.h implementation
From: Guo Ren Implement necessary type and macro for compat elf. See the code comment for detail. Signed-off-by: Guo Ren Signed-off-by: Guo Ren Cc: Arnd Bergmann --- arch/riscv/include/asm/elf.h | 49 +++- 1 file changed, 48 insertions(+), 1 deletion(-) 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 +#include #include #include #include @@ -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) : \ +0x3 >> (PAGE_SHIFT - 12)) +#else #define STACK_RND_MASK (0x3 >> (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 */ -- 2.25.1