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

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  | 265 ++++++++++++++
 .../sysv/linux/tst-socket-timestamp-time64.c  |   1 +
 .../unix/sysv/linux/tst-socket-timestamp.c    | 336 ++++++++++++++++++
 12 files changed, 735 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] 9+ messages in thread

* [PATCH v6 1/3] support: Add support_socket_so_timestamp_time64
  2022-01-28 17:50 [PATCH v6 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
@ 2022-01-28 17:50 ` Adhemerval Zanella
  2022-01-28 17:50 ` [PATCH v6 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
  2022-01-28 17:50 ` [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
  2 siblings, 0 replies; 9+ messages in thread
From: Adhemerval Zanella @ 2022-01-28 17:50 UTC (permalink / raw)
  To: libc-alpha, Florian Weimer

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.

Reviewed-by: Florian Weimer <fweimer@redhat.com>
---
 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] 9+ messages in thread

* [PATCH v6 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350)
  2022-01-28 17:50 [PATCH v6 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
  2022-01-28 17:50 ` [PATCH v6 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
@ 2022-01-28 17:50 ` Adhemerval Zanella
  2022-01-28 17:50 ` [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
  2 siblings, 0 replies; 9+ messages in thread
From: Adhemerval Zanella @ 2022-01-28 17:50 UTC (permalink / raw)
  To: libc-alpha, Florian Weimer

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>

Reviewed-by: Florian Weimer <fweimer@redhat.com>
---
 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    | 336 ++++++++++++++++++
 4 files changed, 348 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..9c2e76f7e2
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp.c
@@ -0,0 +1,336 @@
+/* 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;
+	}
+    }
+
+  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] 9+ messages in thread

* [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-01-28 17:50 [PATCH v6 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
  2022-01-28 17:50 ` [PATCH v6 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
  2022-01-28 17:50 ` [PATCH v6 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
@ 2022-01-28 17:50 ` Adhemerval Zanella
  2022-01-28 18:19   ` Florian Weimer
                     ` (2 more replies)
  2 siblings, 3 replies; 9+ messages in thread
From: Adhemerval Zanella @ 2022-01-28 17:50 UTC (permalink / raw)
  To: libc-alpha, Florian Weimer

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  | 265 ++++++++++++++++++
 6 files changed, 336 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..de261dae5a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
@@ -0,0 +1,265 @@
+/* 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,
+		      void *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.  */
+
+  union
+  {
+    struct cmsghdr cmsghdr;
+    char msgbuf[512];
+  } control;
+
+  /* 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, &control, sizeof control, 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, &control, sizeof control, 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.  */
+  {
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP_OLD, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+
+    union
+    {
+      struct cmsghdr cmsghdr;
+      char msgbuf[CMSG_SPACE (sizeof (struct timeval))];
+    } control;
+
+    do_sendto (&srv_addr, 42);
+    do_recvmsg_ancillary (mc, &mmhdr, &control, sizeof control, 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.  */
+  {
+    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS_OLD, &(int){1},
+			sizeof (int));
+    TEST_VERIFY_EXIT (r != -1);
+
+    union
+    {
+      struct cmsghdr cmsghdr;
+      char msgbuf[CMSG_SPACE (sizeof (struct timespec))];
+    } control;
+
+    do_sendto (&srv_addr, 42);
+    do_recvmsg_ancillary (mc, &mmhdr, &control, sizeof control, 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, 0);
+      }
+  }
+}
+
+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] 9+ messages in thread

* Re: [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-01-28 17:50 ` [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
@ 2022-01-28 18:19   ` Florian Weimer
  2022-01-28 19:41     ` Carlos O'Donell
  2022-01-29  4:51   ` H.J. Lu
  2022-02-03 18:34   ` Szabolcs Nagy
  2 siblings, 1 reply; 9+ messages in thread
From: Florian Weimer @ 2022-01-28 18:19 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

* Adhemerval Zanella:

> 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  | 265 ++++++++++++++++++
>  6 files changed, 336 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

Looks okay.

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

Thanks,
Florian


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

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

On 1/28/22 13:19, Florian Weimer via Libc-alpha wrote:
> * Adhemerval Zanella:
> 
>> 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  | 265 ++++++++++++++++++
>>  6 files changed, 336 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
> 
> Looks okay.
> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>

OK to push for glibc 2.35. This fixes a blocker for the release.

-- 
Cheers,
Carlos.


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

* Re: [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-01-28 17:50 ` [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
  2022-01-28 18:19   ` Florian Weimer
@ 2022-01-29  4:51   ` H.J. Lu
  2022-02-03 18:34   ` Szabolcs Nagy
  2 siblings, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2022-01-29  4:51 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: GNU C Library, Florian Weimer

On Fri, Jan 28, 2022 at 9:57 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> 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  | 265 ++++++++++++++++++
>  6 files changed, 336 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..de261dae5a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-socket-timestamp-compat.c
> @@ -0,0 +1,265 @@
> +/* 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,
> +                     void *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.  */
> +
> +  union
> +  {
> +    struct cmsghdr cmsghdr;
> +    char msgbuf[512];
> +  } control;
> +
> +  /* 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, &control, sizeof control, 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, &control, sizeof control, 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.  */
> +  {
> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMP_OLD, &(int){1},
> +                       sizeof (int));
> +    TEST_VERIFY_EXIT (r != -1);
> +
> +    union
> +    {
> +      struct cmsghdr cmsghdr;
> +      char msgbuf[CMSG_SPACE (sizeof (struct timeval))];
> +    } control;
> +
> +    do_sendto (&srv_addr, 42);
> +    do_recvmsg_ancillary (mc, &mmhdr, &control, sizeof control, 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.  */
> +  {
> +    int r = setsockopt (srv, SOL_SOCKET, SO_TIMESTAMPNS_OLD, &(int){1},
> +                       sizeof (int));
> +    TEST_VERIFY_EXIT (r != -1);
> +
> +    union
> +    {
> +      struct cmsghdr cmsghdr;
> +      char msgbuf[CMSG_SPACE (sizeof (struct timespec))];
> +    } control;
> +
> +    do_sendto (&srv_addr, 42);
> +    do_recvmsg_ancillary (mc, &mmhdr, &control, sizeof control, 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, 0);
> +      }
> +  }
> +}
> +
> +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)

This is wrong for x32 which has 64-bit time_t from day one, but __WORDSIZE
is 32.

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


-- 
H.J.

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

* Re: [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-01-28 17:50 ` [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
  2022-01-28 18:19   ` Florian Weimer
  2022-01-29  4:51   ` H.J. Lu
@ 2022-02-03 18:34   ` Szabolcs Nagy
  2022-02-03 18:36     ` Adhemerval Zanella
  2 siblings, 1 reply; 9+ messages in thread
From: Szabolcs Nagy @ 2022-02-03 18:34 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Florian Weimer

The 01/28/2022 14:50, Adhemerval Zanella via Libc-alpha wrote:
> 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


i think this can cause a build issue on 32bit timesize targets:

https://sourceware.org/bugzilla/show_bug.cgi?id=28860

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

* Re: [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg
  2022-02-03 18:34   ` Szabolcs Nagy
@ 2022-02-03 18:36     ` Adhemerval Zanella
  0 siblings, 0 replies; 9+ messages in thread
From: Adhemerval Zanella @ 2022-02-03 18:36 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: libc-alpha, Florian Weimer



On 03/02/2022 15:34, Szabolcs Nagy wrote:
> The 01/28/2022 14:50, Adhemerval Zanella via Libc-alpha wrote:
>> 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
> 
> 
> i think this can cause a build issue on 32bit timesize targets:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=28860

Yeah, I will fix it (the __convert_scm_timestamps should be always
be built now instead of just for __ASSUME_TIME64_SYSCALLS).

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

end of thread, other threads:[~2022-02-03 18:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 17:50 [PATCH v6 0/3] Fix socket ancillary timestamp on 32 bit time_t ABIs Adhemerval Zanella
2022-01-28 17:50 ` [PATCH v6 1/3] support: Add support_socket_so_timestamp_time64 Adhemerval Zanella
2022-01-28 17:50 ` [PATCH v6 2/3] linux: Fix ancillary 64-bit time timestamp conversion (BZ #28349, BZ#28350) Adhemerval Zanella
2022-01-28 17:50 ` [PATCH v6 3/3] Linux: Only generate 64 bit timestamps for 64 bit time_t recvmsg/recvmmsg Adhemerval Zanella
2022-01-28 18:19   ` Florian Weimer
2022-01-28 19:41     ` Carlos O'Donell
2022-01-29  4:51   ` H.J. Lu
2022-02-03 18:34   ` Szabolcs Nagy
2022-02-03 18:36     ` 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).