[PATCH v2 1/2] support: Add support_socket_so_timestamp_time64

[PATCH v2 1/2] support: Add support_socket_so_timestamp_time64

[Adhemerval Zanella]

Check if the socket support 64-bit network packages timestamps
(SO_TIMESTAMP and SO_TIMESTAMPNS).  This will be used on recvmsg
and recvmmsg tests to check if the timestamp should be generated.

---
v2: Rename to support_socket_so_timestamp_time64.

---
 support/Makefile                             |  1 +
 support/support.h                            |  4 ++
 support/support_socket_so_timestamp_time64.c | 56 ++++++++++++++++++++
 3 files changed, 61 insertions(+)
 create mode 100644 support/support_socket_so_timestamp_time64.c

diff --git a/support/Makefile b/support/Makefile
index 2a0731796f..724ae6d712 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -78,6 +78,7 @@ libsupport-routines = \
   support_set_small_thread_stack_size \
   support_shared_allocate \
   support_small_stack_thread_attribute \
+  support_socket_so_timestamp_time64 \
   support_stat_nanoseconds \
   support_subprocess \
   support_test_compare_blob \
diff --git a/support/support.h b/support/support.h
index 837a806531..ec15d81009 100644
--- a/support/support.h
+++ b/support/support.h
@@ -176,6 +176,10 @@ extern bool support_select_modifies_timeout (void);
    tv_usec larger than 1000000.  */
 extern bool support_select_normalizes_timeout (void);
 
+/* Return true if socket FD supports 64-bit timestamps with the SOL_SOCKET
+   and SO_TIMESTAMP/SO_TIMESTAMPNS.  */
+extern bool support_socket_so_timestamp_time64 (int fd);
+
 /* Create a timer that trigger after SEC seconds and NSEC nanoseconds.  If
    REPEAT is true the timer will repeat indefinitely.  If CALLBACK is not
    NULL, the function will be called when the timer expires; otherwise a
diff --git a/support/support_socket_so_timestamp_time64.c b/support/support_socket_so_timestamp_time64.c
new file mode 100644
index 0000000000..eb8f8ef0cc
--- /dev/null
+++ b/support/support_socket_so_timestamp_time64.c
@@ -0,0 +1,56 @@
+/* Return whether socket supports 64-bit timestamps.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <sys/socket.h>
+#include <support/support.h>
+#ifdef __linux__
+# include <socket-constants-time64.h>
+#endif
+
+bool
+support_socket_so_timestamp_time64 (int fd)
+{
+#ifdef __linux__
+# if __LINUX_KERNEL_VERSION >= 0x050100                          \
+   || __WORDSIZE == 64                                           \
+   || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64)
+  return true;
+# else
+  int level = SOL_SOCKET;
+  int optname = COMPAT_SO_TIMESTAMP_NEW;
+  int optval;
+  socklen_t len = sizeof (optval);
+
+  int r = syscall (__NR_getsockopt, fd, level, optname, &optval, &len);
+#  ifdef __NR_socketcall
+  if (r == -1 && errno == ENOSYS)
+    {
+      long int args[] = { fd, level, optname, (long int) &optval,
+			  sizeof (optval) };
+      r = syscall (__NR_socketcall, 15 /* SOCKOP_getsockopt  */ , args);
+    }
+#  endif
+  return r != -1;
+# endif
+#else
+  return false;
+#endif
+}
-- 
2.30.2

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

[Adhemerval Zanella]

The __convert_scm_timestamps() only updates the control message last
pointer for SOL_SOCKET type, so if the message control buffer contains
multiple ancillary message types the converted timestamp one might
overwrite a valid message.

The test check if the extra ancillary space is correctly handled
by recvmsg/recvmmsg, where if there is no extra space for the 64-bit
time_t converted message the control buffer should be marked with
MSG_TRUNC.  It also check if recvmsg/recvmmsg handle correctly multiple
ancillary data.

Checked on x86_64-linux and on i686-linux-gnu on both 5.11 and
4.15 kernel.

---
v2: Fix some typos and use support_socket_time64_timestamp.

---
 sysdeps/unix/sysv/linux/Makefile              |   3 +
 .../unix/sysv/linux/convert_scm_timestamps.c  |  40 +--
 .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
 .../unix/sysv/linux/tst-socket-timestamp.c    | 338 ++++++++++++++++++
 4 files changed, 362 insertions(+), 20 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp.c

diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 76ad06361c..3399c87468 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -262,6 +262,9 @@ sysdep_routines += cmsg_nxthdr
 CFLAGS-recvmmsg.c = -fexceptions -fasynchronous-unwind-tables
 CFLAGS-sendmmsg.c = -fexceptions -fasynchronous-unwind-tables
 
+tests += tst-socket-timestamp
+tests-time64 += tst-socket-timestamp-time64
+
 tests-special += $(objpfx)tst-socket-consts.out
 $(objpfx)tst-socket-consts.out: ../sysdeps/unix/sysv/linux/tst-socket-consts.py
 	PYTHONPATH=../scripts \
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)
-	continue;
-
-      switch (cmsg->cmsg_type)
+      if (cmsg->cmsg_level == SOL_SOCKET)
 	{
-	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;
+	  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;
@@ -89,7 +89,7 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
     }
 
   msg->msg_controllen += CMSG_SPACE (sizeof tvts);
-  cmsg = CMSG_NXTHDR(msg, last);
+  cmsg = CMSG_NXTHDR (msg, last);
   if (cmsg == NULL)
     return;
   cmsg->cmsg_level = SOL_SOCKET;
diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
new file mode 100644
index 0000000000..ae424c2a70
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
@@ -0,0 +1 @@
+#include "tst-socket-timestamp.c"
diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
new file mode 100644
index 0000000000..9aaa705054
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
@@ -0,0 +1,338 @@
+/* Check recvmsg/recvmmsg 64-bit timestamp support.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <array_length.h>
+#include <arpa/inet.h>
+#include <errno.h>
+#include <string.h>
+#include <stdio.h>
+#include <support/check.h>
+#include <support/next_to_fault.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+#include <support/xunistd.h>
+#include <support/xsocket.h>
+#include <sys/mman.h>
+
+/* Some extra space added for ancillary data, it might be used to convert
+   32-bit timestamp to 64-bit for _TIME_BITS=64.  */
+enum { slack_max_size = 64 };
+static const int slack[] = { 0, 4, 8, 16, 32, slack_max_size };
+
+static bool support_64_timestamp;
+/* AF_INET socket and address used to send and receive data.  */
+static int srv;
+static struct sockaddr_in srv_addr;
+
+static int
+do_sendto (const struct sockaddr_in *addr, int nmsgs)
+{
+  int s = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+  xconnect (s, (const struct sockaddr *) addr, sizeof (*addr));
+
+  for (int i = 0; i < nmsgs; i++)
+    xsendto (s, &i, sizeof (i), 0, (const struct sockaddr *) addr,
+	     sizeof (*addr));
+
+  return 0;
+}
+
+static void
+do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
+			    size_t slack, size_t tsize, int exp_msg)
+{
+  int msg;
+  struct iovec iov =
+    {
+      .iov_base = &msg,
+      .iov_len = sizeof (msg)
+    };
+  size_t msgs = CMSG_SPACE (tsize) + slack;
+  struct mmsghdr mmhdr =
+    {
+      .msg_hdr =
+      {
+        .msg_name = NULL,
+        .msg_namelen = 0,
+        .msg_iov = &iov,
+        .msg_iovlen = 1,
+        .msg_controllen = msgs,
+        .msg_control = cmsg != NULL ? cmsg - msgs : NULL,
+      },
+    };
+
+  int r;
+  if (use_multi_call)
+    {
+      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
+      if (r >= 0)
+	r = mmhdr.msg_len;
+    }
+  else
+    r = recvmsg (s, &mmhdr.msg_hdr, 0);
+  TEST_COMPARE (r, sizeof (int));
+  TEST_COMPARE (msg, exp_msg);
+
+  if (cmsg == NULL)
+    return;
+
+  /* A timestamp is expected if 32-bit timestamp are used (support in every
+     configuration) or if underlying kernel support 64-bit timestamps.
+     Otherwise recvmsg will need extra space do add the 64-bit timestamp.  */
+  bool exp_timestamp;
+  if (sizeof (time_t) == 4 || support_64_timestamp)
+    exp_timestamp = true;
+   else
+    exp_timestamp = slack >= CMSG_SPACE (tsize);
+
+  bool timestamp = false;
+  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
+       cmsg != NULL;
+       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
+    {
+      if (cmsg->cmsg_level != SOL_SOCKET)
+	continue;
+      if (cmsg->cmsg_type == SCM_TIMESTAMP
+	  && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timeval)))
+	{
+	  struct timeval tv;
+	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
+	  if (test_verbose)
+	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
+		    (intmax_t)tv.tv_usec);
+	  timestamp = true;
+	}
+      else if (cmsg->cmsg_type == SCM_TIMESTAMPNS
+	       && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timespec)))
+	{
+	  struct timespec ts;
+	  memcpy (&ts, CMSG_DATA (cmsg), sizeof (ts));
+	  if (test_verbose)
+	    printf ("SCM_TIMESTAMPNS: {%jd, %jd}\n", (intmax_t)ts.tv_sec,
+		    (intmax_t)ts.tv_nsec);
+	  timestamp = true;
+	}
+    }
+
+  /* If there is not timestamp in the ancilliary data, recvmsg should set
+     the flag inidcating it.  */
+  if (exp_timestamp && !timestamp)
+    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
+  else
+    TEST_COMPARE (timestamp, exp_timestamp);
+}
+
+/* Check if the extra ancillary space is correctly handled by recvmsg and
+   recvmmsg: if there is no extra space for the 64-bit timestamp the syscall
+   should set MSG_TRUNC flag, otherwise an extra cmsghdr with the converted
+   timestamp is appended at the message control buffer.  */
+static void
+do_test_slack_space (void)
+{
+  /* Setup the ancillary data buffer with an extra page with PROT_NONE to
+     check the possible timestamp conversion on some systems.  */
+  struct support_next_to_fault nf =
+    support_next_to_fault_allocate (slack_max_size);
+  void *msgbuf = nf.buffer + slack_max_size;
+
+  /* Enable the timestamp using struct timeval precision.  */
+  {
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+  }
+  /* Check recvmsg.  */
+  do_sendto (&srv_addr, array_length (slack));
+  for (int s = 0; s < array_length (slack); s++)
+    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
+				sizeof (struct timeval), s);
+  /* Check recvmmsg.  */
+  do_sendto (&srv_addr, array_length (slack));
+  for (int s = 0; s < array_length (slack); s++)
+    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
+				sizeof (struct timeval), s);
+
+  /* Now enable timestamp using a higher precision, it overwrites the previous
+     precision.  */
+  {
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+  }
+  /* Check recvmsg.  */
+  do_sendto (&srv_addr, array_length (slack));
+  for (int s = 0; s < array_length (slack); s++)
+    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
+				sizeof (struct timespec), s);
+  /* Check recvmmsg.  */
+  do_sendto (&srv_addr, array_length (slack));
+  for (int s = 0; s < array_length (slack); s++)
+    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
+				sizeof (struct timespec), s);
+
+  support_next_to_fault_free (&nf);
+}
+
+/* Check if the converted 64-bit timestamp is correctly appended when there
+   are multiple ancillary messages.  */
+static void
+do_recvmsg_multiple_ancillary (bool use_multi_call, int s, void *cmsg,
+			       size_t cmsgsize, int exp_msg)
+{
+  int msg;
+  struct iovec iov =
+    {
+      .iov_base = &msg,
+      .iov_len = sizeof (msg)
+    };
+  size_t msgs = cmsgsize;
+  struct mmsghdr mmhdr =
+    {
+      .msg_hdr =
+      {
+        .msg_name = NULL,
+        .msg_namelen = 0,
+        .msg_iov = &iov,
+        .msg_iovlen = 1,
+        .msg_controllen = msgs,
+        .msg_control = cmsg,
+      },
+    };
+
+  int r;
+  if (use_multi_call)
+    {
+      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
+      if (r >= 0)
+	r = mmhdr.msg_len;
+    }
+  else
+    r = recvmsg (s, &mmhdr.msg_hdr, 0);
+  TEST_COMPARE (r, sizeof (int));
+  TEST_COMPARE (msg, exp_msg);
+
+  if (cmsg == NULL)
+    return;
+
+  bool timestamp = false;
+  bool origdstaddr = false;
+  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
+       cmsg != NULL;
+       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
+    {
+      if (cmsg->cmsg_level == SOL_IP
+	  && cmsg->cmsg_type == IP_ORIGDSTADDR
+	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct sockaddr_in)))
+	{
+	  struct sockaddr_in sa;
+	  memcpy (&sa, CMSG_DATA (cmsg), sizeof (sa));
+	  if (test_verbose)
+	    {
+	      char str[INET_ADDRSTRLEN];
+	      inet_ntop (AF_INET, &sa.sin_addr, str, INET_ADDRSTRLEN);
+	      printf ("IP_ORIGDSTADDR:  %s:%d\n", str, ntohs (sa.sin_port));
+	    }
+	  origdstaddr = sa.sin_addr.s_addr == srv_addr.sin_addr.s_addr
+			&& sa.sin_port == srv_addr.sin_port;
+	}
+      if (cmsg->cmsg_level == SOL_SOCKET
+	  && cmsg->cmsg_type == SCM_TIMESTAMP
+	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct timeval)))
+	{
+	  struct timeval tv;
+	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
+	  if (test_verbose)
+	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
+		    (intmax_t)tv.tv_usec);
+	  timestamp = true;
+	}
+    }
+
+  /* If there is no timestamp in the ancillary data, recvmsg should set
+     the flag to indicate it.  */
+  if (!timestamp)
+    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
+  else
+    TEST_COMPARE (timestamp, true);
+  TEST_COMPARE (origdstaddr, true);
+}
+
+static void
+do_test_multiple_ancillary (void)
+{
+  {
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+  }
+  {
+    int r = setsockopt (srv, IPPROTO_IP, IP_RECVORIGDSTADDR, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+  }
+
+  /* Enougth data for default SO_TIMESTAMP, the IP_RECVORIGDSTADDR, and the
+     extra 64-bit SO_TIMESTAMP.  */
+  enum { msgbuflen = CMSG_SPACE (2 * sizeof (uint64_t))
+		     + CMSG_SPACE (sizeof (struct sockaddr_in))
+		     + CMSG_SPACE (2 * sizeof (uint64_t)) };
+  char msgbuf[msgbuflen];
+
+  enum { nmsgs = 8 };
+  /* Check recvmsg.  */
+  do_sendto (&srv_addr, nmsgs);
+  for (int s = 0; s < nmsgs; s++)
+    do_recvmsg_multiple_ancillary (false, srv, msgbuf, msgbuflen, s);
+  /* Check recvmmsg.  */
+  do_sendto (&srv_addr, nmsgs);
+  for (int s = 0; s < nmsgs; s++)
+    do_recvmsg_multiple_ancillary (true, srv, msgbuf, msgbuflen, s);
+}
+
+static int
+do_test (void)
+{
+  srv = xsocket (AF_INET, SOCK_DGRAM, 0);
+  srv_addr = (struct sockaddr_in) {
+    .sin_family = AF_INET,
+    .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) },
+  };
+  xbind (srv, (struct sockaddr *) &srv_addr, sizeof (srv_addr));
+  {
+    socklen_t sa_len = sizeof (srv_addr);
+    xgetsockname (srv, (struct sockaddr *) &srv_addr, &sa_len);
+    TEST_VERIFY (sa_len == sizeof (srv_addr));
+  }
+
+  TEST_COMPARE (recvmsg (-1, NULL, 0), -1);
+  TEST_COMPARE (errno, EBADF);
+  TEST_COMPARE (recvmmsg (-1, NULL, 0, 0, NULL), -1);
+  TEST_COMPARE (errno, EBADF);
+
+  /* If underlying kernel does not support   */
+  support_64_timestamp = support_socket_so_timestamp_time64 (srv);
+
+  do_test_slack_space ();
+  do_test_multiple_ancillary ();
+
+  xclose (srv);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.30.2

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

