RE: [RFC PATCH 0/3] net: Drop size arg from ->sendmsg() and pass msghdr into __ip{,6}_append_data()

From: Willem de Bruijn
Date: Wed Mar 22 2023 - 21:17:08 EST


David Howells wrote:
> Hi Willem,
>
> Here's another option to passing MSG_SPLICE_PAGES into sendmsg()[1] without
> polluting the flags in msg->msg_flags. The idea here is to put the flag
> into a new field in msghdr, msg_kflags, that holds internal kernel flags
> that aren't available to userspace.
>
> What I've done here is:
>
> (1) Pass msg down to __ip_append_data() and __ip6_append_data() so that
> they can access the extra flags.
>
> (2) In order to avoid adding extra arguments to these functions and the
> functions in their call chains (such as ip_make_skb()), remove the
> size and flags arguments as these values are redundant if msg is
> passed in.
>
> (3) msg is then passed into getfrag(). I would like to get rid of the
> "from" argument also in favour of using something in msghdr, but I'm
> not sure how best to do that.
>
> (4) The size parameter to ->sendmsg() seems to be redundant; indeed
> sock_sendmsg() doesn't actually take it, but rather gets the count
> from msg_iter - so remove this parameter.
>
> kernel_sendmsg() will still take a size, but it sets it on the
> iterator and then calls sock_sendmsg().
>
> (5) Protocol sendmsg implementations then extract the length and the flags
> from the iterator.
>
> (6) Illustrate the addition of msg_kflags and MSG_SPLICE_PAGES. I think
> that, at some point in the future, some of the other flags could be
> moved from msg_flags to msg_kflags.
>
> David
>
> Link: https://lore.kernel.org/r/20230316152618.711970-1-dhowells@xxxxxxxxxx/ [1]
>
> David Howells (3):
> net: Drop the size argument from ->sendmsg()
> ip: Make __ip{,6}_append_data() and co. take a msghdr*
> net: Declare MSG_SPLICE_PAGES internal sendmsg() flag
>
> crypto/af_alg.c | 12 +--
> crypto/algif_aead.c | 9 +--
> crypto/algif_hash.c | 8 +-
> crypto/algif_rng.c | 3 +-
> crypto/algif_skcipher.c | 10 +--
> drivers/isdn/mISDN/socket.c | 3 +-
> .../chelsio/inline_crypto/chtls/chtls.h | 2 +-
> .../chelsio/inline_crypto/chtls/chtls_io.c | 15 ++--
> drivers/net/ppp/pppoe.c | 4 +-
> drivers/net/tap.c | 3 +-
> drivers/net/tun.c | 3 +-
> drivers/vhost/net.c | 6 +-
> drivers/xen/pvcalls-back.c | 2 +-
> drivers/xen/pvcalls-front.c | 4 +-
> drivers/xen/pvcalls-front.h | 3 +-
> fs/afs/rxrpc.c | 8 +-
> include/crypto/if_alg.h | 3 +-
> include/linux/lsm_hook_defs.h | 3 +-
> include/linux/lsm_hooks.h | 1 -
> include/linux/net.h | 6 +-
> include/linux/security.h | 4 +-
> include/linux/socket.h | 3 +
> include/net/af_rxrpc.h | 3 +-
> include/net/inet_common.h | 2 +-
> include/net/ip.h | 24 +++---
> include/net/ipv6.h | 22 +++---
> include/net/ping.h | 7 +-
> include/net/sock.h | 7 +-
> include/net/tcp.h | 8 +-
> include/net/udp.h | 2 +-
> include/net/udplite.h | 4 +-
> net/appletalk/ddp.c | 3 +-
> net/atm/common.c | 3 +-
> net/atm/common.h | 2 +-
> net/ax25/af_ax25.c | 4 +-
> net/bluetooth/hci_sock.c | 4 +-
> net/bluetooth/iso.c | 4 +-
> net/bluetooth/l2cap_sock.c | 5 +-
> net/bluetooth/rfcomm/sock.c | 7 +-
> net/bluetooth/sco.c | 4 +-
> net/caif/caif_socket.c | 13 ++--
> net/can/bcm.c | 3 +-
> net/can/isotp.c | 3 +-
> net/can/j1939/socket.c | 4 +-
> net/can/raw.c | 3 +-
> net/core/sock.c | 4 +-
> net/dccp/dccp.h | 2 +-
> net/dccp/proto.c | 3 +-
> net/ieee802154/socket.c | 11 +--
> net/ipv4/af_inet.c | 4 +-
> net/ipv4/icmp.c | 14 ++--
> net/ipv4/ip_output.c | 73 ++++++++++---------
> net/ipv4/ping.c | 18 ++---
> net/ipv4/raw.c | 23 +++---
> net/ipv4/tcp.c | 17 +++--
> net/ipv4/tcp_bpf.c | 5 +-
> net/ipv4/tcp_input.c | 3 +-
> net/ipv4/udp.c | 24 +++---
> net/ipv6/af_inet6.c | 7 +-
> net/ipv6/icmp.c | 21 ++++--
> net/ipv6/ip6_output.c | 57 +++++++--------
> net/ipv6/ping.c | 12 +--
> net/ipv6/raw.c | 25 +++----
> net/ipv6/udp.c | 26 ++++---
> net/ipv6/udp_impl.h | 2 +-
> net/iucv/af_iucv.c | 4 +-
> net/kcm/kcmsock.c | 2 +-
> net/key/af_key.c | 3 +-
> net/l2tp/l2tp_ip.c | 3 +-
> net/l2tp/l2tp_ip6.c | 3 +-
> net/l2tp/l2tp_ppp.c | 4 +-
> net/llc/af_llc.c | 5 +-
> net/mctp/af_mctp.c | 3 +-
> net/mptcp/protocol.c | 8 +-
> net/netlink/af_netlink.c | 11 +--
> net/netrom/af_netrom.c | 3 +-
> net/nfc/llcp_sock.c | 7 +-
> net/nfc/rawsock.c | 3 +-
> net/packet/af_packet.c | 11 +--
> net/phonet/datagram.c | 3 +-
> net/phonet/pep.c | 3 +-
> net/phonet/socket.c | 5 +-
> net/qrtr/af_qrtr.c | 4 +-
> net/rds/rds.h | 2 +-
> net/rds/send.c | 3 +-
> net/rose/af_rose.c | 3 +-
> net/rxrpc/af_rxrpc.c | 6 +-
> net/rxrpc/ar-internal.h | 2 +-
> net/rxrpc/output.c | 22 +++---
> net/rxrpc/rxperf.c | 4 +-
> net/rxrpc/sendmsg.c | 15 ++--
> net/sctp/socket.c | 3 +-
> net/smc/af_smc.c | 5 +-
> net/socket.c | 16 ++--
> net/tipc/socket.c | 34 ++++-----
> net/tls/tls.h | 4 +-
> net/tls/tls_device.c | 5 +-
> net/tls/tls_sw.c | 2 +-
> net/unix/af_unix.c | 19 +++--
> net/vmw_vsock/af_vsock.c | 16 ++--
> net/x25/af_x25.c | 3 +-
> net/xdp/xsk.c | 6 +-
> net/xfrm/espintcp.c | 8 +-
> security/apparmor/lsm.c | 6 +-
> security/security.c | 4 +-
> security/selinux/hooks.c | 3 +-
> security/smack/smack_lsm.c | 4 +-
> security/tomoyo/common.h | 3 +-
> security/tomoyo/network.c | 4 +-
> security/tomoyo/tomoyo.c | 6 +-
> 110 files changed, 444 insertions(+), 456 deletions(-)

That's a significant code change if only for this purpose.

If this bit is undefined and ignored by all socket families today,
masking it out in sock_sendmsg should be enough to start using it
safely as an internal flag.