Re: [PATCH 1/2] dlopen: Rework handling of pending NODELETE status

[Florian Weimer <fweimer at redhat dot com>]

* 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?

Thanks,
Florian