[Adhemerval Zanella]

Ping.

On 29/09/2021 11:06, Adhemerval Zanella wrote:
> The __convert_scm_timestamps() only updates the control message last
> pointer for SOL_SOCKET type, so if the message control buffer contains
> multiple ancillary message types the converted timestamp one might
> overwrite a valid message.
> 
> The test check if the extra ancillary space is correctly handled
> by recvmsg/recvmmsg, where if there is no extra space for the 64-bit
> time_t converted message the control buffer should be marked with
> MSG_TRUNC.  It also check if recvmsg/recvmmsg handle correctly multiple
> ancillary data.
> 
> Checked on x86_64-linux and on i686-linux-gnu on both 5.11 and
> 4.15 kernel.
> 
> ---
> v2: Fix some typos and use support_socket_time64_timestamp.
> 
> ---
>  sysdeps/unix/sysv/linux/Makefile              |   3 +
>  .../unix/sysv/linux/convert_scm_timestamps.c  |  40 +--
>  .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
>  .../unix/sysv/linux/tst-socket-timestamp.c    | 338 ++++++++++++++++++
>  4 files changed, 362 insertions(+), 20 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp.c
> 
> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 76ad06361c..3399c87468 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -262,6 +262,9 @@ sysdep_routines += cmsg_nxthdr
>  CFLAGS-recvmmsg.c = -fexceptions -fasynchronous-unwind-tables
>  CFLAGS-sendmmsg.c = -fexceptions -fasynchronous-unwind-tables
>  
> +tests += tst-socket-timestamp
> +tests-time64 += tst-socket-timestamp-time64
> +
>  tests-special += $(objpfx)tst-socket-consts.out
>  $(objpfx)tst-socket-consts.out: ../sysdeps/unix/sysv/linux/tst-socket-consts.py
>  	PYTHONPATH=../scripts \
> 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)
> -	continue;
> -
> -      switch (cmsg->cmsg_type)
> +      if (cmsg->cmsg_level == SOL_SOCKET)
>  	{
> -	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;
> +	  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;
> @@ -89,7 +89,7 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>      }
>  
>    msg->msg_controllen += CMSG_SPACE (sizeof tvts);
> -  cmsg = CMSG_NXTHDR(msg, last);
> +  cmsg = CMSG_NXTHDR (msg, last);
>    if (cmsg == NULL)
>      return;
>    cmsg->cmsg_level = SOL_SOCKET;
> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
> new file mode 100644
> index 0000000000..ae424c2a70
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
> @@ -0,0 +1 @@
> +#include "tst-socket-timestamp.c"
> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
> new file mode 100644
> index 0000000000..9aaa705054
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
> @@ -0,0 +1,338 @@
> +/* Check recvmsg/recvmmsg 64-bit timestamp support.
> +   Copyright (C) 2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <array_length.h>
> +#include <arpa/inet.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <stdio.h>
> +#include <support/check.h>
> +#include <support/next_to_fault.h>
> +#include <support/support.h>
> +#include <support/test-driver.h>
> +#include <support/xunistd.h>
> +#include <support/xsocket.h>
> +#include <sys/mman.h>
> +
> +/* Some extra space added for ancillary data, it might be used to convert
> +   32-bit timestamp to 64-bit for _TIME_BITS=64.  */
> +enum { slack_max_size = 64 };
> +static const int slack[] = { 0, 4, 8, 16, 32, slack_max_size };
> +
> +static bool support_64_timestamp;
> +/* AF_INET socket and address used to send and receive data.  */
> +static int srv;
> +static struct sockaddr_in srv_addr;
> +
> +static int
> +do_sendto (const struct sockaddr_in *addr, int nmsgs)
> +{
> +  int s = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
> +  xconnect (s, (const struct sockaddr *) addr, sizeof (*addr));
> +
> +  for (int i = 0; i < nmsgs; i++)
> +    xsendto (s, &i, sizeof (i), 0, (const struct sockaddr *) addr,
> +	     sizeof (*addr));
> +
> +  return 0;
> +}
> +
> +static void
> +do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
> +			    size_t slack, size_t tsize, int exp_msg)
> +{
> +  int msg;
> +  struct iovec iov =
> +    {
> +      .iov_base = &msg,
> +      .iov_len = sizeof (msg)
> +    };
> +  size_t msgs = CMSG_SPACE (tsize) + slack;
> +  struct mmsghdr mmhdr =
> +    {
> +      .msg_hdr =
> +      {
> +        .msg_name = NULL,
> +        .msg_namelen = 0,
> +        .msg_iov = &iov,
> +        .msg_iovlen = 1,
> +        .msg_controllen = msgs,
> +        .msg_control = cmsg != NULL ? cmsg - msgs : NULL,
> +      },
> +    };
> +
> +  int r;
> +  if (use_multi_call)
> +    {
> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
> +      if (r >= 0)
> +	r = mmhdr.msg_len;
> +    }
> +  else
> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
> +  TEST_COMPARE (r, sizeof (int));
> +  TEST_COMPARE (msg, exp_msg);
> +
> +  if (cmsg == NULL)
> +    return;
> +
> +  /* A timestamp is expected if 32-bit timestamp are used (support in every
> +     configuration) or if underlying kernel support 64-bit timestamps.
> +     Otherwise recvmsg will need extra space do add the 64-bit timestamp.  */
> +  bool exp_timestamp;
> +  if (sizeof (time_t) == 4 || support_64_timestamp)
> +    exp_timestamp = true;
> +   else
> +    exp_timestamp = slack >= CMSG_SPACE (tsize);
> +
> +  bool timestamp = false;
> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
> +       cmsg != NULL;
> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
> +    {
> +      if (cmsg->cmsg_level != SOL_SOCKET)
> +	continue;
> +      if (cmsg->cmsg_type == SCM_TIMESTAMP
> +	  && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timeval)))
> +	{
> +	  struct timeval tv;
> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
> +	  if (test_verbose)
> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
> +		    (intmax_t)tv.tv_usec);
> +	  timestamp = true;
> +	}
> +      else if (cmsg->cmsg_type == SCM_TIMESTAMPNS
> +	       && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timespec)))
> +	{
> +	  struct timespec ts;
> +	  memcpy (&ts, CMSG_DATA (cmsg), sizeof (ts));
> +	  if (test_verbose)
> +	    printf ("SCM_TIMESTAMPNS: {%jd, %jd}\n", (intmax_t)ts.tv_sec,
> +		    (intmax_t)ts.tv_nsec);
> +	  timestamp = true;
> +	}
> +    }
> +
> +  /* If there is not timestamp in the ancilliary data, recvmsg should set
> +     the flag inidcating it.  */
> +  if (exp_timestamp && !timestamp)
> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
> +  else
> +    TEST_COMPARE (timestamp, exp_timestamp);
> +}
> +
> +/* Check if the extra ancillary space is correctly handled by recvmsg and
> +   recvmmsg: if there is no extra space for the 64-bit timestamp the syscall
> +   should set MSG_TRUNC flag, otherwise an extra cmsghdr with the converted
> +   timestamp is appended at the message control buffer.  */
> +static void
> +do_test_slack_space (void)
> +{
> +  /* Setup the ancillary data buffer with an extra page with PROT_NONE to
> +     check the possible timestamp conversion on some systems.  */
> +  struct support_next_to_fault nf =
> +    support_next_to_fault_allocate (slack_max_size);
> +  void *msgbuf = nf.buffer + slack_max_size;
> +
> +  /* Enable the timestamp using struct timeval precision.  */
> +  {
> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
> +			sizeof (int));
> +    TEST_VERIFY_EXIT (r != -1);
> +  }
> +  /* Check recvmsg.  */
> +  do_sendto (&srv_addr, array_length (slack));
> +  for (int s = 0; s < array_length (slack); s++)
> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
> +				sizeof (struct timeval), s);
> +  /* Check recvmmsg.  */
> +  do_sendto (&srv_addr, array_length (slack));
> +  for (int s = 0; s < array_length (slack); s++)
> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
> +				sizeof (struct timeval), s);
> +
> +  /* Now enable timestamp using a higher precision, it overwrites the previous
> +     precision.  */
> +  {
> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1},
> +			sizeof (int));
> +    TEST_VERIFY_EXIT (r != -1);
> +  }
> +  /* Check recvmsg.  */
> +  do_sendto (&srv_addr, array_length (slack));
> +  for (int s = 0; s < array_length (slack); s++)
> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
> +				sizeof (struct timespec), s);
> +  /* Check recvmmsg.  */
> +  do_sendto (&srv_addr, array_length (slack));
> +  for (int s = 0; s < array_length (slack); s++)
> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
> +				sizeof (struct timespec), s);
> +
> +  support_next_to_fault_free (&nf);
> +}
> +
> +/* Check if the converted 64-bit timestamp is correctly appended when there
> +   are multiple ancillary messages.  */
> +static void
> +do_recvmsg_multiple_ancillary (bool use_multi_call, int s, void *cmsg,
> +			       size_t cmsgsize, int exp_msg)
> +{
> +  int msg;
> +  struct iovec iov =
> +    {
> +      .iov_base = &msg,
> +      .iov_len = sizeof (msg)
> +    };
> +  size_t msgs = cmsgsize;
> +  struct mmsghdr mmhdr =
> +    {
> +      .msg_hdr =
> +      {
> +        .msg_name = NULL,
> +        .msg_namelen = 0,
> +        .msg_iov = &iov,
> +        .msg_iovlen = 1,
> +        .msg_controllen = msgs,
> +        .msg_control = cmsg,
> +      },
> +    };
> +
> +  int r;
> +  if (use_multi_call)
> +    {
> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
> +      if (r >= 0)
> +	r = mmhdr.msg_len;
> +    }
> +  else
> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
> +  TEST_COMPARE (r, sizeof (int));
> +  TEST_COMPARE (msg, exp_msg);
> +
> +  if (cmsg == NULL)
> +    return;
> +
> +  bool timestamp = false;
> +  bool origdstaddr = false;
> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
> +       cmsg != NULL;
> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
> +    {
> +      if (cmsg->cmsg_level == SOL_IP
> +	  && cmsg->cmsg_type == IP_ORIGDSTADDR
> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct sockaddr_in)))
> +	{
> +	  struct sockaddr_in sa;
> +	  memcpy (&sa, CMSG_DATA (cmsg), sizeof (sa));
> +	  if (test_verbose)
> +	    {
> +	      char str[INET_ADDRSTRLEN];
> +	      inet_ntop (AF_INET, &sa.sin_addr, str, INET_ADDRSTRLEN);
> +	      printf ("IP_ORIGDSTADDR:  %s:%d\n", str, ntohs (sa.sin_port));
> +	    }
> +	  origdstaddr = sa.sin_addr.s_addr == srv_addr.sin_addr.s_addr
> +			&& sa.sin_port == srv_addr.sin_port;
> +	}
> +      if (cmsg->cmsg_level == SOL_SOCKET
> +	  && cmsg->cmsg_type == SCM_TIMESTAMP
> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct timeval)))
> +	{
> +	  struct timeval tv;
> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
> +	  if (test_verbose)
> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
> +		    (intmax_t)tv.tv_usec);
> +	  timestamp = true;
> +	}
> +    }
> +
> +  /* If there is no timestamp in the ancillary data, recvmsg should set
> +     the flag to indicate it.  */
> +  if (!timestamp)
> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
> +  else
> +    TEST_COMPARE (timestamp, true);
> +  TEST_COMPARE (origdstaddr, true);
> +}
> +
> +static void
> +do_test_multiple_ancillary (void)
> +{
> +  {
> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
> +			sizeof (int));
> +    TEST_VERIFY_EXIT (r != -1);
> +  }
> +  {
> +    int r = setsockopt (srv, IPPROTO_IP, IP_RECVORIGDSTADDR, &(int){1},
> +			sizeof (int));
> +    TEST_VERIFY_EXIT (r != -1);
> +  }
> +
> +  /* Enougth data for default SO_TIMESTAMP, the IP_RECVORIGDSTADDR, and the
> +     extra 64-bit SO_TIMESTAMP.  */
> +  enum { msgbuflen = CMSG_SPACE (2 * sizeof (uint64_t))
> +		     + CMSG_SPACE (sizeof (struct sockaddr_in))
> +		     + CMSG_SPACE (2 * sizeof (uint64_t)) };
> +  char msgbuf[msgbuflen];
> +
> +  enum { nmsgs = 8 };
> +  /* Check recvmsg.  */
> +  do_sendto (&srv_addr, nmsgs);
> +  for (int s = 0; s < nmsgs; s++)
> +    do_recvmsg_multiple_ancillary (false, srv, msgbuf, msgbuflen, s);
> +  /* Check recvmmsg.  */
> +  do_sendto (&srv_addr, nmsgs);
> +  for (int s = 0; s < nmsgs; s++)
> +    do_recvmsg_multiple_ancillary (true, srv, msgbuf, msgbuflen, s);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  srv = xsocket (AF_INET, SOCK_DGRAM, 0);
> +  srv_addr = (struct sockaddr_in) {
> +    .sin_family = AF_INET,
> +    .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) },
> +  };
> +  xbind (srv, (struct sockaddr *) &srv_addr, sizeof (srv_addr));
> +  {
> +    socklen_t sa_len = sizeof (srv_addr);
> +    xgetsockname (srv, (struct sockaddr *) &srv_addr, &sa_len);
> +    TEST_VERIFY (sa_len == sizeof (srv_addr));
> +  }
> +
> +  TEST_COMPARE (recvmsg (-1, NULL, 0), -1);
> +  TEST_COMPARE (errno, EBADF);
> +  TEST_COMPARE (recvmmsg (-1, NULL, 0, 0, NULL), -1);
> +  TEST_COMPARE (errno, EBADF);
> +
> +  /* If underlying kernel does not support   */
> +  support_64_timestamp = support_socket_so_timestamp_time64 (srv);
> +
> +  do_test_slack_space ();
> +  do_test_multiple_ancillary ();
> +
> +  xclose (srv);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> 

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

