[Acked w/ cmt] [Lucid][CVE-2013-7266] net: rework recvmsg handler msg_name and msg_namelen logic

Andy Whitcroft apw at canonical.com
Mon Jan 20 12:31:14 UTC 2014


On Fri, Jan 17, 2014 at 02:31:09PM +0000, Luis Henriques wrote:
> From: Hannes Frederic Sowa <hannes at stressinduktion.org>
> 
> CVE-2013-7266
> 
> BugLink: http://bugs.launchpad.net/bugs/1267081
> 
> 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 at davemloft.net>
> Suggested-by: Eric Dumazet <eric.dumazet at gmail.com>
> Signed-off-by: Hannes Frederic Sowa <hannes at stressinduktion.org>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (back ported from commit f3d3342602f8bcbf37d7c46641cb9bca7618eb1c)
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>  drivers/isdn/mISDN/socket.c  | 13 ++++---------
>  drivers/net/pppoe.c          |  2 --
>  drivers/net/pppol2tp.c       |  2 --
>  include/linux/net.h          |  8 ++++++++
>  net/appletalk/ddp.c          | 16 +++++++---------
>  net/atm/common.c             |  2 --
>  net/ax25/af_ax25.c           |  4 ++--
>  net/bluetooth/af_bluetooth.c |  2 --
>  net/bluetooth/hci_sock.c     |  2 --
>  net/bluetooth/rfcomm/sock.c  |  3 ---
>  net/compat.c                 |  3 ++-
>  net/core/iovec.c             |  3 ++-
>  net/ipx/af_ipx.c             |  3 +--
>  net/irda/af_irda.c           |  4 ----
>  net/iucv/af_iucv.c           |  2 --
>  net/key/af_key.c             |  1 -
>  net/llc/af_llc.c             |  2 --
>  net/netlink/af_netlink.c     |  2 --
>  net/netrom/af_netrom.c       |  3 +--
>  net/packet/af_packet.c       | 32 +++++++++++++++-----------------
>  net/rds/recv.c               |  2 --
>  net/rose/af_rose.c           |  8 +++++---
>  net/rxrpc/ar-recvmsg.c       |  9 ++++++---
>  net/socket.c                 | 19 +++++++++++--------
>  net/tipc/socket.c            |  6 ------
>  net/unix/af_unix.c           |  5 -----
>  net/x25/af_x25.c             |  3 +--
>  27 files changed, 65 insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
> index feb0fa4..db69cb4 100644
> --- a/drivers/isdn/mISDN/socket.c
> +++ b/drivers/isdn/mISDN/socket.c
> @@ -115,7 +115,6 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  {
>  	struct sk_buff		*skb;
>  	struct sock		*sk = sock->sk;
> -	struct sockaddr_mISDN	*maddr;
>  
>  	int		copied, err;
>  
> @@ -133,9 +132,9 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	if (!skb)
>  		return err;
>  
> -	if (msg->msg_namelen >= sizeof(struct sockaddr_mISDN)) {
> -		msg->msg_namelen = sizeof(struct sockaddr_mISDN);
> -		maddr = (struct sockaddr_mISDN *)msg->msg_name;
> +	if (msg->msg_name) {
> +		struct sockaddr_mISDN *maddr = msg->msg_name;
> +
>  		maddr->family = AF_ISDN;
>  		maddr->dev = _pms(sk)->dev->id;
>  		if ((sk->sk_protocol == ISDN_P_LAPD_TE) ||
> @@ -148,11 +147,7 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  			maddr->sapi = _pms(sk)->ch.addr & 0xFF;
>  			maddr->tei =  (_pms(sk)->ch.addr >> 8) & 0xFF;
>  		}
> -	} else {
> -		if (msg->msg_namelen)
> -			printk(KERN_WARNING "%s: too small namelen %d\n",
> -			    __func__, msg->msg_namelen);
> -		msg->msg_namelen = 0;
> +		msg->msg_namelen = sizeof(*maddr);
>  	}
>  
>  	copied = skb->len + MISDN_HEADER_LEN;
> diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
> index 2559991..343fd1e 100644
> --- a/drivers/net/pppoe.c
> +++ b/drivers/net/pppoe.c
> @@ -992,8 +992,6 @@ static int pppoe_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	if (error < 0)
>  		goto end;
>  
> -	m->msg_namelen = 0;
> -
>  	if (skb) {
>  		total_len = min_t(size_t, total_len, skb->len);
>  		error = skb_copy_datagram_iovec(skb, 0, m->msg_iov, total_len);
> diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
> index 9235901..4cdc1cf 100644
> --- a/drivers/net/pppol2tp.c
> +++ b/drivers/net/pppol2tp.c
> @@ -829,8 +829,6 @@ static int pppol2tp_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	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/include/linux/net.h b/include/linux/net.h
> index 529a093..e40cbcc 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -187,6 +187,14 @@ struct proto_ops {
>  				      int optname, char __user *optval, int __user *optlen);
>  	int		(*sendmsg)   (struct kiocb *iocb, struct socket *sock,
>  				      struct msghdr *m, size_t total_len);
> +	/* Notes for implementing recvmsg:
> +	 * ===============================
> +	 * msg->msg_namelen should get updated by the recvmsg handlers
> +	 * iff msg_name != NULL. It is by default 0 to prevent
> +	 * returning uninitialized memory to user space.  The recvfrom
> +	 * handlers can assume that msg.msg_name is either NULL or has
> +	 * a minimum size of sizeof(struct sockaddr_storage).
> +	 */
>  	int		(*recvmsg)   (struct kiocb *iocb, struct socket *sock,
>  				      struct msghdr *m, size_t total_len,
>  				      int flags);
> diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
> index b1a4290..5eae360 100644
> --- a/net/appletalk/ddp.c
> +++ b/net/appletalk/ddp.c
> @@ -1703,7 +1703,6 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
>  			 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;
> @@ -1728,14 +1727,13 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
>  	}
>  	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 65737b8..0baf05e 100644
> --- a/net/atm/common.c
> +++ b/net/atm/common.c
> @@ -473,8 +473,6 @@ int vcc_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
>  	struct sk_buff *skb;
>  	int copied, error = -EINVAL;
>  
> -	msg->msg_namelen = 0;
> -
>  	if (sock->state != SS_CONNECTED)
>  		return -ENOTCONN;
>  	if (flags & ~MSG_DONTWAIT)		/* only handle MSG_DONTWAIT */
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
> index 8613bd1..6b9d62b 100644
> --- a/net/ax25/af_ax25.c
> +++ b/net/ax25/af_ax25.c
> @@ -1648,11 +1648,11 @@ static int ax25_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  	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 d7239dd..143b8a7 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -240,8 +240,6 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	if (flags & (MSG_OOB))
>  		return -EOPNOTSUPP;
>  
> -	msg->msg_namelen = 0;
> -
>  	if (!(skb = skb_recv_datagram(sk, flags, noblock, &err))) {
>  		if (sk->sk_shutdown & RCV_SHUTDOWN)
>  			return 0;
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 6c00bf7..bb2548b 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -370,8 +370,6 @@ static int hci_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	if (!(skb = skb_recv_datagram(sk, flags, noblock, &err)))
>  		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 1db0132..3fabaad 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -652,15 +652,12 @@ static int rfcomm_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  	if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
>  		rfcomm_dlc_accept(d);
> -		msg->msg_namelen = 0;
>  		return 0;
>  	}
>  
>  	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/compat.c b/net/compat.c
> index 9559afc..305bca6 100644
> --- a/net/compat.c
> +++ b/net/compat.c
> @@ -89,7 +89,8 @@ int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
>  			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 f911e66..39369e9 100644
> --- a/net/core/iovec.c
> +++ b/net/core/iovec.c
> @@ -47,7 +47,8 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr *address,
>  			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 66c7a20..25931b3 100644
> --- a/net/ipx/af_ipx.c
> +++ b/net/ipx/af_ipx.c
> @@ -1808,8 +1808,6 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	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;
> @@ -1817,6 +1815,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
>  		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 bfb325d..7cb7613 100644
> --- a/net/irda/af_irda.c
> +++ b/net/irda/af_irda.c
> @@ -1338,8 +1338,6 @@ static int irda_recvmsg_dgram(struct kiocb *iocb, struct socket *sock,
>  	if ((err = sock_error(sk)) < 0)
>  		return err;
>  
> -	msg->msg_namelen = 0;
> -
>  	skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
>  				flags & MSG_DONTWAIT, &err);
>  	if (!skb)
> @@ -1402,8 +1400,6 @@ static int irda_recvmsg_stream(struct kiocb *iocb, struct socket *sock,
>  	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 f605b23..bada1b9 100644
> --- a/net/iucv/af_iucv.c
> +++ b/net/iucv/af_iucv.c
> @@ -1160,8 +1160,6 @@ static int iucv_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	struct sk_buff *skb, *rskb, *cskb;
>  	int err = 0;
>  
> -	msg->msg_namelen = 0;
> -
>  	if ((sk->sk_state == IUCV_DISCONN || sk->sk_state == IUCV_SEVERED) &&
>  	    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 9d22e46..b6a6b85 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -3593,7 +3593,6 @@ static int pfkey_recvmsg(struct kiocb *kiocb,
>  	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/llc/af_llc.c b/net/llc/af_llc.c
> index 8a814a5..606b6ad 100644
> --- a/net/llc/af_llc.c
> +++ b/net/llc/af_llc.c
> @@ -674,8 +674,6 @@ static int llc_ui_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	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 fc91ff6..39a6d5d 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1400,8 +1400,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
>  	}
>  #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 d240523..b3c9b48 100644
> --- a/net/netrom/af_netrom.c
> +++ b/net/netrom/af_netrom.c
> @@ -1185,10 +1185,9 @@ static int nr_recvmsg(struct kiocb *iocb, struct socket *sock,
>  		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/packet/af_packet.c b/net/packet/af_packet.c
> index 728c080..1de1992 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1423,7 +1423,6 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	struct sock *sk = sock->sk;
>  	struct sk_buff *skb;
>  	int copied, err;
> -	struct sockaddr_ll *sll;
>  
>  	err = -EINVAL;
>  	if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
> @@ -1455,22 +1454,10 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	if (skb == NULL)
>  		goto out;
>  
> -	/*
> -	 *	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;
> @@ -1483,9 +1470,20 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  	sock_recv_timestamp(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 c45a881c..a11cab9 100644
> --- a/net/rds/recv.c
> +++ b/net/rds/recv.c
> @@ -410,8 +410,6 @@ int rds_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
>  
>  	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 2984999..08a86f6 100644
> --- a/net/rose/af_rose.c
> +++ b/net/rose/af_rose.c
> @@ -1238,7 +1238,6 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket *sock,
>  {
>  	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;
> @@ -1274,8 +1273,11 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  	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 a39bf97..d5630d9 100644
> --- a/net/rxrpc/ar-recvmsg.c
> +++ b/net/rxrpc/ar-recvmsg.c
> @@ -142,10 +142,13 @@ int rxrpc_recvmsg(struct kiocb *iocb, struct socket *sock,
>  
>  		/* 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_timestamp(msg, &rx->sk, skb);
>  		}
>  
> diff --git a/net/socket.c b/net/socket.c
> index bf9fc68..e6c3396 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -1744,8 +1744,10 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
>  	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);
> @@ -2017,18 +2019,16 @@ SYSCALL_DEFINE3(recvmsg, int, fd, struct msghdr __user *, msg,
>  			goto out_put;
>  	}
>  
> -	/*
> -	 *      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,
>  					  (struct sockaddr *)&addr,
>  					  VERIFY_WRITE);
> -	} else
> +	else
>  		err = verify_iovec(&msg_sys, iov,
>  				   (struct sockaddr *)&addr,
>  				   VERIFY_WRITE);
> @@ -2039,6 +2039,9 @@ SYSCALL_DEFINE3(recvmsg, int, fd, struct msghdr __user *, msg,
>  	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 = sock_recvmsg(sock, &msg_sys, total_len, flags);
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index b453345..024f490 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -917,9 +917,6 @@ static int recv_msg(struct kiocb *iocb, struct socket *sock,
>  		goto exit;
>  	}
>  
> -	/* will be updated in set_orig_addr() if needed */
> -	m->msg_namelen = 0;
> -
>  restart:
>  
>  	/* Look for a message in receive queue; wait if necessary */
> @@ -1053,9 +1050,6 @@ static int recv_stream(struct kiocb *iocb, struct socket *sock,
>  		goto exit;
>  	}
>  
> -	/* will be updated in set_orig_addr() if needed */
> -	m->msg_namelen = 0;
> -
>  restart:
>  
>  	/* Look for a message in receive queue; wait if necessary */
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index d65e7f0..fc57017 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1668,7 +1668,6 @@ static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
>  {
>  	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);
> @@ -1691,8 +1690,6 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	if (flags&MSG_OOB)
>  		goto out;
>  
> -	msg->msg_namelen = 0;
> -
>  	mutex_lock(&u->readlock);
>  
>  	skb = skb_recv_datagram(sk, flags, noblock, &err);
> @@ -1818,8 +1815,6 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	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 2e9e300..40c447f 100644
> --- a/net/x25/af_x25.c
> +++ b/net/x25/af_x25.c
> @@ -1294,10 +1294,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
>  	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);
> -
>  	lock_sock(sk);
>  	x25_check_rbuf(sk);
>  	release_sock(sk);

Looks to apply all the bits we have.  The other parts are indeed MIA
(new code in later releases).  This patch therefore looks ok to me:

Acked-by: Andy Whitcroft <apw at canonical.com>

However, I note in mainline we have the commit below on top for rose
(which Lucid does have).  Do we need it too?

  commit f81152e35001e91997ec74a7b4e040e6ab0acccf
  Author: Florian Westphal <fw at strlen.de>
  Date:   Mon Dec 23 00:32:31 2013 +0100

    net: rose: restore old recvmsg behavior
    
    recvmsg handler in net/rose/af_rose.c performs size-check ->msg_namelen.

-apw




More information about the kernel-team mailing list