Re: [PATCH 3/4] Add 32 bit VDSO support for 32 kernel

[Andy Lutomirski]

On Thu, Jan 30, 2014 at 11:39 AM, Stefani Seibold <stefani@xxxxxxxxxxx> wrote:
> Am Donnerstag, den 30.01.2014, 10:17 -0800 schrieb Andy Lutomirski:
>> > +struct vsyscall_gtod_data vvar_vsyscall_gtod_data
>> > +       __attribute__((visibility("hidden")));
>> > +
>> > +u32 hpet_counter
>> > +       __attribute__((visibility("hidden")));
>> > +
>> > +#define gtod (&vvar_vsyscall_gtod_data)
>>
>> Is it possible to keep the VVAR macro working for this?  It'll keep
>> this code more unified and make it easier for anyone who tries to add
>> vgetcpu support.
>>
>
> Nope, since the access to the VVAR area differs in a 32 bit VDSO differ.
> 64 Bit VDSO always use FIXMAP Address, 32 bit VDSO depends on the sysctl
> parameter vsyscall32 and the vdso32= kernel parameter.

It should be possible to modify arch/x86/include/asm/vvar.h to make
the macro work, I think.

>
> In the origin code there is still gtod macro in the vclock_gettime.c,
> but there is a mixed used of the gtod macro and
> VVAR(vsyscall_gtod_data), so i cleaned it up only use the gtod Macro.
>
> This has also nothing to do with vgetcpu, this is locate in a own file
> called vgetcpu.c

If you fix up the VVAR macro, then you can enable __vdso_getcpu for
32-bit userspace for free.

>
>> >         if (compat_uses_vma || !compat) {
>> > -               /*
>> > -                * MAYWRITE to allow gdb to COW and set breakpoints
>> > -                */
>> > -               ret = install_special_mapping(mm, addr, PAGE_SIZE,
>> > -                                             VM_READ|VM_EXEC|
>> > -                                             VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
>> > -                                             vdso32_pages);
>> > +
>> > +               vma = _install_special_mapping(mm,
>> > +                       addr,
>> > +                       VDSO_OFFSET(VDSO_PAGES),
>> > +                       VM_READ|VM_EXEC,
>> > +                       vdso32_pages);
>> > +
>> > +               if (IS_ERR(vma)) {
>> > +                       ret = PTR_ERR(vma);
>> > +                       goto up_fail;
>> > +               }
>> > +
>> > +               ret = remap_pfn_range(vma,
>> > +                       vma->vm_start + VDSO_OFFSET(VDSO_VVAR_PAGE),
>> > +                       __pa_symbol(&__vvar_page) >> PAGE_SHIFT,
>> > +                       PAGE_SIZE,
>> > +                       PAGE_READONLY);
>> >
>> >                 if (ret)
>> >                         goto up_fail;
>> > +
>> > +#ifdef CONFIG_HPET_TIMER
>> > +               if (hpet_address) {
>> > +                       ret = io_remap_pfn_range(vma,
>> > +                               vma->vm_start + VDSO_OFFSET(VDSO_HPET_PAGE),
>> > +                               hpet_address >> PAGE_SHIFT,
>> > +                               PAGE_SIZE,
>> > +                               pgprot_noncached(PAGE_READONLY));
>> > +
>> > +                       if (ret)
>> > +                               goto up_fail;
>> > +               }
>> > +#endif
>> >         }
>>
>> Now I'm confused.  What's the fixmap stuff for?  Isn't this sufficient?
>>
>> Also, presumably remap_pfn_range is already smart enough to prevent
>> mprotect and ptrace from doing evil things.
>>
>
> The fixmap code is for the original behaviour. It saves address space
> and does not change the layout. Never break user space.

I must still be missing something here.  There *is* no 32-bit
userspace with any expectation of where the VVAR page should be (or
the HPET, for that matter).

In any case, unless I'm reading it wrong, you've now added two
mappings of the vvar page and the hpet on 32-bit: the special mapping
and the fixmap.

Can you summarize exactly what mappings are intended to exist on
32-bit and for compat tasks?

(x32 can already use the 64-bit vdso, right?  I lost track of the
current state of that variant.)

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/