public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs
@ 2022-01-27 20:15 Adhemerval Zanella
  2022-01-27 20:15 ` [PATCH v5 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Adhemerval Zanella @ 2022-01-27 20:15 UTC (permalink / raw)
  To: libc-alpha, Florian Weimer, Carlos O'Donell

Adhemerval Zanella (3):
  support: Add support_socket_so_timestamp_time64
  linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349,
    BZ#28350)
  Linux: Only generate 64 bit timestamps for 64 bit time_t
    recvmsg/recvmmsg

 include/sys/socket.h                          |  10 +-
 support/Makefile                              |   1 +
 support/support.h                             |   4 +
 support/support_socket_so_timestamp_time64.c  |  48 +++
 sysdeps/unix/sysv/linux/Makefile              |   9 +
 .../unix/sysv/linux/convert_scm_timestamps.c  |  14 +-
 sysdeps/unix/sysv/linux/recvmmsg.c            |  45 ++-
 sysdeps/unix/sysv/linux/recvmsg.c             |  34 +-
 .../tst-socket-timestamp-compat-time64.c      |   1 +
 .../sysv/linux/tst-socket-timestamp-compat.c  | 250 +++++++++++++
 .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
 .../unix/sysv/linux/tst-socket-timestamp.c    | 338 ++++++++++++++++++
 12 files changed, 722 insertions(+), 33 deletions(-)
 create mode 100644 support/support_socket_so_timestamp_time64.c
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-compat-time64.c
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-time64.c
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp.c

-- 
2.32.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 1/3] support: Add support_socket_so_timestamp_time64
  2022-01-27 20:15 [PATCH v5 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
@ 2022-01-27 20:15 ` Adhemerval Zanella
  2022-01-28 12:37   ` Florian Weimer
  2022-01-27 20:15 ` [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
  2022-01-27 20:15 ` [PATCH v5 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
  2 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella @ 2022-01-27 20:15 UTC (permalink / raw)
  To: libc-alpha, Florian Weimer, Carlos O'Donell

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.
---
 support/Makefile                             |  1 +
 support/support.h                            |  4 ++
 support/support_socket_so_timestamp_time64.c | 48 ++++++++++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 support/support_socket_so_timestamp_time64.c

diff --git a/support/Makefile b/support/Makefile
index 6dc6c9d49a..5ddcb8d158 100644
--- a/support/Makefile
+++ b/support/Makefile
@@ -80,6 +80,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 ee27149d98..73b9fc48f0 100644
--- a/support/support.h
+++ b/support/support.h
@@ -183,6 +183,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..54bf3f4272
--- /dev/null
+++ b/support/support_socket_so_timestamp_time64.c
@@ -0,0 +1,48 @@
+/* Return whether socket supports 64-bit timestamps.
+   Copyright (C) 2022 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);
+  return r != -1;
+# endif
+#else
+  return false;
+#endif
+}
-- 
2.32.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-01-27 20:15 [PATCH v5 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
  2022-01-27 20:15 ` [PATCH v5 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
@ 2022-01-27 20:15 ` Adhemerval Zanella
  2022-01-28 13:22   ` Florian Weimer
  2022-09-30 10:47   ` Szabolcs Nagy
  2022-01-27 20:15 ` [PATCH v5 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
  2 siblings, 2 replies; 18+ messages in thread
From: Adhemerval Zanella @ 2022-01-27 20:15 UTC (permalink / raw)
  To: libc-alpha, Florian Weimer, Carlos O'Donell

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 checks 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.

Co-authored-by: Fabian Vogt <fvogt@suse.de>
---
 sysdeps/unix/sysv/linux/Makefile              |   3 +
 .../unix/sysv/linux/convert_scm_timestamps.c  |  14 +-
 .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
 .../unix/sysv/linux/tst-socket-timestamp.c    | 338 ++++++++++++++++++
 4 files changed, 350 insertions(+), 6 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 7ca9350c99..4c80ff809c 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -277,6 +277,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 580eb4be84..82171bf325 100644
--- a/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
+++ b/sysdeps/unix/sysv/linux/convert_scm_timestamps.c
@@ -54,6 +54,8 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
        cmsg != NULL;
        cmsg = CMSG_NXTHDR (msg, cmsg))
     {
+      last = cmsg;
+
       if (cmsg->cmsg_level != SOL_SOCKET)
 	continue;
 
@@ -75,11 +77,9 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
 	  tvts[1] = tmp[1];
 	  break;
 	}
-
-      last = cmsg;
     }
 
-  if (last == NULL || type == 0)
+  if (type == 0)
     return;
 
   if (CMSG_SPACE (sizeof tvts) > msgsize - msg->msg_controllen)
@@ -88,10 +88,12 @@ __convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
       return;
     }
 
+  /* Zero memory for the new cmsghdr, so reading cmsg_len field
+     by CMSG_NXTHDR does not trigger UB.  */
+  memset (msg->msg_control + msg->msg_controllen, 0,
+	  CMSG_SPACE (sizeof tvts));
   msg->msg_controllen += CMSG_SPACE (sizeof tvts);