[Adhemerval Zanella]

Ping (x2).

On 08/10/2021 08:58, Adhemerval Zanella wrote:
> Ping.
> 
> On 29/09/2021 11:06, Adhemerval Zanella wrote:
>> The __convert_scm_timestamps() only updates the control message last
>> pointer for SOL_SOCKET type, so if the message control buffer contains
>> multiple ancillary message types the converted timestamp one might
>> overwrite a valid message.
>>
>> The test check if the extra ancillary space is correctly handled
>> by recvmsg/recvmmsg, where if there is no extra space for the 64-bit
>> time_t converted message the control buffer should be marked with
>> MSG_TRUNC.  It also check if recvmsg/recvmmsg handle correctly multiple
>> ancillary data.
>>
>> Checked on x86_64-linux and on i686-linux-gnu on both 5.11 and
>> 4.15 kernel.
>>
>> ---
>> v2: Fix some typos and use support_socket_time64_timestamp.
>>
>> ---
>>  sysdeps/unix/sysv/linux/Makefile              |   3 +
>>  .../unix/sysv/linux/convert_scm_timestamps.c  |  40 +--
>>  .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
>>  .../unix/sysv/linux/tst-socket-timestamp.c    | 338 ++++++++++++++++++
>>  4 files changed, 362 insertions(+), 20 deletions(-)
>>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>>
>> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
>> index 76ad06361c..3399c87468 100644
>> --- a/sysdeps/unix/sysv/linux/Makefile
>> +++ b/sysdeps/unix/sysv/linux/Makefile
>> @@ -262,6 +262,9 @@ sysdep_routines += cmsg_nxthdr
>>  CFLAGS-recvmmsg.c = -fexceptions -fasynchronous-unwind-tables
>>  CFLAGS-sendmmsg.c = -fexceptions -fasynchronous-unwind-tables
>>  
>> +tests += tst-socket-timestamp
>> +tests-time64 += tst-socket-timestamp-time64
>> +
>>  tests-special += $(objpfx)tst-socket-consts.out
>>  $(objpfx)tst-socket-consts.out: ../sysdeps/unix/sysv/linux/tst-socket-consts.py
>>  	PYTHONPATH=../scripts \
>> 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)
>> -	continue;
>> -
>> -      switch (cmsg->cmsg_type)
>> +      if (cmsg->cmsg_level == SOL_SOCKET)
>>  	{
>> -	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;
>> +	  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;
>> @@ -89,7 +89,7 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>>      }
>>  
>>    msg->msg_controllen += CMSG_SPACE (sizeof tvts);
>> -  cmsg = CMSG_NXTHDR(msg, last);
>> +  cmsg = CMSG_NXTHDR (msg, last);
>>    if (cmsg == NULL)
>>      return;
>>    cmsg->cmsg_level = SOL_SOCKET;
>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>> new file mode 100644
>> index 0000000000..ae424c2a70
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>> @@ -0,0 +1 @@
>> +#include "tst-socket-timestamp.c"
>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>> new file mode 100644
>> index 0000000000..9aaa705054
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>> @@ -0,0 +1,338 @@
>> +/* Check recvmsg/recvmmsg 64-bit timestamp support.
>> +   Copyright (C) 2021 Free Software Foundation, Inc.
>> +   This file is part of the GNU C Library.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>> +   modify it under the terms of the GNU Lesser General Public
>> +   License as published by the Free Software Foundation; either
>> +   version 2.1 of the License, or (at your option) any later version.
>> +
>> +   The GNU C Library is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> +   Lesser General Public License for more details.
>> +
>> +   You should have received a copy of the GNU Lesser General Public
>> +   License along with the GNU C Library; if not, see
>> +   <https://www.gnu.org/licenses/>.  */
>> +
>> +#include <array_length.h>
>> +#include <arpa/inet.h>
>> +#include <errno.h>
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <support/check.h>
>> +#include <support/next_to_fault.h>
>> +#include <support/support.h>
>> +#include <support/test-driver.h>
>> +#include <support/xunistd.h>
>> +#include <support/xsocket.h>
>> +#include <sys/mman.h>
>> +
>> +/* Some extra space added for ancillary data, it might be used to convert
>> +   32-bit timestamp to 64-bit for _TIME_BITS=64.  */
>> +enum { slack_max_size = 64 };
>> +static const int slack[] = { 0, 4, 8, 16, 32, slack_max_size };
>> +
>> +static bool support_64_timestamp;
>> +/* AF_INET socket and address used to send and receive data.  */
>> +static int srv;
>> +static struct sockaddr_in srv_addr;
>> +
>> +static int
>> +do_sendto (const struct sockaddr_in *addr, int nmsgs)
>> +{
>> +  int s = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
>> +  xconnect (s, (const struct sockaddr *) addr, sizeof (*addr));
>> +
>> +  for (int i = 0; i < nmsgs; i++)
>> +    xsendto (s, &i, sizeof (i), 0, (const struct sockaddr *) addr,
>> +	     sizeof (*addr));
>> +
>> +  return 0;
>> +}
>> +
>> +static void
>> +do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
>> +			    size_t slack, size_t tsize, int exp_msg)
>> +{
>> +  int msg;
>> +  struct iovec iov =
>> +    {
>> +      .iov_base = &msg,
>> +      .iov_len = sizeof (msg)
>> +    };
>> +  size_t msgs = CMSG_SPACE (tsize) + slack;
>> +  struct mmsghdr mmhdr =
>> +    {
>> +      .msg_hdr =
>> +      {
>> +        .msg_name = NULL,
>> +        .msg_namelen = 0,
>> +        .msg_iov = &iov,
>> +        .msg_iovlen = 1,
>> +        .msg_controllen = msgs,
>> +        .msg_control = cmsg != NULL ? cmsg - msgs : NULL,
>> +      },
>> +    };
>> +
>> +  int r;
>> +  if (use_multi_call)
>> +    {
>> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
>> +      if (r >= 0)
>> +	r = mmhdr.msg_len;
>> +    }
>> +  else
>> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
>> +  TEST_COMPARE (r, sizeof (int));
>> +  TEST_COMPARE (msg, exp_msg);
>> +
>> +  if (cmsg == NULL)
>> +    return;
>> +
>> +  /* A timestamp is expected if 32-bit timestamp are used (support in every
>> +     configuration) or if underlying kernel support 64-bit timestamps.
>> +     Otherwise recvmsg will need extra space do add the 64-bit timestamp.  */
>> +  bool exp_timestamp;
>> +  if (sizeof (time_t) == 4 || support_64_timestamp)
>> +    exp_timestamp = true;
>> +   else
>> +    exp_timestamp = slack >= CMSG_SPACE (tsize);
>> +
>> +  bool timestamp = false;
>> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
>> +       cmsg != NULL;
>> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
>> +    {
>> +      if (cmsg->cmsg_level != SOL_SOCKET)
>> +	continue;
>> +      if (cmsg->cmsg_type == SCM_TIMESTAMP
>> +	  && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timeval)))
>> +	{
>> +	  struct timeval tv;
>> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
>> +	  if (test_verbose)
>> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
>> +		    (intmax_t)tv.tv_usec);
>> +	  timestamp = true;
>> +	}
>> +      else if (cmsg->cmsg_type == SCM_TIMESTAMPNS
>> +	       && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timespec)))
>> +	{
>> +	  struct timespec ts;
>> +	  memcpy (&ts, CMSG_DATA (cmsg), sizeof (ts));
>> +	  if (test_verbose)
>> +	    printf ("SCM_TIMESTAMPNS: {%jd, %jd}\n", (intmax_t)ts.tv_sec,
>> +		    (intmax_t)ts.tv_nsec);
>> +	  timestamp = true;
>> +	}
>> +    }
>> +
>> +  /* If there is not timestamp in the ancilliary data, recvmsg should set
>> +     the flag inidcating it.  */
>> +  if (exp_timestamp && !timestamp)
>> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
>> +  else
>> +    TEST_COMPARE (timestamp, exp_timestamp);
>> +}
>> +
>> +/* Check if the extra ancillary space is correctly handled by recvmsg and
>> +   recvmmsg: if there is no extra space for the 64-bit timestamp the syscall
>> +   should set MSG_TRUNC flag, otherwise an extra cmsghdr with the converted
>> +   timestamp is appended at the message control buffer.  */
>> +static void
>> +do_test_slack_space (void)
>> +{
>> +  /* Setup the ancillary data buffer with an extra page with PROT_NONE to
>> +     check the possible timestamp conversion on some systems.  */
>> +  struct support_next_to_fault nf =
>> +    support_next_to_fault_allocate (slack_max_size);
>> +  void *msgbuf = nf.buffer + slack_max_size;
>> +
>> +  /* Enable the timestamp using struct timeval precision.  */
>> +  {
>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
>> +			sizeof (int));
>> +    TEST_VERIFY_EXIT (r != -1);
>> +  }
>> +  /* Check recvmsg.  */
>> +  do_sendto (&srv_addr, array_length (slack));
>> +  for (int s = 0; s < array_length (slack); s++)
>> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
>> +				sizeof (struct timeval), s);
>> +  /* Check recvmmsg.  */
>> +  do_sendto (&srv_addr, array_length (slack));
>> +  for (int s = 0; s < array_length (slack); s++)
>> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
>> +				sizeof (struct timeval), s);
>> +
>> +  /* Now enable timestamp using a higher precision, it overwrites the previous
>> +     precision.  */
>> +  {
>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1},
>> +			sizeof (int));
>> +    TEST_VERIFY_EXIT (r != -1);
>> +  }
>> +  /* Check recvmsg.  */
>> +  do_sendto (&srv_addr, array_length (slack));
>> +  for (int s = 0; s < array_length (slack); s++)
>> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
>> +				sizeof (struct timespec), s);
>> +  /* Check recvmmsg.  */
>> +  do_sendto (&srv_addr, array_length (slack));
>> +  for (int s = 0; s < array_length (slack); s++)
>> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
>> +				sizeof (struct timespec), s);
>> +
>> +  support_next_to_fault_free (&nf);
>> +}
>> +
>> +/* Check if the converted 64-bit timestamp is correctly appended when there
>> +   are multiple ancillary messages.  */
>> +static void
>> +do_recvmsg_multiple_ancillary (bool use_multi_call, int s, void *cmsg,
>> +			       size_t cmsgsize, int exp_msg)
>> +{
>> +  int msg;
>> +  struct iovec iov =
>> +    {
>> +      .iov_base = &msg,
>> +      .iov_len = sizeof (msg)
>> +    };
>> +  size_t msgs = cmsgsize;
>> +  struct mmsghdr mmhdr =
>> +    {
>> +      .msg_hdr =
>> +      {
>> +        .msg_name = NULL,
>> +        .msg_namelen = 0,
>> +        .msg_iov = &iov,
>> +        .msg_iovlen = 1,
>> +        .msg_controllen = msgs,
>> +        .msg_control = cmsg,
>> +      },
>> +    };
>> +
>> +  int r;
>> +  if (use_multi_call)
>> +    {
>> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
>> +      if (r >= 0)
>> +	r = mmhdr.msg_len;
>> +    }
>> +  else
>> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
>> +  TEST_COMPARE (r, sizeof (int));
>> +  TEST_COMPARE (msg, exp_msg);
>> +
>> +  if (cmsg == NULL)
>> +    return;
>> +
>> +  bool timestamp = false;
>> +  bool origdstaddr = false;
>> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
>> +       cmsg != NULL;
>> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
>> +    {
>> +      if (cmsg->cmsg_level == SOL_IP
>> +	  && cmsg->cmsg_type == IP_ORIGDSTADDR
>> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct sockaddr_in)))
>> +	{
>> +	  struct sockaddr_in sa;
>> +	  memcpy (&sa, CMSG_DATA (cmsg), sizeof (sa));
>> +	  if (test_verbose)
>> +	    {
>> +	      char str[INET_ADDRSTRLEN];
>> +	      inet_ntop (AF_INET, &sa.sin_addr, str, INET_ADDRSTRLEN);
>> +	      printf ("IP_ORIGDSTADDR:  %s:%d\n", str, ntohs (sa.sin_port));
>> +	    }
>> +	  origdstaddr = sa.sin_addr.s_addr == srv_addr.sin_addr.s_addr
>> +			&& sa.sin_port == srv_addr.sin_port;
>> +	}
>> +      if (cmsg->cmsg_level == SOL_SOCKET
>> +	  && cmsg->cmsg_type == SCM_TIMESTAMP
>> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct timeval)))
>> +	{
>> +	  struct timeval tv;
>> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
>> +	  if (test_verbose)
>> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
>> +		    (intmax_t)tv.tv_usec);
>> +	  timestamp = true;
>> +	}
>> +    }
>> +
>> +  /* If there is no timestamp in the ancillary data, recvmsg should set
>> +     the flag to indicate it.  */
>> +  if (!timestamp)
>> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
>> +  else
>> +    TEST_COMPARE (timestamp, true);
>> +  TEST_COMPARE (origdstaddr, true);
>> +}
>> +
>> +static void
>> +do_test_multiple_ancillary (void)
>> +{
>> +  {
>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
>> +			sizeof (int));
>> +    TEST_VERIFY_EXIT (r != -1);
>> +  }
>> +  {
>> +    int r = setsockopt (srv, IPPROTO_IP, IP_RECVORIGDSTADDR, &(int){1},
>> +			sizeof (int));
>> +    TEST_VERIFY_EXIT (r != -1);
>> +  }
>> +
>> +  /* Enougth data for default SO_TIMESTAMP, the IP_RECVORIGDSTADDR, and the
>> +     extra 64-bit SO_TIMESTAMP.  */
>> +  enum { msgbuflen = CMSG_SPACE (2 * sizeof (uint64_t))
>> +		     + CMSG_SPACE (sizeof (struct sockaddr_in))
>> +		     + CMSG_SPACE (2 * sizeof (uint64_t)) };
>> +  char msgbuf[msgbuflen];
>> +
>> +  enum { nmsgs = 8 };
>> +  /* Check recvmsg.  */
>> +  do_sendto (&srv_addr, nmsgs);
>> +  for (int s = 0; s < nmsgs; s++)
>> +    do_recvmsg_multiple_ancillary (false, srv, msgbuf, msgbuflen, s);
>> +  /* Check recvmmsg.  */
>> +  do_sendto (&srv_addr, nmsgs);
>> +  for (int s = 0; s < nmsgs; s++)
>> +    do_recvmsg_multiple_ancillary (true, srv, msgbuf, msgbuflen, s);
>> +}
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  srv = xsocket (AF_INET, SOCK_DGRAM, 0);
>> +  srv_addr = (struct sockaddr_in) {
>> +    .sin_family = AF_INET,
>> +    .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) },
>> +  };
>> +  xbind (srv, (struct sockaddr *) &srv_addr, sizeof (srv_addr));
>> +  {
>> +    socklen_t sa_len = sizeof (srv_addr);
>> +    xgetsockname (srv, (struct sockaddr *) &srv_addr, &sa_len);
>> +    TEST_VERIFY (sa_len == sizeof (srv_addr));
>> +  }
>> +
>> +  TEST_COMPARE (recvmsg (-1, NULL, 0), -1);
>> +  TEST_COMPARE (errno, EBADF);
>> +  TEST_COMPARE (recvmmsg (-1, NULL, 0, 0, NULL), -1);
>> +  TEST_COMPARE (errno, EBADF);
>> +
>> +  /* If underlying kernel does not support   */
>> +  support_64_timestamp = support_socket_so_timestamp_time64 (srv);
>> +
>> +  do_test_slack_space ();
>> +  do_test_multiple_ancillary ();
>> +
>> +  xclose (srv);
>> +
>> +  return 0;
>> +}
>> +
>> +#include <support/test-driver.c>
>>

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

