net: rework recvmsg handler msg_name and msg_namelen logic
[ Upstream commit f3d3342602f8bcbf37d7c46641cb9bca7618eb1c ]
This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
to return msg_name to the user.
This prevents numerous uninitialized memory leaks we had in the
recvmsg handlers and makes it harder for new code to accidentally leak
uninitialized memory.
Optimize for the case recvfrom is called with NULL as address. We don't
need to copy the address at all, so set it to NULL before invoking the
recvmsg handler. We can do so, because all the recvmsg handlers must
cope with the case a plain read() is called on them. read() also sets
msg_name to NULL.
Also document these changes in include/linux/net.h as suggested by David
Miller.
Changes since RFC:
Set msg->msg_name = NULL if user specified a NULL in msg_name but had a
non-null msg_namelen in verify_iovec/verify_compat_iovec. This doesn't
affect sendto as it would bail out earlier while trying to copy-in the
address. It also more naturally reflects the logic by the callers of
verify_iovec.
With this change in place I could remove "
if (!uaddr || msg_sys->msg_namelen == 0)
msg->msg_name = NULL
".
This change does not alter the user visible error logic as we ignore
msg_namelen as long as msg_name is NULL.
Also remove two unnecessary curly brackets in ___sys_recvmsg and change
comments to netdev style.
Cc: David Miller <davem@davemloft.net>
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index bfa9ab9..334d4cd 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1740,7 +1740,6 @@
size_t size, int flags)
{
struct sock *sk = sock->sk;
- struct sockaddr_at *sat = (struct sockaddr_at *)msg->msg_name;
struct ddpehdr *ddp;
int copied = 0;
int offset = 0;
@@ -1769,14 +1768,13 @@
}
err = skb_copy_datagram_iovec(skb, offset, msg->msg_iov, copied);
- if (!err) {
- if (sat) {
- sat->sat_family = AF_APPLETALK;
- sat->sat_port = ddp->deh_sport;
- sat->sat_addr.s_node = ddp->deh_snode;
- sat->sat_addr.s_net = ddp->deh_snet;
- }
- msg->msg_namelen = sizeof(*sat);
+ if (!err && msg->msg_name) {
+ struct sockaddr_at *sat = msg->msg_name;
+ sat->sat_family = AF_APPLETALK;
+ sat->sat_port = ddp->deh_sport;
+ sat->sat_addr.s_node = ddp->deh_snode;
+ sat->sat_addr.s_net = ddp->deh_snet;
+ msg->msg_namelen = sizeof(*sat);
}
skb_free_datagram(sk, skb); /* Free the datagram. */
diff --git a/net/atm/common.c b/net/atm/common.c
index f0a9b7e..0c0ad93 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -520,8 +520,6 @@
struct sk_buff *skb;
int copied, error = -EINVAL;
- msg->msg_namelen = 0;
-
if (sock->state != SS_CONNECTED)
return -ENOTCONN;
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 68b3992..ca1820c 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1640,11 +1640,11 @@
skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
- if (msg->msg_namelen != 0) {
- struct sockaddr_ax25 *sax = (struct sockaddr_ax25 *)msg->msg_name;
+ if (msg->msg_name) {
ax25_digi digi;
ax25_address src;
const unsigned char *mac = skb_mac_header(skb);
+ struct sockaddr_ax25 *sax = msg->msg_name;
memset(sax, 0, sizeof(struct full_sockaddr_ax25));
ax25_addr_parse(mac + 1, skb->data - mac - 1, &src, NULL,
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index c294348..af3775f 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -240,8 +240,6 @@
if (flags & (MSG_OOB))
return -EOPNOTSUPP;
- msg->msg_namelen = 0;
-
skb = skb_recv_datagram(sk, flags, noblock, &err);
if (!skb) {
if (sk->sk_shutdown & RCV_SHUTDOWN)
@@ -306,8 +304,6 @@
if (flags & MSG_OOB)
return -EOPNOTSUPP;
- msg->msg_namelen = 0;
-
BT_DBG("sk %p size %zu", sk, size);
lock_sock(sk);
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index bedc768..8c2cb05 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -767,8 +767,6 @@
if (!skb)
return err;
- msg->msg_namelen = 0;
-
copied = skb->len;
if (len < copied) {
msg->msg_flags |= MSG_TRUNC;
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index c79db7f..8d1edd7 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -628,7 +628,6 @@
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
rfcomm_dlc_accept(d);
- msg->msg_namelen = 0;
return 0;
}
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 24a6886..9e83515 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -287,8 +287,6 @@
if (m->msg_flags&MSG_OOB)
goto read_error;
- m->msg_namelen = 0;
-
skb = skb_recv_datagram(sk, flags, 0 , &ret);
if (!skb)
goto read_error;
@@ -362,8 +360,6 @@
if (flags&MSG_OOB)
goto out;
- msg->msg_namelen = 0;
-
/*
* Lock the socket to prevent queue disordering
* while sleeps in memcpy_tomsg
diff --git a/net/compat.c b/net/compat.c
index ee84d82..318a876 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -93,7 +93,8 @@
if (err < 0)
return err;
}
- kern_msg->msg_name = kern_address;
+ if (kern_msg->msg_name)
+ kern_msg->msg_name = kern_address;
} else
kern_msg->msg_name = NULL;
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 7e7aeb0..7fd34a5 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -48,7 +48,8 @@
if (err < 0)
return err;
}
- m->msg_name = address;
+ if (m->msg_name)
+ m->msg_name = address;
} else {
m->msg_name = NULL;
}
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index 9680226..8c06a50 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1835,8 +1835,6 @@
if (skb->tstamp.tv64)
sk->sk_stamp = skb->tstamp;
- msg->msg_namelen = sizeof(*sipx);
-
if (sipx) {
sipx->sipx_family = AF_IPX;
sipx->sipx_port = ipx->ipx_source.sock;
@@ -1844,6 +1842,7 @@
sipx->sipx_network = IPX_SKB_CB(skb)->ipx_source_net;
sipx->sipx_type = ipx->ipx_type;
sipx->sipx_zero = 0;
+ msg->msg_namelen = sizeof(*sipx);
}
rc = copied;
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index bd25678..12218f7 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1386,8 +1386,6 @@
IRDA_DEBUG(4, "%s()\n", __func__);
- msg->msg_namelen = 0;
-
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &err);
if (!skb)
@@ -1452,8 +1450,6 @@
target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
timeo = sock_rcvtimeo(sk, noblock);
- msg->msg_namelen = 0;
-
do {
int chunk;
struct sk_buff *skb = skb_dequeue(&sk->sk_receive_queue);
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 625bc50..cd6f7a9 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1331,8 +1331,6 @@
struct sk_buff *skb, *rskb, *cskb;
int err = 0;
- msg->msg_namelen = 0;
-
if ((sk->sk_state == IUCV_DISCONN) &&
skb_queue_empty(&iucv->backlog_skb_q) &&
skb_queue_empty(&sk->sk_receive_queue) &&
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 2f3ce93..d5cd439 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3595,7 +3595,6 @@
if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
goto out;
- msg->msg_namelen = 0;
skb = skb_recv_datagram(sk, flags, flags & MSG_DONTWAIT, &err);
if (skb == NULL)
goto out;
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 904bc09..2211275 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -200,8 +200,6 @@
if (sk->sk_state & PPPOX_BOUND)
goto end;
- msg->msg_namelen = 0;
-
err = 0;
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &err);
diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index e4d2fbb..df08d77 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -721,8 +721,6 @@
int target; /* Read at least this many bytes */
long timeo;
- msg->msg_namelen = 0;
-
lock_sock(sk);
copied = -ENOTCONN;
if (unlikely(sk->sk_type == SOCK_STREAM && sk->sk_state == TCP_LISTEN))
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 9017e3e..ff960b7 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1443,8 +1443,6 @@
}
#endif
- msg->msg_namelen = 0;
-
copied = data_skb->len;
if (len < copied) {
msg->msg_flags |= MSG_TRUNC;
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 7ed9b1d..dcf6791 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -1181,10 +1181,9 @@
sax->sax25_family = AF_NETROM;
skb_copy_from_linear_data_offset(skb, 7, sax->sax25_call.ax25_call,
AX25_ADDR_LEN);
+ msg->msg_namelen = sizeof(*sax);
}
- msg->msg_namelen = sizeof(*sax);
-
skb_free_datagram(sk, skb);
release_sock(sk);
diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
index e879dce..44835ce 100644
--- a/net/nfc/rawsock.c
+++ b/net/nfc/rawsock.c
@@ -235,8 +235,6 @@
if (!skb)
return rc;
- msg->msg_namelen = 0;
-
copied = skb->len;
if (len < copied) {
msg->msg_flags |= MSG_TRUNC;
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 8ed5d93..b7d6d7f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2691,7 +2691,6 @@
struct sock *sk = sock->sk;
struct sk_buff *skb;
int copied, err;
- struct sockaddr_ll *sll;
int vnet_hdr_len = 0;
err = -EINVAL;
@@ -2774,22 +2773,10 @@
goto out_free;
}
- /*
- * If the address length field is there to be filled in, we fill
- * it in now.
+ /* You lose any data beyond the buffer you gave. If it worries
+ * a user program they can ask the device for its MTU
+ * anyway.
*/
-
- sll = &PACKET_SKB_CB(skb)->sa.ll;
- if (sock->type == SOCK_PACKET)
- msg->msg_namelen = sizeof(struct sockaddr_pkt);
- else
- msg->msg_namelen = sll->sll_halen + offsetof(struct sockaddr_ll, sll_addr);
-
- /*
- * You lose any data beyond the buffer you gave. If it worries a
- * user program they can ask the device for its MTU anyway.
- */
-
copied = skb->len;
if (copied > len) {
copied = len;
@@ -2802,9 +2789,20 @@
sock_recv_ts_and_drops(msg, sk, skb);
- if (msg->msg_name)
+ if (msg->msg_name) {
+ /* If the address length field is there to be filled
+ * in, we fill it in now.
+ */
+ if (sock->type == SOCK_PACKET) {
+ msg->msg_namelen = sizeof(struct sockaddr_pkt);
+ } else {
+ struct sockaddr_ll *sll = &PACKET_SKB_CB(skb)->sa.ll;
+ msg->msg_namelen = sll->sll_halen +
+ offsetof(struct sockaddr_ll, sll_addr);
+ }
memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa,
msg->msg_namelen);
+ }
if (pkt_sk(sk)->auxdata) {
struct tpacket_auxdata aux;
diff --git a/net/rds/recv.c b/net/rds/recv.c
index 9f0f17c..de339b2 100644
--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -410,8 +410,6 @@
rdsdebug("size %zu flags 0x%x timeo %ld\n", size, msg_flags, timeo);
- msg->msg_namelen = 0;
-
if (msg_flags & MSG_OOB)
goto out;
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 7f645d1..ce5f5b9 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -1220,7 +1220,6 @@
{
struct sock *sk = sock->sk;
struct rose_sock *rose = rose_sk(sk);
- struct sockaddr_rose *srose = (struct sockaddr_rose *)msg->msg_name;
size_t copied;
unsigned char *asmptr;
struct sk_buff *skb;
@@ -1256,8 +1255,11 @@
skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
- if (srose != NULL) {
- memset(srose, 0, msg->msg_namelen);
+ if (msg->msg_name) {
+ struct sockaddr_rose *srose;
+
+ memset(msg->msg_name, 0, sizeof(struct full_sockaddr_rose));
+ srose = msg->msg_name;
srose->srose_family = AF_ROSE;
srose->srose_addr = rose->dest_addr;
srose->srose_call = rose->dest_call;
diff --git a/net/rxrpc/ar-recvmsg.c b/net/rxrpc/ar-recvmsg.c
index 4b48687..898492a 100644
--- a/net/rxrpc/ar-recvmsg.c
+++ b/net/rxrpc/ar-recvmsg.c
@@ -143,10 +143,13 @@
/* copy the peer address and timestamp */
if (!continue_call) {
- if (msg->msg_name && msg->msg_namelen > 0)
+ if (msg->msg_name) {
+ size_t len =
+ sizeof(call->conn->trans->peer->srx);
memcpy(msg->msg_name,
- &call->conn->trans->peer->srx,
- sizeof(call->conn->trans->peer->srx));
+ &call->conn->trans->peer->srx, len);
+ msg->msg_namelen = len;
+ }
sock_recv_ts_and_drops(msg, &rx->sk, skb);
}
diff --git a/net/socket.c b/net/socket.c
index acc7695..f379740 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1775,8 +1775,10 @@
msg.msg_iov = &iov;
iov.iov_len = size;
iov.iov_base = ubuf;
- msg.msg_name = (struct sockaddr *)&address;
- msg.msg_namelen = sizeof(address);
+ /* Save some cycles and don't copy the address if not needed */
+ msg.msg_name = addr ? (struct sockaddr *)&address : NULL;
+ /* We assume all kernel code knows the size of sockaddr_storage */
+ msg.msg_namelen = 0;
if (sock->file->f_flags & O_NONBLOCK)
flags |= MSG_DONTWAIT;
err = sock_recvmsg(sock, &msg, size, flags);
@@ -2161,16 +2163,14 @@
goto out;
}
- /*
- * Save the user-mode address (verify_iovec will change the
- * kernel msghdr to use the kernel address space)
+ /* Save the user-mode address (verify_iovec will change the
+ * kernel msghdr to use the kernel address space)
*/
-
uaddr = (__force void __user *)msg_sys->msg_name;
uaddr_len = COMPAT_NAMELEN(msg);
- if (MSG_CMSG_COMPAT & flags) {
+ if (MSG_CMSG_COMPAT & flags)
err = verify_compat_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
- } else
+ else
err = verify_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
if (err < 0)
goto out_freeiov;
@@ -2179,6 +2179,9 @@
cmsg_ptr = (unsigned long)msg_sys->msg_control;
msg_sys->msg_flags = flags & (MSG_CMSG_CLOEXEC|MSG_CMSG_COMPAT);
+ /* We assume all kernel code knows the size of sockaddr_storage */
+ msg_sys->msg_namelen = 0;
+
if (sock->file->f_flags & O_NONBLOCK)
flags |= MSG_DONTWAIT;
err = (nosec ? sock_recvmsg_nosec : sock_recvmsg)(sock, msg_sys,
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 1441ab7..64b8470 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -949,9 +949,6 @@
goto exit;
}
- /* will be updated in set_orig_addr() if needed */
- m->msg_namelen = 0;
-
timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
restart:
@@ -1078,9 +1075,6 @@
goto exit;
}
- /* will be updated in set_orig_addr() if needed */
- m->msg_namelen = 0;
-
target = sock_rcvlowat(sk, flags & MSG_WAITALL, buf_len);
timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
restart:
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index ed005b4..0540dd9 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1755,7 +1755,6 @@
{
struct unix_sock *u = unix_sk(sk);
- msg->msg_namelen = 0;
if (u->addr) {
msg->msg_namelen = u->addr->len;
memcpy(msg->msg_name, u->addr->name, u->addr->len);
@@ -1779,8 +1778,6 @@
if (flags&MSG_OOB)
goto out;
- msg->msg_namelen = 0;
-
err = mutex_lock_interruptible(&u->readlock);
if (err) {
err = sock_intr_errno(sock_rcvtimeo(sk, noblock));
@@ -1922,8 +1919,6 @@
target = sock_rcvlowat(sk, flags&MSG_WAITALL, size);
timeo = sock_rcvtimeo(sk, flags&MSG_DONTWAIT);
- msg->msg_namelen = 0;
-
/* Lock the socket to prevent queue disordering
* while sleeps in memcpy_tomsg
*/
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index b943e3e..92aed9e 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1343,10 +1343,9 @@
if (sx25) {
sx25->sx25_family = AF_X25;
sx25->sx25_addr = x25->dest_addr;
+ msg->msg_namelen = sizeof(*sx25);
}
- msg->msg_namelen = sizeof(struct sockaddr_x25);
-
x25_check_rbuf(sk);
rc = copied;
out_free_dgram: