| Message ID | 20240910-x86-vdso-ifdef-v1-2-877c9df9b081@linutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: vdso: Two small ifdef cleanups | expand |
Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes: > The ifdefs only guard code that is also guarded by in_ia32_syscall(), > which already contains the same ifdefs itself. > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > --- > arch/x86/entry/vdso/vma.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c > index 9059b9d96393..ab2b011471e0 100644 > --- a/arch/x86/entry/vdso/vma.c > +++ b/arch/x86/entry/vdso/vma.c > @@ -75,7 +75,6 @@ static vm_fault_t vdso_fault(const struct vm_special_mapping *sm, > static void vdso_fix_landing(const struct vdso_image *image, > struct vm_area_struct *new_vma) > { > -#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION > if (in_ia32_syscall() && image == &vdso_image_32) { > struct pt_regs *regs = current_pt_regs(); > unsigned long vdso_land = image->sym_int80_landing_pad; > @@ -86,7 +85,6 @@ static void vdso_fix_landing(const struct vdso_image *image, > if (regs->ip == old_land_addr) > regs->ip = new_vma->vm_start + vdso_land; > } > -#endif > } > > static int vdso_mremap(const struct vm_special_mapping *sm, > @@ -339,7 +337,6 @@ int compat_arch_setup_additional_pages(struct linux_binprm *bprm, > > bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) > { > -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION) > const struct vdso_image *image = current->mm->context.vdso_image; > unsigned long vdso = (unsigned long) current->mm->context.vdso; > > @@ -348,7 +345,6 @@ bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) > regs->ip == vdso + image->sym_vdso32_rt_sigreturn_landing_pad) > return true; > } > -#endif > return false; > } Have you tested to verify that after this change arch_syscall_is_vdso_signature compiles out the "image" and "vdso" variables? If the compilers don't it might be worth it rearrange the code as: if (in_ia32_syscall()) { const struct vdso_image *image = current->mm->context.vdso_image; unsigned long vdso = (unsigned long) current->mm->context.vdso; if (image == &vdso_image_32) { .... return true; } } return false. Making the variables depend upon in_ia32_syscall() so you can be certain they are compiles out. Eric
Hi Eric, On 2024-09-10 09:34:46+0000, Eric W. Biederman wrote: > Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes: > > > The ifdefs only guard code that is also guarded by in_ia32_syscall(), > > which already contains the same ifdefs itself. > > > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> > > --- > > arch/x86/entry/vdso/vma.c | 4 ---- > > 1 file changed, 4 deletions(-) > > > > diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c > > index 9059b9d96393..ab2b011471e0 100644 > > --- a/arch/x86/entry/vdso/vma.c > > +++ b/arch/x86/entry/vdso/vma.c > > @@ -75,7 +75,6 @@ static vm_fault_t vdso_fault(const struct vm_special_mapping *sm, > > static void vdso_fix_landing(const struct vdso_image *image, > > struct vm_area_struct *new_vma) > > { > > -#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION > > if (in_ia32_syscall() && image == &vdso_image_32) { > > struct pt_regs *regs = current_pt_regs(); > > unsigned long vdso_land = image->sym_int80_landing_pad; > > @@ -86,7 +85,6 @@ static void vdso_fix_landing(const struct vdso_image *image, > > if (regs->ip == old_land_addr) > > regs->ip = new_vma->vm_start + vdso_land; > > } > > -#endif > > } > > > > static int vdso_mremap(const struct vm_special_mapping *sm, > > @@ -339,7 +337,6 @@ int compat_arch_setup_additional_pages(struct linux_binprm *bprm, > > > > bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) > > { > > -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION) > > const struct vdso_image *image = current->mm->context.vdso_image; > > unsigned long vdso = (unsigned long) current->mm->context.vdso; > > > > @@ -348,7 +345,6 @@ bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) > > regs->ip == vdso + image->sym_vdso32_rt_sigreturn_landing_pad) > > return true; > > } > > -#endif > > return false; > > } > > Have you tested to verify that after this change > arch_syscall_is_vdso_signature compiles out the "image" and "vdso" > variables? Yes, I did: $ objdump --disassemble=arch_syscall_is_vdso_sigreturn arch/x86/entry/vdso/vma.o arch/x86/entry/vdso/vma.o: file format elf64-x86-64 Disassembly of section .text: 00000000000007f0 <arch_syscall_is_vdso_sigreturn>: 7f0: f3 0f 1e fa endbr64 7f4: e8 00 00 00 00 call 7f9 <arch_syscall_is_vdso_sigreturn+0x9> 7f9: 31 c0 xor %eax,%eax 7fb: e9 00 00 00 00 jmp 800 <arch_syscall_is_vdso_sigreturn+0x10> > > If the compilers don't it might be worth it rearrange the code as: > if (in_ia32_syscall()) { > const struct vdso_image *image = current->mm->context.vdso_image; > unsigned long vdso = (unsigned long) current->mm->context.vdso; > > if (image == &vdso_image_32) { > .... > return true; > } > } > return false. > > Making the variables depend upon in_ia32_syscall() so you can be certain > they are compiles out. If that structure is preferred I can send a v2. Thomas
diff --git a/arch/x86/entry/vdso/vma.c b/arch/x86/entry/vdso/vma.c index 9059b9d96393..ab2b011471e0 100644 --- a/arch/x86/entry/vdso/vma.c +++ b/arch/x86/entry/vdso/vma.c @@ -75,7 +75,6 @@ static vm_fault_t vdso_fault(const struct vm_special_mapping *sm, static void vdso_fix_landing(const struct vdso_image *image, struct vm_area_struct *new_vma) { -#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION if (in_ia32_syscall() && image == &vdso_image_32) { struct pt_regs *regs = current_pt_regs(); unsigned long vdso_land = image->sym_int80_landing_pad; @@ -86,7 +85,6 @@ static void vdso_fix_landing(const struct vdso_image *image, if (regs->ip == old_land_addr) regs->ip = new_vma->vm_start + vdso_land; } -#endif } static int vdso_mremap(const struct vm_special_mapping *sm, @@ -339,7 +337,6 @@ int compat_arch_setup_additional_pages(struct linux_binprm *bprm, bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) { -#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION) const struct vdso_image *image = current->mm->context.vdso_image; unsigned long vdso = (unsigned long) current->mm->context.vdso; @@ -348,7 +345,6 @@ bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs) regs->ip == vdso + image->sym_vdso32_rt_sigreturn_landing_pad) return true; } -#endif return false; }
The ifdefs only guard code that is also guarded by in_ia32_syscall(), which already contains the same ifdefs itself. Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de> --- arch/x86/entry/vdso/vma.c | 4 ---- 1 file changed, 4 deletions(-)