-  cmsg = CMSG_NXTHDR(msg, last);
-  if (cmsg == NULL)
-    return;
+  cmsg = CMSG_NXTHDR (msg, last);
   cmsg->cmsg_level = SOL_SOCKET;
   cmsg->cmsg_type = type;
   cmsg->cmsg_len = CMSG_LEN (sizeof tvts);
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..1de245db70
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
@@ -0,0 +1,338 @@
+/* Check recvmsg/recvmmsg 64-bit timestamp support.
+   Copyright (C) 2022 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 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));
+
+  xclose (s);
+
+  return 0;
+}
+
+static void
+do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
+			    size_t slack, size_t tsize, int exp_payload)
+{
+  int payload;
+  struct iovec iov =
+    {
+      .iov_base = &payload,
+      .iov_len = sizeof (payload)
+    };
+  size_t msg_controllen = CMSG_SPACE (tsize) + slack;
+  char *msg_control = cmsg - msg_controllen;
+  memset (msg_control, 0x55, msg_controllen);
+  struct mmsghdr mmhdr =
+    {
+      .msg_hdr =
+      {
+        .msg_name = NULL,
+        .msg_namelen = 0,
+        .msg_iov = &iov,
+        .msg_iovlen = 1,
+        .msg_control = msg_control,
+        .msg_controllen = msg_controllen
+      },
+    };
+
+  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 (payload, exp_payload);
+
+  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;
+	}
+    }
+
+  TEST_COMPARE (timestamp, exp_timestamp);
+}
+
+/* Check if the extra ancillary space is correctly handled by recvmsg and
+   recvmmsg with different extra space for the ancillaty 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++)
+    {
+      memset (nf.buffer, 0x55, nf.length);
+      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++)
+    {
+      memset (nf.buffer, 0x55, nf.length);
+      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.  */
+  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.32.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-01-27 20:15 [PATCH v5 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
  2022-01-27 20:15 ` [PATCH v5 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
  2022-01-27 20:15 ` [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
@ 2022-01-27 20:15 ` Adhemerval Zanella
  2022-01-28 14:02   ` Florian Weimer
  2 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella @ 2022-01-27 20:15 UTC (permalink / raw)
  To: libc-alpha, Florian Weimer, Carlos O'Donell

The timestamps created by __convert_scm_timestamps only make sense for
64 bit time_t programs, 32 bit time_t programs will ignore 64 bit time_t
timestamps since SO_TIMESTAMP will be defined to old values (either by
glibc or kernel headers).

Worse, if the buffer is not suffice MSG_CTRUNC is set to indicate it
(which breaks some programs [1]).

This patch makes only 64 bit time_t recvmsg and recvmmsg to call
__convert_scm_timestamps.  Also, the assumption to called it is changed
from __ASSUME_TIME64_SYSCALLS to __TIMESIZE != 64 since the setsockopt
might be called by libraries built without __TIME_BITS=64.  The
MSG_CTRUNC is only set for the 64 bit symbols, it should happen only
if 64 bit time_t programs run older kernels.

Checked on x86_64-linux-gnu and i686-linux-gnu.

[1] https://github.com/systemd/systemd/pull/20567
---
 include/sys/socket.h                          |  10 +-
 sysdeps/unix/sysv/linux/Makefile              |  10 +-
 sysdeps/unix/sysv/linux/recvmmsg.c            |  45 +++-
 sysdeps/unix/sysv/linux/recvmsg.c             |  34 ++-
 .../tst-socket-timestamp-compat-time64.c      |   1 +
 .../sysv/linux/tst-socket-timestamp-compat.c  | 250 ++++++++++++++++++
 6 files changed, 321 insertions(+), 29 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-compat-time64.c
 create mode 100644 sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c

diff --git a/include/sys/socket.h b/include/sys/socket.h
index a1d749f9fa..6e4cf5077f 100644
--- a/include/sys/socket.h
+++ b/include/sys/socket.h
@@ -98,15 +98,21 @@ extern int __sendmmsg (int __fd, struct mmsghdr *__vmessages,
 libc_hidden_proto (__sendmmsg)
 #endif
 
-/* Receive a message as described by MESSAGE from socket FD.
-   Returns the number of bytes read or -1 for errors.  */
 extern ssize_t __libc_recvmsg (int __fd, struct msghdr *__message,
 			       int __flags);
 extern ssize_t __recvmsg (int __fd, struct msghdr *__message,
 			  int __flags) attribute_hidden;
 #if __TIMESIZE == 64
+# define __libc_recvmsg64 __libc_recvmsg
+# define __recvmsg64  __recvmsg
 # define __recvmmsg64 __recvmmsg
 #else
+extern ssize_t __libc_recvmsg64 (int __fd, struct msghdr *__message,
+				 int __flags);
+extern ssize_t __recvmsg64 (int __fd, struct msghdr *__message,
+			    int __flags);
+/* Receive a message as described by MESSAGE from socket FD.
+   Returns the number of bytes read or -1 for errors.  */
 extern int __recvmmsg64 (int __fd, struct mmsghdr *vmessages,
 			 unsigned int vlen, int flags,
 			 struct __timespec64 *timeout);
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 4c80ff809c..7122f55975 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -277,8 +277,14 @@ 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 += \
+  tst-socket-timestamp \
+  tst-socket-timestamp-compat \
+  # tests
+tests-time64 += \
+  tst-socket-timestamp-time64 \
+  tst-socket-timestamp-compat-time64
+  # tests-time64
 
 tests-special += $(objpfx)tst-socket-consts.out
 $(objpfx)tst-socket-consts.out: ../sysdeps/unix/sysv/linux/tst-socket-consts.py
diff --git a/sysdeps/unix/sysv/linux/recvmmsg.c b/sysdeps/unix/sysv/linux/recvmmsg.c
index 3b9b5f9d1f..1943b9b3d4 100644
--- a/sysdeps/unix/sysv/linux/recvmmsg.c
+++ b/sysdeps/unix/sysv/linux/recvmmsg.c
@@ -19,9 +19,9 @@
 #include <sysdep.h>
 #include <socketcall.h>
 
-int
-__recvmmsg64 (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags,
-	      struct __timespec64 *timeout)
+static int
+recvmmsg_syscall (int fd, struct mmsghdr *vmessages, unsigned int vlen,
+		  int flags, struct __timespec64 *timeout)
 {
 #ifndef __NR_recvmmsg_time64
 # define __NR_recvmmsg_time64 __NR_recvmmsg
@@ -44,12 +44,6 @@ __recvmmsg64 (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags,
       pts32 = &ts32;
     }
 
-  socklen_t csize[IOV_MAX];
-  if (vlen > IOV_MAX)
-    vlen = IOV_MAX;
-  for (int i = 0; i < vlen; i++)
-    csize[i] = vmessages[i].msg_hdr.msg_controllen;
-
 # ifdef __ASSUME_RECVMMSG_SYSCALL
   r = SYSCALL_CANCEL (recvmmsg, fd, vmessages, vlen, flags, pts32);
 # else
@@ -59,11 +53,31 @@ __recvmmsg64 (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags,
     {
       if (timeout != NULL)
         *timeout = valid_timespec_to_timespec64 (ts32);
+    }
+#endif
+  return r;
+}
+
+int
+__recvmmsg64 (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags,
+	      struct __timespec64 *timeout)
+{
+#if __TIMESIZE != 64
+  socklen_t csize[IOV_MAX];
+  if (vlen > IOV_MAX)
+    vlen = IOV_MAX;
+  for (int i = 0; i < vlen; i++)
+    csize[i] = vmessages[i].msg_hdr.msg_controllen;
+#endif
 
+  int r = recvmmsg_syscall (fd, vmessages, vlen, flags, timeout);
+#if __TIMESIZE != 64
+  if (r > 0)
+    {
       for (int i=0; i < r; i++)
         __convert_scm_timestamps (&vmessages[i].msg_hdr, csize[i]);
     }
-#endif /* __ASSUME_TIME64_SYSCALLS  */
+#endif
   return r;
 }
 #if __TIMESIZE != 64
@@ -79,10 +93,13 @@ __recvmmsg (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags,
       ts64 = valid_timespec_to_timespec64 (*timeout);
       pts64 = &ts64;
     }
-  int r = __recvmmsg64 (fd, vmessages, vlen, flags, pts64);
-  if (r >= 0 && timeout != NULL)
-    /* The remanining timeout will be always less the input TIMEOUT.  */
-    *timeout = valid_timespec64_to_timespec (ts64);
+  int r = recvmmsg_syscall (fd, vmessages, vlen, flags, pts64);
+  if (r >= 0)
+    {
+      /* The remanining timeout will be always less the input TIMEOUT.  */
+      if (timeout != NULL)
+	*timeout = valid_timespec64_to_timespec (ts64);
+    }
   return r;
 }
 #endif
diff --git a/sysdeps/unix/sysv/linux/recvmsg.c b/sysdeps/unix/sysv/linux/recvmsg.c
index 53e567ed31..401d415b2c 100644
--- a/sysdeps/unix/sysv/linux/recvmsg.c
+++ b/sysdeps/unix/sysv/linux/recvmsg.c
@@ -20,29 +20,41 @@
 #include <sysdep-cancel.h>
 #include <socketcall.h>
 
+static int
+__recvmsg_syscall (int fd, struct msghdr *msg, int flags)
+{
+#ifdef __ASSUME_RECVMSG_SYSCALL
+  return SYSCALL_CANCEL (recvmsg, fd, msg, flags);
+#else
+  return SOCKETCALL_CANCEL (recvmsg, fd, msg, flags);
+#endif
+}
+
 ssize_t
-__libc_recvmsg (int fd, struct msghdr *msg, int flags)
+__libc_recvmsg64 (int fd, struct msghdr *msg, int flags)
 {
   ssize_t r;
-#ifndef __ASSUME_TIME64_SYSCALLS
+#if __TIMESIZE != 64
   socklen_t orig_controllen = msg != NULL ? msg->msg_controllen : 0;
 #endif
 
-#ifdef __ASSUME_RECVMSG_SYSCALL
-  r = SYSCALL_CANCEL (recvmsg, fd, msg, flags);
-#else
-  r = SOCKETCALL_CANCEL (recvmsg, fd, msg, flags);
-#endif
+  r = __recvmsg_syscall (fd, msg, flags);
 
-#ifndef __ASSUME_TIME64_SYSCALLS
+#if __TIMESIZE != 64
   if (r >= 0 && orig_controllen != 0)
     __convert_scm_timestamps (msg, orig_controllen);
 #endif
 
   return r;
 }
-weak_alias (__libc_recvmsg, recvmsg)
-weak_alias (__libc_recvmsg, __recvmsg)
 #if __TIMESIZE != 64
-weak_alias (__recvmsg, __recvmsg64)
+weak_alias (__libc_recvmsg64, __recvmsg64)
+
+ssize_t
+__libc_recvmsg (int fd, struct msghdr *msg, int flags)
+{
+  return __recvmsg_syscall (fd, msg, flags);
+}
 #endif
+weak_alias (__libc_recvmsg, recvmsg)
+weak_alias (__libc_recvmsg, __recvmsg)
diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat-time64.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat-time64.c
new file mode 100644
index 0000000000..96a0bef0bf
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat-time64.c
@@ -0,0 +1 @@
+#include "tst-socket-timestamp-compat.c"
diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
new file mode 100644
index 0000000000..ca6ed93b4d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
@@ -0,0 +1,250 @@
+/* Check recvmsg/recvmmsg 64-bit timestamp support.
+   Copyright (C) 2022 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 <arpa/inet.h>
+#include <string.h>
+#include <support/check.h>
+#include <support/xsocket.h>
+#include <support/xunistd.h>
+#include <stdbool.h>
+
+/* AF_INET socket and address used to receive data.  */
+static int srv;
+static struct sockaddr_in srv_addr;
+
+static int
+do_sendto (const struct sockaddr_in *addr, int payload)
+{
+  int s = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0);
+  xconnect (s, (const struct sockaddr *) addr, sizeof (*addr));
+
+  xsendto (s, &payload, sizeof (payload), 0, (const struct sockaddr *) addr,
+	   sizeof (*addr));
+
+  xclose (s);
+
+  return 0;
+}
+
+static void
+do_recvmsg_ancillary (bool use_multi_call, struct mmsghdr *mmhdr,
+		      char *msgbuf, size_t msgbuflen, int exp_payload)
+{
+  int payload;
+  struct iovec iov =
+    {
+      .iov_base = &payload,
+      .iov_len = sizeof (payload)
+    };
+  mmhdr->msg_hdr.msg_name = NULL;
+  mmhdr->msg_hdr.msg_iov = &iov;
+  mmhdr->msg_hdr.msg_iovlen = 1;
+  mmhdr->msg_hdr.msg_control = msgbuf;
+  mmhdr->msg_hdr.msg_controllen = msgbuflen;
+
+  int r;
+  if (use_multi_call)
+    {
+      r = recvmmsg (srv, mmhdr, 1, 0, NULL);
+      if (r >= 0)
+	r = mmhdr->msg_len;
+    }
+  else
+    r = recvmsg (srv, &mmhdr->msg_hdr, 0);
+  TEST_COMPARE (r, sizeof (int));
+  TEST_COMPARE (payload, exp_payload);
+}
+
+/* Check if recvmsg create the additional 64 bit timestamp if only 32 bit
+   is enabled for 64 bit recvmsg symbol.  */
+static void
+do_test_large_buffer (bool mc)
+{
+  struct mmsghdr mmhdr = { 0 };
+  /* It should be large enought for either timeval/timespec and the
+     64 time type as well.  */
+  char msgbuf[512];
+
+  /* Enable 32 bit timeval precision and check if no 64 bit timeval stamp
+     is created.  */
+  {
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP_OLD, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+
+    do_sendto (&srv_addr, 42);
+    do_recvmsg_ancillary (mc, &mmhdr, msgbuf, sizeof msgbuf, 42);
+
+    bool found_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 (sizeof (time_t) > 4 && cmsg->cmsg_type == SO_TIMESTAMP_NEW)
+	found_timestamp = true;
+      else
+	TEST_VERIFY (cmsg->cmsg_type != SO_TIMESTAMP_NEW);
+    }
+
+    TEST_COMPARE (found_timestamp, sizeof (time_t) > 4);
+  }
+
+  /* Same as before, but for timespec.  */
+  {
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS_OLD, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+
+    do_sendto (&srv_addr, 42);
+    do_recvmsg_ancillary (mc, &mmhdr, msgbuf, sizeof msgbuf, 42);
+
+    bool found_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 (sizeof (time_t) > 4 && cmsg->cmsg_type == SO_TIMESTAMPNS_NEW)
+	found_timestamp = true;
+      else
+	TEST_VERIFY (cmsg->cmsg_type != SO_TIMESTAMPNS_NEW);
+    }
+
+    TEST_COMPARE (found_timestamp, sizeof (time_t) > 4);
+  }
+}
+
+/* Check if recvmsg does not create the additional 64 bit timestamp if
+   only 32 bit timestamp is enabled if the ancillary buffer is not large
+   enought.  Also checks if MSG_CTRUNC is set iff for 64 bit recvmsg
+   symbol.  */
+static void
+do_test_small_buffer (bool mc)
+{
+  struct mmsghdr mmhdr = { 0 };
+
+  /* Enable 32 bit timeval precision and check if no 64 bit timeval stamp
+     is created.  */
+  {
+    char msgbuf[CMSG_SPACE (sizeof (struct timeval))];
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP_OLD, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+
+    do_sendto (&srv_addr, 42);
+    do_recvmsg_ancillary (mc, &mmhdr, msgbuf, sizeof msgbuf, 42);
+
+    bool found_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 (sizeof (time_t) > 4 && cmsg->cmsg_type == SO_TIMESTAMP_NEW)
+	found_timestamp = true;
+      else
+	TEST_VERIFY (cmsg->cmsg_type != SO_TIMESTAMP_NEW);
+    }
+
+    if (sizeof (time_t) > 4)
+      {
+	TEST_VERIFY ((mmhdr.msg_hdr.msg_flags & MSG_CTRUNC));
+	TEST_COMPARE (found_timestamp, 0);
+      }
+    else
+      {
+	TEST_VERIFY (!(mmhdr.msg_hdr.msg_flags & MSG_CTRUNC));
+	TEST_COMPARE (found_timestamp, 0);
+      }
+  }
+
+  /* Same as before, but for timespec.  */
+  {
+    char msgbuf[CMSG_SPACE (sizeof (struct timespec))];
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS_OLD, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+
+    do_sendto (&srv_addr, 42);
+    do_recvmsg_ancillary (mc, &mmhdr, msgbuf, sizeof msgbuf, 42);
+
+    bool found_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 (sizeof (time_t) > 4 && cmsg->cmsg_type == SO_TIMESTAMPNS_NEW)
+	found_timestamp = true;
+      else
+	TEST_VERIFY (cmsg->cmsg_type != SO_TIMESTAMPNS_NEW);
+    }
+
+    if (sizeof (time_t) > 4)
+      {
+	TEST_VERIFY ((mmhdr.msg_hdr.msg_flags & MSG_CTRUNC));
+	TEST_COMPARE (found_timestamp, 0);
+      }
+    else
+      {
+	TEST_VERIFY ((mmhdr.msg_hdr.msg_flags & MSG_CTRUNC) == 0);
+	TEST_COMPARE (found_timestamp, sizeof (time_t) > 4);
+      }
+  }
+}
+
+static int
+do_test (void)
+{
+  /* This test only make sense for ABIs that support 32 bit time_t socket
+     timestampss.  */
+  if (sizeof (time_t) > 4 && __WORDSIZE == 64)
+    return 0;
+
+  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));
+  }
+
+  /* Check recvmsg;  */
+  do_test_large_buffer (false);
+  do_test_small_buffer (false);
+  /* Check recvmmsg.  */
+  do_test_large_buffer (true);
+  do_test_small_buffer (true);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.32.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/3] support: Add support_socket_so_timestamp_time64
  2022-01-27 20:15 ` [PATCH v5 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
@ 2022-01-28 12:37   ` Florian Weimer
  0 siblings, 0 replies; 18+ messages in thread
