Address fixups in arch/x86/entry/vdso/vma.c

["H. Peter Anvin" <hpa@zytor.com>]

Hi guys,

I was looking through the vdso setup code (because what I meant to be an
easy optimization turned out to be more "interesting" than expected...)

One of the thing that my mind flagged was this:


static void vdso_fix_landing(const struct vdso_image *image,
                struct vm_area_struct *new_vma)
{
        if (in_ia32_syscall() && image == &vdso_image_32) {
                struct pt_regs *regs = current_pt_regs();
                unsigned long vdso_land = image->sym_int80_landing_pad;
                unsigned long old_land_addr = vdso_land +
                        (unsigned long)current->mm->context.vdso;

                /* Fixing userspace landing - look at do_fast_syscall_32 */
                if (regs->ip == old_land_addr)
                        regs->ip = new_vma->vm_start + vdso_land;
        }
}

static int vdso_mremap(const struct vm_special_mapping *sm,
                struct vm_area_struct *new_vma)
{
        const struct vdso_image *image =
		current->mm->context.vdso_image;

        vdso_fix_landing(image, new_vma);
        current->mm->context.vdso = (void __user *)new_vma->vm_start;

        return 0;
}


--- ---

This feels *way* more complicated than it should need to be.  It seems
to me that if the ip is inside the vdso at all, it would need to be
adjusted, regardless of if it in an ia32 system call or not, and if it
is at the specific landing spot or not.

It is possible that it doesn't *matter*, but that's not really a good
reason to make the code more complex.

I came up with the following version as an alternative; I would be
interesting to hear what you think.

(Also, (unsigned long)current->mm->context.vdso occurs *all over the
place*, but there is also a macro defined for it (VDSO_CURRENT_BASE, in
<asm/elf.h>. My personal preference would be to replace both with an
inline function.)

If you don't think I'm missing something, I would like to do something
like this:


static inline void
vdso_fix_address(unsigned long *ptr, const struct vdso_image *image,
                 unsigned long from, unsigned long to)
{
        if (!image)		/* For potential uses elsewhere */
                return;

        unsigned long offset = *ptr - from;
        if (offset < image->size)
                *ptr = offset + to;
}

static int vdso_mremap(const struct vm_special_mapping *sm,
                struct vm_area_struct *new_vma)
{
        vdso_fix_address(&current_pt_regs()->ip,
                         current->mm->context.vdso_image,
                         vdso_current_base(),
                         new_vma->vm_start);

        current->mm->context.vdso = (void __user *)new_vma->vm_start;
        return 0;
}