Re: [PATCH v3 05/24] linux: Add fallback for 64-bit time_t SO_TIMESTAMP{NS}

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella via Libc-alpha:

> +/* It converts the first SO_TIMESTAMP or SO_TIMESTAMPNS with 32-bit time and
> +   appends it to the control buffer.  The 32-bit time field is kept as-is.
> +
> +   Calls with __TIMESIZE=32 will see the converted 64-bit time control
> +   messages as spurious control message of unknown type.
> +
> +   Calls with __TIMESIZE=64 running on pre-time64 kernels will see the
> +   original message as a spurious control ones of unknown typ while running
> +   on kernel with native 64-bit time support will only see the time64 version
> +   of the control message.  */
> +void
> +__convert_scm_timestamps (struct msghdr *msg, socklen_t msgsize)
> +{
> +  if (msg->msg_control == NULL || msg->msg_controllen == 0)
> +    return;
> +
> +  /* The returned control message format for SO_TIMESTAMP_NEW is a
> +     'struct __kernel_sock_timeval' while for SO_TIMESTAMPNS_NEW is a
> +     'struct __kernel_timespec'.  In either case it is two uint64_t
> +     members.  */
> +  uint64_t tvts[2];
> +
> +  struct cmsghdr *cmsg, *last = NULL;
> +  int type = 0;
> +
> +  for (cmsg = CMSG_FIRSTHDR (msg);
> +       cmsg != NULL;
> +       cmsg = CMSG_NXTHDR (msg, cmsg))
> +    {
> +      if (cmsg->cmsg_level != SOL_SOCKET)
> +	continue;
> +
> +      switch (cmsg->cmsg_type)
> +	{
> +	case COMPAT_SO_TIMESTAMP_OLD:
> +	  if (type != 0)
> +	    break;
> +	  type = COMPAT_SO_TIMESTAMP_NEW;
> +	  goto common;
> +
> +	case COMPAT_SO_TIMESTAMPNS_OLD:
> +	  type = COMPAT_SO_TIMESTAMPNS_NEW;
> +
> +	/* fallthrough  */
> +	common:
> +	  memcpy (tvts, CMSG_DATA (cmsg), sizeof (tvts));
> +	  break;
> +	}
> +
> +      last = cmsg;
> +    }
> +
> +  if (last == NULL || type == 0)
> +    return;
> +
> +  if (CMSG_SPACE (sizeof tvts) > msgsize - msg->msg_controllen)
> +    {
> +      msg->msg_flags |= MSG_CTRUNC;
> +      return;
> +    }
> +
> +  msg->msg_controllen += CMSG_SPACE (sizeof tvts);
> +  cmsg = CMSG_NXTHDR(msg, last);
> +  cmsg->cmsg_level = SOL_SOCKET;
> +  cmsg->cmsg_type = type;
> +  cmsg->cmsg_len = CMSG_LEN (sizeof tvts);
> +  memcpy (CMSG_DATA (cmsg), tvts, sizeof tvts);
> +}
> +libc_hidden_def (__convert_scm_timestamps)
> +#endif

The Ruby test suite crashes on this line:

  cmsg->cmsg_level = SOL_SOCKET;

See:

  ruby: FTBFS with test suite failure (glibc 2.34 related)
  <https://bugzilla.redhat.com/show_bug.cgi?id=1975144>

The disassembly suggests that GCC has detected some undefined behavior.

This looks like a related bug:

  __cmsg_nxthdr in cmsg_nxthdr.c (CMSG_NXTHDR) has undefined behavior when setting up ancillary data
  <https://sourceware.org/bugzilla/show_bug.cgi?id=13500>

I believe you cannot use CMSG_NXTHDR to append data in this way.

The other question is why this code is running at all.  Doing this
complex conversion for a 32-bit applications doing a 32-bit function
call on a kernel which supports 32-bit system calls does not make much
sense to me.

Thanks,
Florian