From: Florian Weimer @ 2022-01-28 12:37 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Carlos O'Donell

* 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.
> ---
>  support/Makefile                             |  1 +
>  support/support.h                            |  4 ++
>  support/support_socket_so_timestamp_time64.c | 48 ++++++++++++++++++++
>  3 files changed, 53 insertions(+)
>  create mode 100644 support/support_socket_so_timestamp_time64.c

This looks okay to me.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-01-27 20:15 ` [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
@ 2022-01-28 13:22   ` Florian Weimer
  2022-01-28 16:41     ` Adhemerval Zanella
  2022-09-30 10:47   ` Szabolcs Nagy
  1 sibling, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-28 13:22 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Carlos O'Donell, Fabian Vogt

* Adhemerval Zanella:

> +  /* If there is no timestamp in the ancillary data, recvmsg should set
> +     the flag to indicate it.  */

I think this comment is outdated.  The rest looks okay to me.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-01-27 20:15 ` [PATCH v5 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
@ 2022-01-28 14:02   ` Florian Weimer
  2022-01-28 16:42     ` Adhemerval Zanella
  0 siblings, 1 reply; 18+ messages in thread
From: Florian Weimer @ 2022-01-28 14:02 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Carlos O'Donell

* Adhemerval Zanella:

> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c

> +/* Check if recvmsg create the additional 64 bit timestamp if only 32 bit
> +   is enabled for 64 bit recvmsg symbol.  */
> +static void
> +do_test_large_buffer (bool mc)
> +{
> +  struct mmsghdr mmhdr = { 0 };
> +  /* It should be large enought for either timeval/timespec and the
> +     64 time type as well.  */
> +  char msgbuf[512];

I think this needs a union with struct cmsgbuf to ensure alignment.
Alternatively, use malloc.


> +static void
> +do_test_small_buffer (bool mc)
> +{
> +  struct mmsghdr mmhdr = { 0 };
> +
> +  /* Enable 32 bit timeval precision and check if no 64 bit timeval stamp
> +     is created.  */
> +  {
> +    char msgbuf[CMSG_SPACE (sizeof (struct timeval))];

Likewise.

Rest looks okay.  The kernel does not pass on whether a time32 or time64
system call was used, so implementing time32 recvmsg with recvmsg_time64
is fine in that regard.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-01-28 13:22   ` Florian Weimer
@ 2022-01-28 16:41     ` Adhemerval Zanella
  0 siblings, 0 replies; 18+ messages in thread
From: Adhemerval Zanella @ 2022-01-28 16:41 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha, Carlos O'Donell, Fabian Vogt



On 28/01/2022 10:22, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> +  /* If there is no timestamp in the ancillary data, recvmsg should set
>> +     the flag to indicate it.  */
> 
> I think this comment is outdated.  The rest looks okay to me.

Indeed, I though I had removed all the outdated comments.

> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks.

> 
> Thanks,
> Florian
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-01-28 14:02   ` Florian Weimer
@ 2022-01-28 16:42     ` Adhemerval Zanella
  0 siblings, 0 replies; 18+ messages in thread
From: Adhemerval Zanella @ 2022-01-28 16:42 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha, Carlos O'Donell



On 28/01/2022 11:02, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
> 
>> +/* Check if recvmsg create the additional 64 bit timestamp if only 32 bit
>> +   is enabled for 64 bit recvmsg symbol.  */
>> +static void
>> +do_test_large_buffer (bool mc)
>> +{
>> +  struct mmsghdr mmhdr = { 0 };
>> +  /* It should be large enought for either timeval/timespec and the
>> +     64 time type as well.  */
>> +  char msgbuf[512];
> 
> I think this needs a union with struct cmsgbuf to ensure alignment.
> Alternatively, use malloc.

Yeah, it does require to setup the alignment. I will change to use an union.

> 
> 
>> +static void
>> +do_test_small_buffer (bool mc)
>> +{
>> +  struct mmsghdr mmhdr = { 0 };
>> +
>> +  /* Enable 32 bit timeval precision and check if no 64 bit timeval stamp
>> +     is created.  */
>> +  {
>> +    char msgbuf[CMSG_SPACE (sizeof (struct timeval))];
> 
> Likewise.
> 
> Rest looks okay.  The kernel does not pass on whether a time32 or time64
> system call was used, so implementing time32 recvmsg with recvmsg_time64
> is fine in that regard.
> 
> Thanks,
> Florian
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-01-27 20:15 ` [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
  2022-01-28 13:22   ` Florian Weimer
@ 2022-09-30 10:47   ` Szabolcs Nagy
  2022-09-30 11:05     ` Szabolcs Nagy
  1 sibling, 1 reply; 18+ messages in thread
From: Szabolcs Nagy @ 2022-09-30 10:47 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Florian Weimer, Carlos O'Donell

The 01/27/2022 17:15, Adhemerval Zanella via Libc-alpha 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 checks 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.
> 
> Co-authored-by: Fabian Vogt <fvogt@suse.de>

note: the time64 recvmsg test started to fail on 32bit
arm after i updated my aarch64 kernel to 5.18

FAIL: socket/tst-socket-timestamp-time64

../sysdeps/unix/sysv/linux/tst-socket-timestamp.c:136: numeric comparison failure
   left: 0 (0x0); from: timestamp
  right: 1 (0x1); from: exp_timestamp


> +static void
> +do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
> +			    size_t slack, size_t tsize, int exp_payload)
> +{
> +  int payload;
> +  struct iovec iov =
> +    {
> +      .iov_base = &payload,
> +      .iov_len = sizeof (payload)
> +    };
> +  size_t msg_controllen = CMSG_SPACE (tsize) + slack;
> +  char *msg_control = cmsg - msg_controllen;
> +  memset (msg_control, 0x55, msg_controllen);
> +  struct mmsghdr mmhdr =
> +    {
> +      .msg_hdr =
> +      {
> +        .msg_name = NULL,
> +        .msg_namelen = 0,
> +        .msg_iov = &iov,
> +        .msg_iovlen = 1,
> +        .msg_control = msg_control,
> +        .msg_controllen = msg_controllen
> +      },
> +    };
> +
> +  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 (payload, exp_payload);
> +
> +  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)))


in strace i see

recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\0\0\0\0", iov_len=4}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_TIMESTAMP, cmsg_data={tv_sec=1664533412, tv_usec=56794}}], msg_controllen=20, msg_flags=0}, 0) = 4

i.e. cmsg->cmsg_len == 20, but CMSG_LEN (sizeof (struct timeval)) is 28.

in the loop sometimes i also see a second cmsg where
cmsg->cmsg_type == SO_TIMESTAMP_NEW and cmsg->cmsg_len == 28,
but then the type does not match.

same behaviour with recvmmsg_time64 (but my strace is too
old to show that).

not sure how to debug this further.

> +	{
> +	  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;
> +	}
> +    }
> +
> +  TEST_COMPARE (timestamp, exp_timestamp);

this fails with false != true.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 10:47   ` Szabolcs Nagy
@ 2022-09-30 11:05     ` Szabolcs Nagy
  2022-09-30 11:24       ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 18+ messages in thread
From: Szabolcs Nagy @ 2022-09-30 11:05 UTC (permalink / raw)
  To: Adhemerval Zanella, Florian Weimer, libc-alpha

The 09/30/2022 11:47, Szabolcs Nagy via Libc-alpha wrote:
> The 01/27/2022 17:15, Adhemerval Zanella via Libc-alpha 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 checks 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.
> > 
> > Co-authored-by: Fabian Vogt <fvogt@suse.de>
> 
> note: the time64 recvmsg test started to fail on 32bit
> arm after i updated my aarch64 kernel to 5.18

sorry the kernel is
Linux 8a7948402d35 5.15.0-48-generic #54~20.04.1-Ubuntu SMP Thu Sep 1 16:20:53 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux

> 
> FAIL: socket/tst-socket-timestamp-time64
> 
> ../sysdeps/unix/sysv/linux/tst-socket-timestamp.c:136: numeric comparison failure
>    left: 0 (0x0); from: timestamp
>   right: 1 (0x1); from: exp_timestamp
> 
> 
> > +static void
> > +do_recvmsg_slack_ancillary (bool use_multi_call, int s, void *cmsg,
> > +			    size_t slack, size_t tsize, int exp_payload)
> > +{
> > +  int payload;
> > +  struct iovec iov =
> > +    {
> > +      .iov_base = &payload,
> > +      .iov_len = sizeof (payload)
> > +    };
> > +  size_t msg_controllen = CMSG_SPACE (tsize) + slack;
> > +  char *msg_control = cmsg - msg_controllen;
> > +  memset (msg_control, 0x55, msg_controllen);
> > +  struct mmsghdr mmhdr =
> > +    {
> > +      .msg_hdr =
> > +      {
> > +        .msg_name = NULL,
> > +        .msg_namelen = 0,
> > +        .msg_iov = &iov,
> > +        .msg_iovlen = 1,
> > +        .msg_control = msg_control,
> > +        .msg_controllen = msg_controllen
> > +      },
> > +    };
> > +
> > +  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 (payload, exp_payload);
> > +
> > +  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)))
> 
> 
> in strace i see
> 
> recvmsg(3, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\0\0\0\0", iov_len=4}], msg_iovlen=1, msg_control=[{cmsg_len=20, cmsg_level=SOL_SOCKET, cmsg_type=SCM_TIMESTAMP, cmsg_data={tv_sec=1664533412, tv_usec=56794}}], msg_controllen=20, msg_flags=0}, 0) = 4
> 
> i.e. cmsg->cmsg_len == 20, but CMSG_LEN (sizeof (struct timeval)) is 28.
> 
> in the loop sometimes i also see a second cmsg where
> cmsg->cmsg_type == SO_TIMESTAMP_NEW and cmsg->cmsg_len == 28,
> but then the type does not match.
> 
> same behaviour with recvmmsg_time64 (but my strace is too
> old to show that).
> 
> not sure how to debug this further.
> 
> > +	{
> > +	  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;
> > +	}
> > +    }
> > +
> > +  TEST_COMPARE (timestamp, exp_timestamp);
> 
> this fails with false != true.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 11:05     ` Szabolcs Nagy
@ 2022-09-30 11:24       ` Adhemerval Zanella Netto
  2022-09-30 12:31         ` Szabolcs Nagy
  0 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella Netto @ 2022-09-30 11:24 UTC (permalink / raw)
  To: Szabolcs Nagy, Florian Weimer, libc-alpha



On 30/09/22 08:05, Szabolcs Nagy wrote:
> The 09/30/2022 11:47, Szabolcs Nagy via Libc-alpha wrote:
>> The 01/27/2022 17:15, Adhemerval Zanella via Libc-alpha 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 checks 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.
>>>
>>> Co-authored-by: Fabian Vogt <fvogt@suse.de>
>>
>> note: the time64 recvmsg test started to fail on 32bit
>> arm after i updated my aarch64 kernel to 5.18
> 
> sorry the kernel is
> Linux 8a7948402d35 5.15.0-48-generic #54~20.04.1-Ubuntu SMP Thu Sep 1 16:20:53 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux

I just check on exactly same kernel (ubuntu 22) on a aarch64 VM and I could not
reproduce it:

$ uname -a
Linux ubuntu-aarch64 5.15.0-48-generic #54-Ubuntu SMP Fri Aug 26 13:31:33 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
$ file socket/tst-socket-timestamp
socket/tst-socket-timestamp: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, with debug_info, not stripped
$ file socket/tst-socket-timestamp-time64
socket/tst-socket-timestamp-time64: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, with debug_info, not stripped
$ ./testrun.sh socket/tst-socket-timestamp
$ ./testrun.sh socket/tst-socket-timestamp-time64
$

I used gcc 12.1.1, maybe it is a compiler issue?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 11:24       ` Adhemerval Zanella Netto
@ 2022-09-30 12:31         ` Szabolcs Nagy
  2022-09-30 12:51           ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 18+ messages in thread
From: Szabolcs Nagy @ 2022-09-30 12:31 UTC (permalink / raw)
  To: Adhemerval Zanella Netto; +Cc: Florian Weimer, libc-alpha

The 09/30/2022 08:24, Adhemerval Zanella Netto wrote:
> On 30/09/22 08:05, Szabolcs Nagy wrote:
> > The 09/30/2022 11:47, Szabolcs Nagy via Libc-alpha wrote:
> >> The 01/27/2022 17:15, Adhemerval Zanella via Libc-alpha 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 checks 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.
> >>>
> >>> Co-authored-by: Fabian Vogt <fvogt@suse.de>
> >>
> >> note: the time64 recvmsg test started to fail on 32bit
> >> arm after i updated my aarch64 kernel to 5.18
> > 
> > sorry the kernel is
> > Linux 8a7948402d35 5.15.0-48-generic #54~20.04.1-Ubuntu SMP Thu Sep 1 16:20:53 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
> 
> I just check on exactly same kernel (ubuntu 22) on a aarch64 VM and I could not
> reproduce it:
> 
> $ uname -a
> Linux ubuntu-aarch64 5.15.0-48-generic #54-Ubuntu SMP Fri Aug 26 13:31:33 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
> $ file socket/tst-socket-timestamp
> socket/tst-socket-timestamp: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, with debug_info, not stripped
> $ file socket/tst-socket-timestamp-time64
> socket/tst-socket-timestamp-time64: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, with debug_info, not stripped
> $ ./testrun.sh socket/tst-socket-timestamp
> $ ./testrun.sh socket/tst-socket-timestamp-time64
> $
> 
> I used gcc 12.1.1, maybe it is a compiler issue?

sorry it was my fault: old kernel headers.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 12:31         ` Szabolcs Nagy
@ 2022-09-30 12:51           ` Adhemerval Zanella Netto
  2022-09-30 13:09             ` Arnd Bergmann
  0 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella Netto @ 2022-09-30 12:51 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: Florian Weimer, libc-alpha



On 30/09/22 09:31, Szabolcs Nagy wrote:
> The 09/30/2022 08:24, Adhemerval Zanella Netto wrote:
>> On 30/09/22 08:05, Szabolcs Nagy wrote:
>>> The 09/30/2022 11:47, Szabolcs Nagy via Libc-alpha wrote:
>>>> The 01/27/2022 17:15, Adhemerval Zanella via Libc-alpha 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 checks 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.
>>>>>
>>>>> Co-authored-by: Fabian Vogt <fvogt@suse.de>
>>>>
>>>> note: the time64 recvmsg test started to fail on 32bit
>>>> arm after i updated my aarch64 kernel to 5.18
>>>
>>> sorry the kernel is
>>> Linux 8a7948402d35 5.15.0-48-generic #54~20.04.1-Ubuntu SMP Thu Sep 1 16:20:53 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
>>
>> I just check on exactly same kernel (ubuntu 22) on a aarch64 VM and I could not
>> reproduce it:
>>
>> $ uname -a
>> Linux ubuntu-aarch64 5.15.0-48-generic #54-Ubuntu SMP Fri Aug 26 13:31:33 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
>> $ file socket/tst-socket-timestamp
>> socket/tst-socket-timestamp: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, with debug_info, not stripped
>> $ file socket/tst-socket-timestamp-time64
>> socket/tst-socket-timestamp-time64: ELF 32-bit LSB pie executable, ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 3.2.0, with debug_info, not stripped
>> $ ./testrun.sh socket/tst-socket-timestamp
>> $ ./testrun.sh socket/tst-socket-timestamp-time64
>> $
>>
>> I used gcc 12.1.1, maybe it is a compiler issue?
> 
> sorry it was my fault: old kernel headers.

Right, but I am puzzled since it should not matter (at least glibc should handle it).
What was miscompiled due wrong kernel header?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 12:51           ` Adhemerval Zanella Netto
@ 2022-09-30 13:09             ` Arnd Bergmann
  2022-09-30 13:46               ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2022-09-30 13:09 UTC (permalink / raw)
  To: libc-alpha

On Fri, Sep 30, 2022, at 2:51 PM, Adhemerval Zanella Netto via Libc-alpha wrote:
> On 30/09/22 09:31, Szabolcs Nagy wrote:
>>>
>>> I used gcc 12.1.1, maybe it is a compiler issue?
>> 
>> sorry it was my fault: old kernel headers.
>
> Right, but I am puzzled since it should not matter (at least glibc 
> should handle it).
> What was miscompiled due wrong kernel header?

Using SIOCGSTAMP/SIOCGSTAMPNS wtih 64-bit time_t requires kernel
headers after this 2019 commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0768e17073dc52

      Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 13:09             ` Arnd Bergmann
@ 2022-09-30 13:46               ` Adhemerval Zanella Netto
  2022-09-30 14:56                 ` Arnd Bergmann
  0 siblings, 1 reply; 18+ messages in thread
From: Adhemerval Zanella Netto @ 2022-09-30 13:46 UTC (permalink / raw)
  To: libc-alpha



On 30/09/22 10:09, Arnd Bergmann wrote:
> On Fri, Sep 30, 2022, at 2:51 PM, Adhemerval Zanella Netto via Libc-alpha wrote:
>> On 30/09/22 09:31, Szabolcs Nagy wrote:
>>>>
>>>> I used gcc 12.1.1, maybe it is a compiler issue?
>>>
>>> sorry it was my fault: old kernel headers.
>>
>> Right, but I am puzzled since it should not matter (at least glibc 
>> should handle it).
>> What was miscompiled due wrong kernel header?
> 
> Using SIOCGSTAMP/SIOCGSTAMPNS wtih 64-bit time_t requires kernel
> headers after this 2019 commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0768e17073dc52
> 
>       Arnd

But glibc does not use these definition internally or on tests.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 13:46               ` Adhemerval Zanella Netto
@ 2022-09-30 14:56                 ` Arnd Bergmann
  2022-09-30 15:33                   ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2022-09-30 14:56 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, Xi Ruoyao

On Fri, Sep 30, 2022, at 3:46 PM, Adhemerval Zanella Netto via Libc-alpha wrote:
> On 30/09/22 10:09, Arnd Bergmann wrote:
>> On Fri, Sep 30, 2022, at 2:51 PM, Adhemerval Zanella Netto via Libc-alpha wrote:
>>> On 30/09/22 09:31, Szabolcs Nagy wrote:
>>>>>
>>>>> I used gcc 12.1.1, maybe it is a compiler issue?
>>>>
>>>> sorry it was my fault: old kernel headers.
>>>
>>> Right, but I am puzzled since it should not matter (at least glibc 
>>> should handle it).
>>> What was miscompiled due wrong kernel header?
>> 
>> Using SIOCGSTAMP/SIOCGSTAMPNS wtih 64-bit time_t requires kernel
>> headers after this 2019 commit:
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0768e17073dc52
>
> But glibc does not use these definition internally or on tests.

My mistake. SO_TIMESTAMPNS was a different commit, this is the one
you need then:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=887feae36a

    Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-09-30 14:56                 ` Arnd Bergmann
@ 2022-09-30 15:33                   ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 18+ messages in thread
From: Adhemerval Zanella Netto @ 2022-09-30 15:33 UTC (permalink / raw)
  To: Arnd Bergmann, Xi Ruoyao



On 30/09/22 11:56, Arnd Bergmann wrote:
> On Fri, Sep 30, 2022, at 3:46 PM, Adhemerval Zanella Netto via Libc-alpha wrote:
>> On 30/09/22 10:09, Arnd Bergmann wrote:
>>> On Fri, Sep 30, 2022, at 2:51 PM, Adhemerval Zanella Netto via Libc-alpha wrote:
>>>> On 30/09/22 09:31, Szabolcs Nagy wrote:
>>>>>>
>>>>>> I used gcc 12.1.1, maybe it is a compiler issue?
>>>>>
>>>>> sorry it was my fault: old kernel headers.
>>>>
>>>> Right, but I am puzzled since it should not matter (at least glibc 
>>>> should handle it).
>>>> What was miscompiled due wrong kernel header?
>>>
>>> Using SIOCGSTAMP/SIOCGSTAMPNS wtih 64-bit time_t requires kernel
>>> headers after this 2019 commit:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0768e17073dc52
>>
>> But glibc does not use these definition internally or on tests.
> 
> My mistake. SO_TIMESTAMPNS was a different commit, this is the one
> you need then:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=887feae36a
> 
>     Arnd

Right, so if we need an update kernel to actually build and test glibc
correctly I think it would be better to internally define SO_TIMESTAMPNS
instead of rely on kernel headers.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-09-30 15:33 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-27 20:15 [PATCH v5 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
2022-01-27 20:15 ` [PATCH v5 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
2022-01-28 12:37   ` Florian Weimer
2022-01-27 20:15 ` [PATCH v5 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
2022-01-28 13:22   ` Florian Weimer
2022-01-28 16:41     ` Adhemerval Zanella
2022-09-30 10:47   ` Szabolcs Nagy
2022-09-30 11:05     ` Szabolcs Nagy
2022-09-30 11:24       ` Adhemerval Zanella Netto
2022-09-30 12:31         ` Szabolcs Nagy
2022-09-30 12:51           ` Adhemerval Zanella Netto
2022-09-30 13:09             ` Arnd Bergmann
2022-09-30 13:46               ` Adhemerval Zanella Netto
2022-09-30 14:56                 ` Arnd Bergmann
2022-09-30 15:33                   ` Adhemerval Zanella Netto
2022-01-27 20:15 ` [PATCH v5 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
2022-01-28 14:02   ` Florian Weimer
2022-01-28 16:42     ` Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).