This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/2] dlopen: Rework handling of pending NODELETE status
On 12/12/2019 10:51, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 05/12/2019 12:19, Florian Weimer wrote:
>>> To avoid a read-modify-write cycle on the l_nodelete field, this
>>> commit introduces two flags for active NODELETE status (irrevocable)
>>> and pending NODELETE status (revocable until activate_nodelete) is
>>> invoked. As a result, NODELETE processing in dlopen does not
>>> introduce further reasons why lazy binding from signal handlers
>>> is unsafe during dlopen, and a subsequent commit can remove signal
>>> blocking from dlopen.
>>
>> The changes to use the new two flags seems right, but I don't fully grasp
>> the avoid modification this patch tries to achieve (read-modify-write).
>> Could you explain why would be required without field split?
>
> I think an example of that is in add_dependency in elf/dl-lookup.c. In
> the old code, we check for any kind of NODELETE status and bail out:
>
> /* Redo the NODELETE check, as when dl_load_lock wasn't held
> yet this could have changed. */
> if (map->l_nodelete != link_map_nodelete_inactive)
> goto out;
>
> And then set pending status (during relocation):
>
> if (flags & DL_LOOKUP_FOR_RELOCATE)
> map->l_nodelete = link_map_nodelete_pending;
> else
> map->l_nodelete = link_map_nodelete_active;
>
> If a signal arrives during relocation and the signal handler, through
> lazy binding, adds a global scope dependency on the same map, it will
> set map->l_nodelete to link_map_nodelete_active. This will be
> overwritten with link_map_nodelete_pending by the dlopen relocation
> code.
>
> Does this explain my concerns?
Right, it is more clear. Could you add this comment on commit message
as well?
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>