Re: [PATCHv2 2/6] x86/vdso: introduce do_map_vdso() and vdso_type enum

[Dmitry Safonov]

On 07/06/2016 05:21 PM, Andy Lutomirski wrote:
> On Wed, Jun 29, 2016 at 3:57 AM, Dmitry Safonov <dsafonov@xxxxxxxxxxxxx> wrote:
> > Make in-kernel API to map vDSO blobs on x86.
>
> I think the addr calculation was already confusing and is now even
> worse.  How about simplifying it?  Get rid of calculate_addr entirely
> and push the vdso_addr calls into arch_setup_additional_pages, etc.
> Then just use addr directly in the map_vdso code.

Thanks, will do.

> > +int do_map_vdso(vdso_type type, unsigned long addr, bool randomize_addr)
> >  {
> > -       if (vdso32_enabled != 1)  /* Other values all mean "disabled" */
> > -               return 0;
> > -
> > -       return map_vdso(&vdso_image_32, false);
> > -}
> > +       switch (type) {
> > +#if defined(CONFIG_X86_32) || defined(CONFIG_IA32_EMULATION)
> > +       case VDSO_32:
> > +               if (vdso32_enabled != 1)  /* Other values all mean "disabled" */
> > +                       return 0;
> > +               /* vDSO aslr turned off for i386 vDSO */
> > +               return map_vdso(&vdso_image_32, addr, false);
> > +#endif
> > +#ifdef CONFIG_X86_64
> > +       case VDSO_64:
> > +               if (!vdso64_enabled)
> > +                       return 0;
> > +               return map_vdso(&vdso_image_64, addr, randomize_addr);
> > +#endif
> > +#ifdef CONFIG_X86_X32_ABI
> > +       case VDSO_X32:
> > +               if (!vdso64_enabled)
> > +                       return 0;
> > +               return map_vdso(&vdso_image_x32, addr, randomize_addr);
> >  #endif
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +}
>
> Why is this better than just passing the vdso_image pointer in?

Hmm, then all callers should be under the same ifdefs as vdso_image
blobs. Ok, will do.