[Adhemerval Zanella]

Ping (x3).

On 20/10/2021 09:08, Adhemerval Zanella wrote:
> Ping (x2).
> 
> On 08/10/2021 08:58, Adhemerval Zanella wrote:
>> Ping.
>>
>> On 29/09/2021 11:06, Adhemerval Zanella wrote:
>>> The __convert_scm_timestamps() only updates the control message last
>>> pointer for SOL_SOCKET type, so if the message control buffer contains
>>> multiple ancillary message types the converted timestamp one might
>>> overwrite a valid message.
>>>
>>> The test check if the extra ancillary space is correctly handled
>>> by recvmsg/recvmmsg, where if there is no extra space for the 64-bit
>>> time_t converted message the control buffer should be marked with
>>> MSG_TRUNC.  It also check if recvmsg/recvmmsg handle correctly multiple
>>> ancillary data.
>>>
>>> Checked on x86_64-linux and on i686-linux-gnu on both 5.11 and
>>> 4.15 kernel.
>>>
>>> ---
>>> v2: Fix some typos and use support_socket_time64_timestamp.
>>>
>>> ---
>>>  sysdeps/unix/sysv/linux/Makefile              |   3 +
>>>  .../unix/sysv/linux/convert_scm_timestamps.c  |  40 +--
>>>  .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
>>>  .../unix/sysv/linux/tst-socket-timestamp.c    | 338 ++++++++++++++++++
>>>  4 files changed, 362 insertions(+), 20 deletions(-)
>>>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>>>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>>>
>>> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
>>> index 76ad06361c..3399c87468 100644
>>> --- a/sysdeps/unix/sysv/linux/Makefile
>>> +++ b/sysdeps/unix/sysv/linux/Makefile
>>> @@ -262,6 +262,9 @@ sysdep_routines += cmsg_nxthdr
>>>  CFLAGS-recvmmsg.c = -fexceptions -fasynchronous-unwind-tables
>>>  CFLAGS-sendmmsg.c = -fexceptions -fasynchronous-unwind-tables
>>>  
>>> +tests += tst-socket-timestamp
>>> +tests-time64 += tst-socket-timestamp-time64
>>> +
>>>  tests-special += $(objpfx)tst-socket-consts.out
>>>  $(objpfx)tst-socket-consts.out: ../sysdeps/unix/sysv/linux/tst-socket-consts.py
>>>  	PYTHONPATH=../scripts \
>>> 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)
>>> -	continue;
>>> -
>>> -      switch (cmsg->cmsg_type)
>>> +      if (cmsg->cmsg_level == SOL_SOCKET)
>>>  	{
>>> -	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;
>>> +	  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;
>>> @@ -89,7 +89,7 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>>>      }
>>>  
>>>    msg->msg_controllen += CMSG_SPACE (sizeof tvts);
>>> -  cmsg = CMSG_NXTHDR(msg, last);
>>> +  cmsg = CMSG_NXTHDR (msg, last);
>>>    if (cmsg == NULL)
>>>      return;
>>>    cmsg->cmsg_level = SOL_SOCKET;
>>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>>> new file mode 100644
>>> index 0000000000..ae424c2a70
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>>> @@ -0,0 +1 @@
>>> +#include "tst-socket-timestamp.c"
>>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>>> new file mode 100644
>>> index 0000000000..9aaa705054
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>>> @@ -0,0 +1,338 @@
>>> +/* Check recvmsg/recvmmsg 64-bit timestamp support.
>>> +   Copyright (C) 2021 Free Software Foundation, Inc.
>>> +   This file is part of the GNU C Library.
>>> +
>>> +   The GNU C Library is free software; you can redistribute it and/or
>>> +   modify it under the terms of the GNU Lesser General Public
>>> +   License as published by the Free Software Foundation; either
>>> +   version 2.1 of the License, or (at your option) any later version.
>>> +
>>> +   The GNU C Library is distributed in the hope that it will be useful,
>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +   Lesser General Public License for more details.
>>> +
>>> +   You should have received a copy of the GNU Lesser General Public
>>> +   License along with the GNU C Library; if not, see
>>> +   <https://www.gnu.org/licenses/>.  */
>>> +
>>> +#include <array_length.h>
>>> +#include <arpa/inet.h>
>>> +#include <errno.h>
>>> +#include <string.h>
>>> +#include <stdio.h>
>>> +#include <support/check.h>
>>> +#include <support/next_to_fault.h>
>>> +#include <support/support.h>
>>> +#include <support/test-driver.h>
>>> +#include <support/xunistd.h>
>>> +#include <support/xsocket.h>
>>> +#include <sys/mman.h>
>>> +
>>> +/* Some extra space added for ancillary data, it might be used to convert
>>> +   32-bit timestamp to 64-bit for _TIME_BITS=64.  */
>>> +enum { slack_max_size = 64 };
>>> +static const int slack[] = { 0, 4, 8, 16, 32, slack_max_size };
>>> +
>>> +static bool support_64_timestamp;
>>> +/* AF_INET socket and address used to send and receive data.  */
>>> +static int srv;
>>> +static struct sockaddr_in srv_addr;
>>> +
>>> +static int
>>> +do_sendto (const struct sockaddr_in *addr, int nmsgs)
>>> +{
>>> +  int s = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
>>> +  xconnect (s, (const struct sockaddr *) addr, sizeof (*addr));
>>> +
>>> +  for (int i = 0; i < nmsgs; i++)
>>> +    xsendto (s, &i, sizeof (i), 0, (const struct sockaddr *) addr,
>>> +	     sizeof (*addr));
>>> +
>>> +  return 0;
>>> +}
>>> +
>>> +static void
>>> +do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
>>> +			    size_t slack, size_t tsize, int exp_msg)
>>> +{
>>> +  int msg;
>>> +  struct iovec iov =
>>> +    {
>>> +      .iov_base = &msg,
>>> +      .iov_len = sizeof (msg)
>>> +    };
>>> +  size_t msgs = CMSG_SPACE (tsize) + slack;
>>> +  struct mmsghdr mmhdr =
>>> +    {
>>> +      .msg_hdr =
>>> +      {
>>> +        .msg_name = NULL,
>>> +        .msg_namelen = 0,
>>> +        .msg_iov = &iov,
>>> +        .msg_iovlen = 1,
>>> +        .msg_controllen = msgs,
>>> +        .msg_control = cmsg != NULL ? cmsg - msgs : NULL,
>>> +      },
>>> +    };
>>> +
>>> +  int r;
>>> +  if (use_multi_call)
>>> +    {
>>> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
>>> +      if (r >= 0)
>>> +	r = mmhdr.msg_len;
>>> +    }
>>> +  else
>>> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
>>> +  TEST_COMPARE (r, sizeof (int));
>>> +  TEST_COMPARE (msg, exp_msg);
>>> +
>>> +  if (cmsg == NULL)
>>> +    return;
>>> +
>>> +  /* A timestamp is expected if 32-bit timestamp are used (support in every
>>> +     configuration) or if underlying kernel support 64-bit timestamps.
>>> +     Otherwise recvmsg will need extra space do add the 64-bit timestamp.  */
>>> +  bool exp_timestamp;
>>> +  if (sizeof (time_t) == 4 || support_64_timestamp)
>>> +    exp_timestamp = true;
>>> +   else
>>> +    exp_timestamp = slack >= CMSG_SPACE (tsize);
>>> +
>>> +  bool timestamp = false;
>>> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
>>> +       cmsg != NULL;
>>> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
>>> +    {
>>> +      if (cmsg->cmsg_level != SOL_SOCKET)
>>> +	continue;
>>> +      if (cmsg->cmsg_type == SCM_TIMESTAMP
>>> +	  && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timeval)))
>>> +	{
>>> +	  struct timeval tv;
>>> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
>>> +	  if (test_verbose)
>>> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
>>> +		    (intmax_t)tv.tv_usec);
>>> +	  timestamp = true;
>>> +	}
>>> +      else if (cmsg->cmsg_type == SCM_TIMESTAMPNS
>>> +	       && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timespec)))
>>> +	{
>>> +	  struct timespec ts;
>>> +	  memcpy (&ts, CMSG_DATA (cmsg), sizeof (ts));
>>> +	  if (test_verbose)
>>> +	    printf ("SCM_TIMESTAMPNS: {%jd, %jd}\n", (intmax_t)ts.tv_sec,
>>> +		    (intmax_t)ts.tv_nsec);
>>> +	  timestamp = true;
>>> +	}
>>> +    }
>>> +
>>> +  /* If there is not timestamp in the ancilliary data, recvmsg should set
>>> +     the flag inidcating it.  */
>>> +  if (exp_timestamp && !timestamp)
>>> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
>>> +  else
>>> +    TEST_COMPARE (timestamp, exp_timestamp);
>>> +}
>>> +
>>> +/* Check if the extra ancillary space is correctly handled by recvmsg and
>>> +   recvmmsg: if there is no extra space for the 64-bit timestamp the syscall
>>> +   should set MSG_TRUNC flag, otherwise an extra cmsghdr with the converted
>>> +   timestamp is appended at the message control buffer.  */
>>> +static void
>>> +do_test_slack_space (void)
>>> +{
>>> +  /* Setup the ancillary data buffer with an extra page with PROT_NONE to
>>> +     check the possible timestamp conversion on some systems.  */
>>> +  struct support_next_to_fault nf =
>>> +    support_next_to_fault_allocate (slack_max_size);
>>> +  void *msgbuf = nf.buffer + slack_max_size;
>>> +
>>> +  /* Enable the timestamp using struct timeval precision.  */
>>> +  {
>>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
>>> +			sizeof (int));
>>> +    TEST_VERIFY_EXIT (r != -1);
>>> +  }
>>> +  /* Check recvmsg.  */
>>> +  do_sendto (&srv_addr, array_length (slack));
>>> +  for (int s = 0; s < array_length (slack); s++)
>>> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
>>> +				sizeof (struct timeval), s);
>>> +  /* Check recvmmsg.  */
>>> +  do_sendto (&srv_addr, array_length (slack));
>>> +  for (int s = 0; s < array_length (slack); s++)
>>> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
>>> +				sizeof (struct timeval), s);
>>> +
>>> +  /* Now enable timestamp using a higher precision, it overwrites the previous
>>> +     precision.  */
>>> +  {
>>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1},
>>> +			sizeof (int));
>>> +    TEST_VERIFY_EXIT (r != -1);
>>> +  }
>>> +  /* Check recvmsg.  */
>>> +  do_sendto (&srv_addr, array_length (slack));
>>> +  for (int s = 0; s < array_length (slack); s++)
>>> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
>>> +				sizeof (struct timespec), s);
>>> +  /* Check recvmmsg.  */
>>> +  do_sendto (&srv_addr, array_length (slack));
>>> +  for (int s = 0; s < array_length (slack); s++)
>>> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
>>> +				sizeof (struct timespec), s);
>>> +
>>> +  support_next_to_fault_free (&nf);
>>> +}
>>> +
>>> +/* Check if the converted 64-bit timestamp is correctly appended when there
>>> +   are multiple ancillary messages.  */
>>> +static void
>>> +do_recvmsg_multiple_ancillary (bool use_multi_call, int s, void *cmsg,
>>> +			       size_t cmsgsize, int exp_msg)
>>> +{
>>> +  int msg;
>>> +  struct iovec iov =
>>> +    {
>>> +      .iov_base = &msg,
>>> +      .iov_len = sizeof (msg)
>>> +    };
>>> +  size_t msgs = cmsgsize;
>>> +  struct mmsghdr mmhdr =
>>> +    {
>>> +      .msg_hdr =
>>> +      {
>>> +        .msg_name = NULL,
>>> +        .msg_namelen = 0,
>>> +        .msg_iov = &iov,
>>> +        .msg_iovlen = 1,
>>> +        .msg_controllen = msgs,
>>> +        .msg_control = cmsg,
>>> +      },
>>> +    };
>>> +
>>> +  int r;
>>> +  if (use_multi_call)
>>> +    {
>>> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
>>> +      if (r >= 0)
>>> +	r = mmhdr.msg_len;
>>> +    }
>>> +  else
>>> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
>>> +  TEST_COMPARE (r, sizeof (int));
>>> +  TEST_COMPARE (msg, exp_msg);
>>> +
>>> +  if (cmsg == NULL)
>>> +    return;
>>> +
>>> +  bool timestamp = false;
>>> +  bool origdstaddr = false;
>>> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
>>> +       cmsg != NULL;
>>> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
>>> +    {
>>> +      if (cmsg->cmsg_level == SOL_IP
>>> +	  && cmsg->cmsg_type == IP_ORIGDSTADDR
>>> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct sockaddr_in)))
>>> +	{
>>> +	  struct sockaddr_in sa;
>>> +	  memcpy (&sa, CMSG_DATA (cmsg), sizeof (sa));
>>> +	  if (test_verbose)
>>> +	    {
>>> +	      char str[INET_ADDRSTRLEN];
>>> +	      inet_ntop (AF_INET, &sa.sin_addr, str, INET_ADDRSTRLEN);
>>> +	      printf ("IP_ORIGDSTADDR:  %s:%d\n", str, ntohs (sa.sin_port));
>>> +	    }
>>> +	  origdstaddr = sa.sin_addr.s_addr == srv_addr.sin_addr.s_addr
>>> +			&& sa.sin_port == srv_addr.sin_port;
>>> +	}
>>> +      if (cmsg->cmsg_level == SOL_SOCKET
>>> +	  && cmsg->cmsg_type == SCM_TIMESTAMP
>>> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct timeval)))
>>> +	{
>>> +	  struct timeval tv;
>>> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
>>> +	  if (test_verbose)
>>> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
>>> +		    (intmax_t)tv.tv_usec);
>>> +	  timestamp = true;
>>> +	}
>>> +    }
>>> +
>>> +  /* If there is no timestamp in the ancillary data, recvmsg should set
>>> +     the flag to indicate it.  */
>>> +  if (!timestamp)
>>> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
>>> +  else
>>> +    TEST_COMPARE (timestamp, true);
>>> +  TEST_COMPARE (origdstaddr, true);
>>> +}
>>> +
>>> +static void
>>> +do_test_multiple_ancillary (void)
>>> +{
>>> +  {
>>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
>>> +			sizeof (int));
>>> +    TEST_VERIFY_EXIT (r != -1);
>>> +  }
>>> +  {
>>> +    int r = setsockopt (srv, IPPROTO_IP, IP_RECVORIGDSTADDR, &(int){1},
>>> +			sizeof (int));
>>> +    TEST_VERIFY_EXIT (r != -1);
>>> +  }
>>> +
>>> +  /* Enougth data for default SO_TIMESTAMP, the IP_RECVORIGDSTADDR, and the
>>> +     extra 64-bit SO_TIMESTAMP.  */
>>> +  enum { msgbuflen = CMSG_SPACE (2 * sizeof (uint64_t))
>>> +		     + CMSG_SPACE (sizeof (struct sockaddr_in))
>>> +		     + CMSG_SPACE (2 * sizeof (uint64_t)) };
>>> +  char msgbuf[msgbuflen];
>>> +
>>> +  enum { nmsgs = 8 };
>>> +  /* Check recvmsg.  */
>>> +  do_sendto (&srv_addr, nmsgs);
>>> +  for (int s = 0; s < nmsgs; s++)
>>> +    do_recvmsg_multiple_ancillary (false, srv, msgbuf, msgbuflen, s);
>>> +  /* Check recvmmsg.  */
>>> +  do_sendto (&srv_addr, nmsgs);
>>> +  for (int s = 0; s < nmsgs; s++)
>>> +    do_recvmsg_multiple_ancillary (true, srv, msgbuf, msgbuflen, s);
>>> +}
>>> +
>>> +static int
>>> +do_test (void)
>>> +{
>>> +  srv = xsocket (AF_INET, SOCK_DGRAM, 0);
>>> +  srv_addr = (struct sockaddr_in) {
>>> +    .sin_family = AF_INET,
>>> +    .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) },
>>> +  };
>>> +  xbind (srv, (struct sockaddr *) &srv_addr, sizeof (srv_addr));
>>> +  {
>>> +    socklen_t sa_len = sizeof (srv_addr);
>>> +    xgetsockname (srv, (struct sockaddr *) &srv_addr, &sa_len);
>>> +    TEST_VERIFY (sa_len == sizeof (srv_addr));
>>> +  }
>>> +
>>> +  TEST_COMPARE (recvmsg (-1, NULL, 0), -1);
>>> +  TEST_COMPARE (errno, EBADF);
>>> +  TEST_COMPARE (recvmmsg (-1, NULL, 0, 0, NULL), -1);
>>> +  TEST_COMPARE (errno, EBADF);
>>> +
>>> +  /* If underlying kernel does not support   */
>>> +  support_64_timestamp = support_socket_so_timestamp_time64 (srv);
>>> +
>>> +  do_test_slack_space ();
>>> +  do_test_multiple_ancillary ();
>>> +
>>> +  xclose (srv);
>>> +
>>> +  return 0;
>>> +}
>>> +
>>> +#include <support/test-driver.c>
>>>

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

