Re: [PATCH 1/3] inet: Fix getnameinfo (NI_NOFQDN) race condition (BZ#28566)

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella:

> On 11/11/2021 05:16, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>> 
>>> diff --git a/inet/getnameinfo.c b/inet/getnameinfo.c
>>> index 8380d85783..58ebbb1154 100644
>>> --- a/inet/getnameinfo.c
>>> +++ b/inet/getnameinfo.c
>>> @@ -86,55 +86,75 @@ libc_freeres_ptr (static char *domain);
>>>  static char *
>>>  nrl_domainname (void)
>>>  {
>>> +  __libc_lock_define_initialized (static, lock);
>>> +  __libc_lock_lock (lock);
>>>  
>>> +  static bool not_first = false;
>>>    if (! not_first)
>> 
>>> +    done:
>>> +      scratch_buffer_free (&tmpbuf);
>>> +      not_first = true;
>> 
>> This is missing the acquire/release pairing for the double-checked
>> locking idiom.  You can probably use the domain variable directly.
>
> But it is done now within the lock, different than current implementation
> which does outside.  I moved to be within the lock exactly to avoid the
> double-checked locking idiom.

Ah, sorry, I had missed that.

> I think now that we might be moving to a more optimized lll_lock internally 
> using a acquire-load+CAS instead of just CAS we can get it without need 
> to code it explicitly.

The double-checked locking idiom avoids the CAS after initialization.
With the lll_lock change, an atomic read-modify-write operation still
happens on the lock in all cases (prior to the eventual return to the
caller).

Thanks,
Florian