Re: [PATCH v2 2/2] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ #28350)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 10/12/2021 08:53, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
>> index 00c934c413..5ee930035b 100644
>> --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
>> +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
>> @@ -54,26 +54,26 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>>         cmsg != NULL;
>>         cmsg = CMSG_NXTHDR (msg, cmsg))
>>      {
>> +      if (cmsg->cmsg_level == SOL_SOCKET)
>>  	{
>> +	  switch (cmsg->cmsg_type)
>> +	    {
>> +	    case COMPAT_SO_TIMESTAMP_OLD:
>> +	      if (type != 0)
>> +		break;
>> +	      type = COMPAT_SO_TIMESTAMP_NEW;
>> +	      goto common;
>> +
>> +	    case COMPAT_SO_TIMESTAMPNS_OLD:
>> +	      type = COMPAT_SO_TIMESTAMPNS_NEW;
>> +
>> +	    /* fallthrough  */
>> +	    common:
>> +	      memcpy (tmp, CMSG_DATA (cmsg), sizeof (tmp));
>> +	      tvts[0] = tmp[0];
>> +	      tvts[1] = tmp[1];
>> +	      break;
>> +	    }
>>  	}
>>  
>>        last = cmsg;
>        }
>    
>      if (last == NULL || type == 0)
>        return;
>    
>      if (CMSG_SPACE (sizeof tvts) > msgsize - msg->msg_controllen)
>        {
>          msg->msg_flags |= MSG_CTRUNC;
>          return;
>>      }
>>
>>    msg->msg_controllen += CMSG_SPACE (sizeof tvts);
>> +  cmsg = CMSG_NXTHDR (msg, last);
>>    if (cmsg == NULL)
>>      return;
>>    cmsg->cmsg_level = SOL_SOCKET;
> 
> How can cmsg be NULL at the final if?  We always iterate until we have
> cmsg == NULL, so CMSG_NXTHDR (msg, last) should be NULL?

Indeed I think it should be safe to drop the check, we already check if before
with CMSG_SPACE and return MSG_CTRUNC if this is no extra space (which will
make CMSG_NXTHDR return NULL).

Are you ok with this change?