[Adhemerval Zanella]

Ping (x4), this seems to be an important fix for 64-bit time_t.

On 08/11/2021 15:30, Adhemerval Zanella wrote:
> Ping (x3).
> 
> On 20/10/2021 09:08, Adhemerval Zanella wrote:
>> Ping (x2).
>>
>> On 08/10/2021 08:58, Adhemerval Zanella wrote:
>>> Ping.
>>>
>>> On 29/09/2021 11:06, Adhemerval Zanella wrote:
>>>> The __convert_scm_timestamps() only updates the control message last
>>>> pointer for SOL_SOCKET type, so if the message control buffer contains
>>>> multiple ancillary message types the converted timestamp one might
>>>> overwrite a valid message.
>>>>
>>>> The test check if the extra ancillary space is correctly handled
>>>> by recvmsg/recvmmsg, where if there is no extra space for the 64-bit
>>>> time_t converted message the control buffer should be marked with
>>>> MSG_TRUNC.  It also check if recvmsg/recvmmsg handle correctly multiple
>>>> ancillary data.
>>>>
>>>> Checked on x86_64-linux and on i686-linux-gnu on both 5.11 and
>>>> 4.15 kernel.
>>>>
>>>> ---
>>>> v2: Fix some typos and use support_socket_time64_timestamp.
>>>>
>>>> ---
>>>>  sysdeps/unix/sysv/linux/Makefile              |   3 +
>>>>  .../unix/sysv/linux/convert_scm_timestamps.c  |  40 +--
>>>>  .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
>>>>  .../unix/sysv/linux/tst-socket-timestamp.c    | 338 ++++++++++++++++++
>>>>  4 files changed, 362 insertions(+), 20 deletions(-)
>>>>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>>>>  create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>>>>
>>>> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
>>>> index 76ad06361c..3399c87468 100644
>>>> --- a/sysdeps/unix/sysv/linux/Makefile
>>>> +++ b/sysdeps/unix/sysv/linux/Makefile
>>>> @@ -262,6 +262,9 @@ sysdep_routines += cmsg_nxthdr
>>>>  CFLAGS-recvmmsg.c = -fexceptions -fasynchronous-unwind-tables
>>>>  CFLAGS-sendmmsg.c = -fexceptions -fasynchronous-unwind-tables
>>>>  
>>>> +tests += tst-socket-timestamp
>>>> +tests-time64 += tst-socket-timestamp-time64
>>>> +
>>>>  tests-special += $(objpfx)tst-socket-consts.out
>>>>  $(objpfx)tst-socket-consts.out: ../sysdeps/unix/sysv/linux/tst-socket-consts.py
>>>>  	PYTHONPATH=../scripts \
>>>> 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)
>>>> -	continue;
>>>> -
>>>> -      switch (cmsg->cmsg_type)
>>>> +      if (cmsg->cmsg_level == SOL_SOCKET)
>>>>  	{
>>>> -	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;
>>>> +	  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;
>>>> @@ -89,7 +89,7 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
>>>>      }
>>>>  
>>>>    msg->msg_controllen += CMSG_SPACE (sizeof tvts);
>>>> -  cmsg = CMSG_NXTHDR(msg, last);
>>>> +  cmsg = CMSG_NXTHDR (msg, last);
>>>>    if (cmsg == NULL)
>>>>      return;
>>>>    cmsg->cmsg_level = SOL_SOCKET;
>>>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>>>> new file mode 100644
>>>> index 0000000000..ae424c2a70
>>>> --- /dev/null
>>>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
>>>> @@ -0,0 +1 @@
>>>> +#include "tst-socket-timestamp.c"
>>>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>>>> new file mode 100644
>>>> index 0000000000..9aaa705054
>>>> --- /dev/null
>>>> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
>>>> @@ -0,0 +1,338 @@
>>>> +/* Check recvmsg/recvmmsg 64-bit timestamp support.
>>>> +   Copyright (C) 2021 Free Software Foundation, Inc.
>>>> +   This file is part of the GNU C Library.
>>>> +
>>>> +   The GNU C Library is free software; you can redistribute it and/or
>>>> +   modify it under the terms of the GNU Lesser General Public
>>>> +   License as published by the Free Software Foundation; either
>>>> +   version 2.1 of the License, or (at your option) any later version.
>>>> +
>>>> +   The GNU C Library is distributed in the hope that it will be useful,
>>>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> +   Lesser General Public License for more details.
>>>> +
>>>> +   You should have received a copy of the GNU Lesser General Public
>>>> +   License along with the GNU C Library; if not, see
>>>> +   <https://www.gnu.org/licenses/>.  */
>>>> +
>>>> +#include <array_length.h>
>>>> +#include <arpa/inet.h>
>>>> +#include <errno.h>
>>>> +#include <string.h>
>>>> +#include <stdio.h>
>>>> +#include <support/check.h>
>>>> +#include <support/next_to_fault.h>
>>>> +#include <support/support.h>
>>>> +#include <support/test-driver.h>
>>>> +#include <support/xunistd.h>
>>>> +#include <support/xsocket.h>
>>>> +#include <sys/mman.h>
>>>> +
>>>> +/* Some extra space added for ancillary data, it might be used to convert
>>>> +   32-bit timestamp to 64-bit for _TIME_BITS=64.  */
>>>> +enum { slack_max_size = 64 };
>>>> +static const int slack[] = { 0, 4, 8, 16, 32, slack_max_size };
>>>> +
>>>> +static bool support_64_timestamp;
>>>> +/* AF_INET socket and address used to send and receive data.  */
>>>> +static int srv;
>>>> +static struct sockaddr_in srv_addr;
>>>> +
>>>> +static int
>>>> +do_sendto (const struct sockaddr_in *addr, int nmsgs)
>>>> +{
>>>> +  int s = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
>>>> +  xconnect (s, (const struct sockaddr *) addr, sizeof (*addr));
>>>> +
>>>> +  for (int i = 0; i < nmsgs; i++)
>>>> +    xsendto (s, &i, sizeof (i), 0, (const struct sockaddr *) addr,
>>>> +	     sizeof (*addr));
>>>> +
>>>> +  return 0;
>>>> +}
>>>> +
>>>> +static void
>>>> +do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
>>>> +			    size_t slack, size_t tsize, int exp_msg)
>>>> +{
>>>> +  int msg;
>>>> +  struct iovec iov =
>>>> +    {
>>>> +      .iov_base = &msg,
>>>> +      .iov_len = sizeof (msg)
>>>> +    };
>>>> +  size_t msgs = CMSG_SPACE (tsize) + slack;
>>>> +  struct mmsghdr mmhdr =
>>>> +    {
>>>> +      .msg_hdr =
>>>> +      {
>>>> +        .msg_name = NULL,
>>>> +        .msg_namelen = 0,
>>>> +        .msg_iov = &iov,
>>>> +        .msg_iovlen = 1,
>>>> +        .msg_controllen = msgs,
>>>> +        .msg_control = cmsg != NULL ? cmsg - msgs : NULL,
>>>> +      },
>>>> +    };
>>>> +
>>>> +  int r;
>>>> +  if (use_multi_call)
>>>> +    {
>>>> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
>>>> +      if (r >= 0)
>>>> +	r = mmhdr.msg_len;
>>>> +    }
>>>> +  else
>>>> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
>>>> +  TEST_COMPARE (r, sizeof (int));
>>>> +  TEST_COMPARE (msg, exp_msg);
>>>> +
>>>> +  if (cmsg == NULL)
>>>> +    return;
>>>> +
>>>> +  /* A timestamp is expected if 32-bit timestamp are used (support in every
>>>> +     configuration) or if underlying kernel support 64-bit timestamps.
>>>> +     Otherwise recvmsg will need extra space do add the 64-bit timestamp.  */
>>>> +  bool exp_timestamp;
>>>> +  if (sizeof (time_t) == 4 || support_64_timestamp)
>>>> +    exp_timestamp = true;
>>>> +   else
>>>> +    exp_timestamp = slack >= CMSG_SPACE (tsize);
>>>> +
>>>> +  bool timestamp = false;
>>>> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
>>>> +       cmsg != NULL;
>>>> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
>>>> +    {
>>>> +      if (cmsg->cmsg_level != SOL_SOCKET)
>>>> +	continue;
>>>> +      if (cmsg->cmsg_type == SCM_TIMESTAMP
>>>> +	  && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timeval)))
>>>> +	{
>>>> +	  struct timeval tv;
>>>> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
>>>> +	  if (test_verbose)
>>>> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
>>>> +		    (intmax_t)tv.tv_usec);
>>>> +	  timestamp = true;
>>>> +	}
>>>> +      else if (cmsg->cmsg_type == SCM_TIMESTAMPNS
>>>> +	       && cmsg->cmsg_len == CMSG_LEN (sizeof (struct timespec)))
>>>> +	{
>>>> +	  struct timespec ts;
>>>> +	  memcpy (&ts, CMSG_DATA (cmsg), sizeof (ts));
>>>> +	  if (test_verbose)
>>>> +	    printf ("SCM_TIMESTAMPNS: {%jd, %jd}\n", (intmax_t)ts.tv_sec,
>>>> +		    (intmax_t)ts.tv_nsec);
>>>> +	  timestamp = true;
>>>> +	}
>>>> +    }
>>>> +
>>>> +  /* If there is not timestamp in the ancilliary data, recvmsg should set
>>>> +     the flag inidcating it.  */
>>>> +  if (exp_timestamp && !timestamp)
>>>> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
>>>> +  else
>>>> +    TEST_COMPARE (timestamp, exp_timestamp);
>>>> +}
>>>> +
>>>> +/* Check if the extra ancillary space is correctly handled by recvmsg and
>>>> +   recvmmsg: if there is no extra space for the 64-bit timestamp the syscall
>>>> +   should set MSG_TRUNC flag, otherwise an extra cmsghdr with the converted
>>>> +   timestamp is appended at the message control buffer.  */
>>>> +static void
>>>> +do_test_slack_space (void)
>>>> +{
>>>> +  /* Setup the ancillary data buffer with an extra page with PROT_NONE to
>>>> +     check the possible timestamp conversion on some systems.  */
>>>> +  struct support_next_to_fault nf =
>>>> +    support_next_to_fault_allocate (slack_max_size);
>>>> +  void *msgbuf = nf.buffer + slack_max_size;
>>>> +
>>>> +  /* Enable the timestamp using struct timeval precision.  */
>>>> +  {
>>>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
>>>> +			sizeof (int));
>>>> +    TEST_VERIFY_EXIT (r != -1);
>>>> +  }
>>>> +  /* Check recvmsg.  */
>>>> +  do_sendto (&srv_addr, array_length (slack));
>>>> +  for (int s = 0; s < array_length (slack); s++)
>>>> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
>>>> +				sizeof (struct timeval), s);
>>>> +  /* Check recvmmsg.  */
>>>> +  do_sendto (&srv_addr, array_length (slack));
>>>> +  for (int s = 0; s < array_length (slack); s++)
>>>> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
>>>> +				sizeof (struct timeval), s);
>>>> +
>>>> +  /* Now enable timestamp using a higher precision, it overwrites the previous
>>>> +     precision.  */
>>>> +  {
>>>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS, &(int){1},
>>>> +			sizeof (int));
>>>> +    TEST_VERIFY_EXIT (r != -1);
>>>> +  }
>>>> +  /* Check recvmsg.  */
>>>> +  do_sendto (&srv_addr, array_length (slack));
>>>> +  for (int s = 0; s < array_length (slack); s++)
>>>> +    do_recvmsg_slack_ancillary (false, srv, msgbuf, slack[s],
>>>> +				sizeof (struct timespec), s);
>>>> +  /* Check recvmmsg.  */
>>>> +  do_sendto (&srv_addr, array_length (slack));
>>>> +  for (int s = 0; s < array_length (slack); s++)
>>>> +    do_recvmsg_slack_ancillary (true, srv, msgbuf, slack[s],
>>>> +				sizeof (struct timespec), s);
>>>> +
>>>> +  support_next_to_fault_free (&nf);
>>>> +}
>>>> +
>>>> +/* Check if the converted 64-bit timestamp is correctly appended when there
>>>> +   are multiple ancillary messages.  */
>>>> +static void
>>>> +do_recvmsg_multiple_ancillary (bool use_multi_call, int s, void *cmsg,
>>>> +			       size_t cmsgsize, int exp_msg)
>>>> +{
>>>> +  int msg;
>>>> +  struct iovec iov =
>>>> +    {
>>>> +      .iov_base = &msg,
>>>> +      .iov_len = sizeof (msg)
>>>> +    };
>>>> +  size_t msgs = cmsgsize;
>>>> +  struct mmsghdr mmhdr =
>>>> +    {
>>>> +      .msg_hdr =
>>>> +      {
>>>> +        .msg_name = NULL,
>>>> +        .msg_namelen = 0,
>>>> +        .msg_iov = &iov,
>>>> +        .msg_iovlen = 1,
>>>> +        .msg_controllen = msgs,
>>>> +        .msg_control = cmsg,
>>>> +      },
>>>> +    };
>>>> +
>>>> +  int r;
>>>> +  if (use_multi_call)
>>>> +    {
>>>> +      r = recvmmsg (s, &mmhdr, 1, 0, NULL);
>>>> +      if (r >= 0)
>>>> +	r = mmhdr.msg_len;
>>>> +    }
>>>> +  else
>>>> +    r = recvmsg (s, &mmhdr.msg_hdr, 0);
>>>> +  TEST_COMPARE (r, sizeof (int));
>>>> +  TEST_COMPARE (msg, exp_msg);
>>>> +
>>>> +  if (cmsg == NULL)
>>>> +    return;
>>>> +
>>>> +  bool timestamp = false;
>>>> +  bool origdstaddr = false;
>>>> +  for (struct cmsghdr *cmsg = CMSG_FIRSTHDR (&mmhdr.msg_hdr);
>>>> +       cmsg != NULL;
>>>> +       cmsg = CMSG_NXTHDR (&mmhdr.msg_hdr, cmsg))
>>>> +    {
>>>> +      if (cmsg->cmsg_level == SOL_IP
>>>> +	  && cmsg->cmsg_type == IP_ORIGDSTADDR
>>>> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct sockaddr_in)))
>>>> +	{
>>>> +	  struct sockaddr_in sa;
>>>> +	  memcpy (&sa, CMSG_DATA (cmsg), sizeof (sa));
>>>> +	  if (test_verbose)
>>>> +	    {
>>>> +	      char str[INET_ADDRSTRLEN];
>>>> +	      inet_ntop (AF_INET, &sa.sin_addr, str, INET_ADDRSTRLEN);
>>>> +	      printf ("IP_ORIGDSTADDR:  %s:%d\n", str, ntohs (sa.sin_port));
>>>> +	    }
>>>> +	  origdstaddr = sa.sin_addr.s_addr == srv_addr.sin_addr.s_addr
>>>> +			&& sa.sin_port == srv_addr.sin_port;
>>>> +	}
>>>> +      if (cmsg->cmsg_level == SOL_SOCKET
>>>> +	  && cmsg->cmsg_type == SCM_TIMESTAMP
>>>> +	  && cmsg->cmsg_len >= CMSG_LEN (sizeof (struct timeval)))
>>>> +	{
>>>> +	  struct timeval tv;
>>>> +	  memcpy (&tv, CMSG_DATA (cmsg), sizeof (tv));
>>>> +	  if (test_verbose)
>>>> +	    printf ("SCM_TIMESTAMP:   {%jd, %jd}\n", (intmax_t)tv.tv_sec,
>>>> +		    (intmax_t)tv.tv_usec);
>>>> +	  timestamp = true;
>>>> +	}
>>>> +    }
>>>> +
>>>> +  /* If there is no timestamp in the ancillary data, recvmsg should set
>>>> +     the flag to indicate it.  */
>>>> +  if (!timestamp)
>>>> +    TEST_VERIFY (mmhdr.msg_hdr.msg_flags & MSG_TRUNC);
>>>> +  else
>>>> +    TEST_COMPARE (timestamp, true);
>>>> +  TEST_COMPARE (origdstaddr, true);
>>>> +}
>>>> +
>>>> +static void
>>>> +do_test_multiple_ancillary (void)
>>>> +{
>>>> +  {
>>>> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP, &(int){1},
>>>> +			sizeof (int));
>>>> +    TEST_VERIFY_EXIT (r != -1);
>>>> +  }
>>>> +  {
>>>> +    int r = setsockopt (srv, IPPROTO_IP, IP_RECVORIGDSTADDR, &(int){1},
>>>> +			sizeof (int));
>>>> +    TEST_VERIFY_EXIT (r != -1);
>>>> +  }
>>>> +
>>>> +  /* Enougth data for default SO_TIMESTAMP, the IP_RECVORIGDSTADDR, and the
>>>> +     extra 64-bit SO_TIMESTAMP.  */
>>>> +  enum { msgbuflen = CMSG_SPACE (2 * sizeof (uint64_t))
>>>> +		     + CMSG_SPACE (sizeof (struct sockaddr_in))
>>>> +		     + CMSG_SPACE (2 * sizeof (uint64_t)) };
>>>> +  char msgbuf[msgbuflen];
>>>> +
>>>> +  enum { nmsgs = 8 };
>>>> +  /* Check recvmsg.  */
>>>> +  do_sendto (&srv_addr, nmsgs);
>>>> +  for (int s = 0; s < nmsgs; s++)
>>>> +    do_recvmsg_multiple_ancillary (false, srv, msgbuf, msgbuflen, s);
>>>> +  /* Check recvmmsg.  */
>>>> +  do_sendto (&srv_addr, nmsgs);
>>>> +  for (int s = 0; s < nmsgs; s++)
>>>> +    do_recvmsg_multiple_ancillary (true, srv, msgbuf, msgbuflen, s);
>>>> +}
>>>> +
>>>> +static int
>>>> +do_test (void)
>>>> +{
>>>> +  srv = xsocket (AF_INET, SOCK_DGRAM, 0);
>>>> +  srv_addr = (struct sockaddr_in) {
>>>> +    .sin_family = AF_INET,
>>>> +    .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) },
>>>> +  };
>>>> +  xbind (srv, (struct sockaddr *) &srv_addr, sizeof (srv_addr));
>>>> +  {
>>>> +    socklen_t sa_len = sizeof (srv_addr);
>>>> +    xgetsockname (srv, (struct sockaddr *) &srv_addr, &sa_len);
>>>> +    TEST_VERIFY (sa_len == sizeof (srv_addr));
>>>> +  }
>>>> +
>>>> +  TEST_COMPARE (recvmsg (-1, NULL, 0), -1);
>>>> +  TEST_COMPARE (errno, EBADF);
>>>> +  TEST_COMPARE (recvmmsg (-1, NULL, 0, 0, NULL), -1);
>>>> +  TEST_COMPARE (errno, EBADF);
>>>> +
>>>> +  /* If underlying kernel does not support   */
>>>> +  support_64_timestamp = support_socket_so_timestamp_time64 (srv);
>>>> +
>>>> +  do_test_slack_space ();
>>>> +  do_test_multiple_ancillary ();
>>>> +
>>>> +  xclose (srv);
>>>> +
>>>> +  return 0;
>>>> +}
>>>> +
>>>> +#include <support/test-driver.c>
>>>>

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

