Re: sendmmsg flags userspace ABI change in kernel 4.6

Hi,

Adding valgrind-developers to CC.

On Wed, 2018-04-18 at 21:03 +0200, Florian Weimer wrote:

Since this commit:

commit 28a94d8fb35b3a75b802f368ae6f4a9f6b0d435a
Author: Tom Herbert [off-list ref]
Date:   Mon Mar 7 14:11:02 2016 -0800

    net: Allow MSG_EOR in each msghdr of sendmmsg

    This patch allows setting MSG_EOR in each individual msghdr passed
    in sendmmsg. This allows a sendmmsg to send multiple messages when
    using SOCK_SEQPACKET.

    Signed-off-by: Tom Herbert [off-list ref]
    Signed-off-by: David S. Miller [off-list ref]

the msg_flags argument in individual msghdr arguments is longer
completely ignored for SOCK_SEQPACKET sockets.  msg_flags was and is
still documented as ignored for sendmsg(2), so by analogy for
sendmmsg(2) as well.

It seems that valgrind does not know about this yet, and due to
limited use of SCTP, this userspace ABI change has not been noticed so
far.

That is correct. If you look at coregrind/m_syswrap/syswrap-generic.c
(msghdr_foreachfield), it explicitly says:

   /* msg_flags is completely ignored for send_mesg, recv_mesg doesn't read
      the field, but does write to it. */

So the various valgrind syscall wrappers will not be called to inspect
the msg_flags field. This means you don't get warned if the msg_flags
field contains undefined bits for any syscall send_msg variant.

What are the plans in this area?  Will other kinds of sockets start
using the msghdr flags for sending?

A fully backwards-compatibility way to achieve this would be to
specify that you have to pass a new flag to sendmmsg (MSG_PERHDR?), in
its flags argument, to activate the per-msghdr flags.

The glibc DNS stub resolver relies on the previously documented
behavior, and I wonder how widely we should backport the change:

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

If the MSG_PERHDR route will be taken, we can skip this work, and
valgrind can flag uninitialized bits in msg_flags only if MSG_PERHDR
is passed.  (I believe it would be difficult for valgrind to look at
the socket type to determine whether undefined bits need reporting.)

We have abstracted the checking a bit so it can be reused for the
different ways a msg can be send, through socketcall, sendmsg and
sendmmsg. I am not sure we can easily determine the underlying socket
type. It seems to mean we would have to track the creation of all
socketfds. So having a flag bit for sendmsg and sendmmsg would be much
nicer.

Thanks,

Mark