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

Message ID 20210705183027.3804093-4-adhemerval.zanella@linaro.org
State New
Headers show
Series
  • Linux 64-bit time_t socket fixes
Related show

Commit Message

Adhemerval Zanella via Libc-alpha July 5, 2021, 6:30 p.m.
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(-)

-- 
2.30.2

Comments

Adhemerval Zanella via Libc-alpha July 5, 2021, 7 p.m. | #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 via Libc-alpha July 6, 2021, 1:06 p.m. | #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;
 	}