[Florian Weimer]

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

Thanks,
Florian

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

[Adhemerval Zanella]

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?

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

[Florian Weimer]

* Adhemerval Zanella:

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

Let me read it again …

Thanks,
Florian

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

[Florian Weimer]

* Florian Weimer:

> * Adhemerval Zanella:
>
>> 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?
>
> Let me read it again …

I don't see how we patch anything if CMSG_NEXT (msg, last) is null.

Thanks,
Florian

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

[Adhemerval Zanella]

On 10/12/2021 09:59, Florian Weimer wrote:
> * Florian Weimer:
> 
>> * Adhemerval Zanella:
>>
>>> 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?
>>
>> Let me read it again …
> 
> I don't see how we patch anything if CMSG_NEXT (msg, last) is null.

The issue is the 'last = cmsg' not being updated for 'cmsg->cmsg_level != SOL_SOCKET'.

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

[Florian Weimer]

* Adhemerval Zanella:

> On 10/12/2021 09:59, Florian Weimer wrote:
>> * Florian Weimer:
>> 
>>> * Adhemerval Zanella:
>>>
>>>> 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?
>>>
>>> Let me read it again …
>> 
>> I don't see how we patch anything if CMSG_NEXT (msg, last) is null.
>
> The issue is the 'last = cmsg' not being updated for 'cmsg->cmsg_level
> != SOL_SOCKET'.

