[3/7] linux: Use the expected size for SO_TIMESTAMP{NS} convertion

Message ID 20210705183027.3804093-4-adhemerval.zanella@linaro.org
State Committed
Headers
Series Linux 64-bit time_t socket fixes |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella July 5, 2021, 6:30 p.m. UTC
  Kernel returns 32-bit values for COMPAT_SO_TIMESTAMP{NS}_OLD,
no 64-bit ones.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 sysdeps/unix/sysv/linux/convert_scm_timestamps.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
  

Comments

Florian Weimer July 5, 2021, 7 p.m. UTC | #1
* Adhemerval Zanella via Libc-alpha:

> Kernel returns 32-bit values for COMPAT_SO_TIMESTAMP{NS}_OLD,
> no 64-bit ones.

“not 64-bit values”?

> diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> index 5af71847f5..2db5750f50 100644
> --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
> @@ -44,7 +44,8 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>       'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a
>       'struct __kernel_timespec'.  In either case it is two uint64_t
>       members.  */
> -  uint64_t tvts[2];
> +  int64_t tvts[2];
> +  int32_t tmp;
>  
>    struct cmsghdr *cmsg, *last = NULL;
>    int type = 0;
> @@ -69,7 +70,10 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>  
>  	/* fallthrough  */
>  	common:
> -	  memcpy (tvts, CMSG_DATA (cmsg), sizeof (tvts));
> +	  memcpy (&tmp, CMSG_DATA (cmsg), sizeof (tmp));
> +	  tvts[0] = tmp;
> +	  memcpy (&tmp, CMSG_DATA (cmsg) + sizeof (tmp), sizeof (tmp));
> +	  tvts[1] = tmp;
>  	  break;

Maybe it's clearer to make tmp an array and use a single memcpy call?

Thanks,
Florian
  
Adhemerval Zanella July 6, 2021, 1:06 p.m. UTC | #2
On 05/07/2021 16:00, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> Kernel returns 32-bit values for COMPAT_SO_TIMESTAMP{NS}_OLD,
>> no 64-bit ones.
> 
> “not 64-bit values”?

Ack.

> 
>> diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
>> index 5af71847f5..2db5750f50 100644
>> --- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
>> +++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
>> @@ -44,7 +44,8 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>>       'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a
>>       'struct __kernel_timespec'.  In either case it is two uint64_t
>>       members.  */
>> -  uint64_t tvts[2];
>> +  int64_t tvts[2];
>> +  int32_t tmp;
>>  
>>    struct cmsghdr *cmsg, *last = NULL;
>>    int type = 0;
>> @@ -69,7 +70,10 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>>  
>>  	/* fallthrough  */
>>  	common:
>> -	  memcpy (tvts, CMSG_DATA (cmsg), sizeof (tvts));
>> +	  memcpy (&tmp, CMSG_DATA (cmsg), sizeof (tmp));
>> +	  tvts[0] = tmp;
>> +	  memcpy (&tmp, CMSG_DATA (cmsg) + sizeof (tmp), sizeof (tmp));
>> +	  tvts[1] = tmp;
>>  	  break;
> 
> Maybe it's clearer to make tmp an array and use a single memcpy call?

I agreed, I change it to your suggestion.
  

Patch

diff --git a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
index 5af71847f5..2db5750f50 100644
--- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
+++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
@@ -44,7 +44,8 @@  __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
      'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a
      'struct __kernel_timespec'.  In either case it is two uint64_t
      members.  */
-  uint64_t tvts[2];
+  int64_t tvts[2];
+  int32_t tmp;
 
   struct cmsghdr *cmsg, *last = NULL;
   int type = 0;
@@ -69,7 +70,10 @@  __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
 
 	/* fallthrough  */
 	common:
-	  memcpy (tvts, CMSG_DATA (cmsg), sizeof (tvts));
+	  memcpy (&tmp, CMSG_DATA (cmsg), sizeof (tmp));
+	  tvts[0] = tmp;
+	  memcpy (&tmp, CMSG_DATA (cmsg) + sizeof (tmp), sizeof (tmp));
+	  tvts[1] = tmp;
 	  break;
 	}