linux-kernel - Re: sendmmsg flags userspace ABI change in kernel 4.6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1524413453.3097.2.camel@klomp.org>
Date:   Sun, 22 Apr 2018 18:10:53 +0200
From:   Mark Wielaard <mark@...mp.org>
To:     Florian Weimer <fw@...eb.enyo.de>, linux-api@...r.kernel.org,
        netdev@...r.kernel.org, linux-sctp@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, Tom Herbert <tom@...bertland.com>,
        valgrind-developers@...ts.sourceforge.net
Subject: 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 <tom@...bertland.com>
> 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 <tom@...bertland.com>
>     Signed-off-by: David S. Miller <davem@...emloft.net>
> 
> 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