Sorry, I don't understand.  Does this mean that the patch needs fixing?

Thanks,
Florian

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

[Adhemerval Zanella]

On 10/12/2021 10:38, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 10/12/2021 09:59, Florian Weimer wrote:
>>> * Florian Weimer:
>>>
>>>> * Adhemerval Zanella:
>>>>
>>>>> 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?
>>>>
>>>> Let me read it again …
>>>
>>> I don't see how we patch anything if CMSG_NEXT (msg, last) is null.
>>
>> The issue is the 'last = cmsg' not being updated for 'cmsg->cmsg_level
>> != SOL_SOCKET'.
> 
> Sorry, I don't understand.  Does this mean that the patch needs fixing?

I think the indentation change made the patch hard to read, the issues is
currently we have:

  for (cmsg = CMSG_FIRSTHDR (msg);
       cmsg != NULL;
       cmsg = CMSG_NXTHDR (msg, cmsg))
    {
      if (cmsg->cmsg_level != SOL_SOCKET)
        continue;
      [...]
      last = cmsg;
    }

Where we need 'last' to updated regardless of 'cmsg_level':

  for (cmsg = CMSG_FIRSTHDR (msg);
       cmsg != NULL;
       cmsg = CMSG_NXTHDR (msg, cmsg))
    {
      if (cmsg->cmsg_level == SOL_SOCKET)
        [...]
      last = cmsg;
    }

