Re: [PATCH v4 2/2] ARM: ftrace: Add MODULE_PLTS support

[Alexander Sverdlin <alexander.sverdlin@nokia.com>]

Hi Ard!

On 13/03/18 18:18, Ard Biesheuvel wrote:
>>>>  u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val)
>>>>  {
>>>>         struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core :
>>>>                                                           &mod->arch.init;
>>>> +       struct plt_entries *plt;
>>>> +       int idx;
>>>>
>>>> -       struct plt_entries *plt = (struct plt_entries *)pltsec->plt->sh_addr;
>>                                                            ^^^^^^^^^^^ (*)
>>
>>>> -       int idx = 0;
>>>> +       /* cache the address, ELF header is available only during module load */
>>>> +       if (!pltsec->plt_ent)
>>>> +               pltsec->plt_ent = (struct plt_entries *)pltsec->plt->sh_addr;
>>>> +       plt = pltsec->plt_ent;
>>>> +
>>> Where is plt_ent ever used?
>> Above is exactly the place it's used.
>> I need to cache it because after the module load is finished the ELF header is freed,
>> pltsec->plt pointer (*) is not valid any more.
>> With the above modification it's possible to call the function during the whole life
>> time of the module.
>>
> Right, ok. That's a problem.
> 
> This means that you are relying on get_module_plt() being called at
> least once at module load time, which is not guaranteed.

This is indeed guaranteed. For FTRACE use case. If it's being called from FTRACE in
run time, this would mean there were long calls in this module section, which in
turn means, get_module_plt() was called at least once for this module and this
section.

This doesn't hold in general, though.

In any case, if you insist, I can try to rework the whole stuff implementing module_finalize().

-- 
Best regards,
Alexander Sverdlin.