The 'cmsg == NULL' at the end that you spotted is just an optimization.

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

[Andreas Schwab]

On Dez 10 2021, Adhemerval Zanella via Libc-alpha wrote:

> I think the indentation change made the patch hard to read, the issues is
> currently we have:
>
>   for (cmsg = CMSG_FIRSTHDR (msg);
>        cmsg != NULL;
>        cmsg = CMSG_NXTHDR (msg, cmsg))
>     {
>       if (cmsg->cmsg_level != SOL_SOCKET)
>         continue;
>       [...]
>       last = cmsg;
>     }
>
> Where we need 'last' to updated regardless of 'cmsg_level':
>
>   for (cmsg = CMSG_FIRSTHDR (msg);
>        cmsg != NULL;
>        cmsg = CMSG_NXTHDR (msg, cmsg))
>     {
>       if (cmsg->cmsg_level == SOL_SOCKET)
>         [...]
>       last = cmsg;
>     }

You could move the assignment in the loop header so that continue
continues to work.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

[Adhemerval Zanella]

On 10/12/2021 10:53, Andreas Schwab wrote:
> On Dez 10 2021, Adhemerval Zanella via Libc-alpha wrote:
> 
>> I think the indentation change made the patch hard to read, the issues is
>> currently we have:
>>
>>   for (cmsg = CMSG_FIRSTHDR (msg);
>>        cmsg != NULL;
>>        cmsg = CMSG_NXTHDR (msg, cmsg))
>>     {
>>       if (cmsg->cmsg_level != SOL_SOCKET)
>>         continue;
>>       [...]
>>       last = cmsg;
>>     }
>>
>> Where we need 'last' to updated regardless of 'cmsg_level':
>>
>>   for (cmsg = CMSG_FIRSTHDR (msg);
>>        cmsg != NULL;
>>        cmsg = CMSG_NXTHDR (msg, cmsg))
>>     {
>>       if (cmsg->cmsg_level == SOL_SOCKET)
>>         [...]
>>       last = cmsg;
>>     }
> 
> You could move the assignment in the loop header so that continue
> continues to work.

Right, it would result in a more readable patch.  I will send a new version.