From patchwork Mon Jan 20 15:38:32 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Whitcroft X-Patchwork-Id: 312623 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3B39B2C00A3 for ; Tue, 21 Jan 2014 02:38:51 +1100 (EST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1W5Gvv-0003H2-Ms; Mon, 20 Jan 2014 15:38:39 +0000 Received: from mail-wi0-f178.google.com ([209.85.212.178]) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1W5Gvr-0003Gv-0g for kernel-team@lists.ubuntu.com; Mon, 20 Jan 2014 15:38:35 +0000 Received: by mail-wi0-f178.google.com with SMTP id cc10so3298487wib.11 for ; Mon, 20 Jan 2014 07:38:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=6NRaJSvftbAuyyE+DTQLBUtBYRDxRvHSj6etBXXpXVs=; b=UrWNy+yNfYEHcJs56nDTrRQGTFfAv6WWDVRftkodcT0L1NRYM/Z9E+oaxcyHZdyFOp LDhZn2VcwlGL0H1f+buH3xhCSYB9r1Dlubo7tHzgFsP5GOEffdSWpx4dn4+nvbjwXLRz YJToUnplFZfqY2ALTUtfheeQ+05h3foHbI9Nj5YysfO2fDPYqYtyiT6F+zhNn6ruQX8+ p/9aH6g+fJp8r8YMFtRhQxBIGrUK6eNUyyeKoUSyMeYRvCnJnMd6HM/W93ur6OS+9JJK pF0sb93xBtuGy2J3YZn3mvP0izcSlwRM3iW6yL4SlWy844GGVPkLPdiCFv8MW4/kFFKr Tdcg== X-Gm-Message-State: ALoCoQkctu8orxZWYzdr/AIVEPBqpEwHFWgsaqNvd4CWxZM4pZ2/2B3xSSKC2qebZGpQck08gyEi X-Received: by 10.194.1.238 with SMTP id 14mr14458968wjp.20.1390232314636; Mon, 20 Jan 2014 07:38:34 -0800 (PST) Received: from localhost ([2001:470:6973:2:221:70ff:fe81:b177]) by mx.google.com with ESMTPSA id z1sm1973631wjq.19.2014.01.20.07.38.33 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 20 Jan 2014 07:38:33 -0800 (PST) Date: Mon, 20 Jan 2014 15:38:32 +0000 From: Andy Whitcroft To: Luis Henriques Subject: Re: [Acked w/ cmt] [Lucid][CVE-2013-7266] net: rework recvmsg handler msg_name and msg_namelen logic Message-ID: <20140120153832.GA6786@dm> References: <1389969069-2148-1-git-send-email-luis.henriques@canonical.com> <1389969069-2148-2-git-send-email-luis.henriques@canonical.com> <20140120123114.GN3368@bark> <20140120132250.GB4656@hercules> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20140120132250.GB4656@hercules> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: kernel-team@lists.ubuntu.com X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: kernel-team-bounces@lists.ubuntu.com On Mon, Jan 20, 2014 at 01:22:50PM +0000, Luis Henriques wrote: > On Mon, Jan 20, 2014 at 12:31:14PM +0000, Andy Whitcroft wrote: > > On Fri, Jan 17, 2014 at 02:31:09PM +0000, Luis Henriques wrote: > > > From: Hannes Frederic Sowa > > > > > > 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 > > > Suggested-by: Eric Dumazet > > > Signed-off-by: Hannes Frederic Sowa > > > Signed-off-by: David S. Miller > > > (back ported from commit f3d3342602f8bcbf37d7c46641cb9bca7618eb1c) > > > Signed-off-by: Luis Henriques > > > --- > > > 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 > > > > 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 > > 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. > > Thanks Andy, it seems to make sense for this commit to be applied as well. > I missed this fix, although I've applied it to the stable kernels I'm > maintaining... > > So, once we have the required review ACKs, please apply it on top of the > CVE fix backport (its a clean cherry-pick for Lucid). Alternatively, I > can resubmit the CVE fix to include the 2 patches. Sounds fine to apply once this is acked, it is minute compared. Full patch is below. -apw commit f81152e35001e91997ec74a7b4e040e6ab0acccf Author: Florian Westphal 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. After commit f3d3342602f8bcbf37d7c46641cb9bca7618eb1c (net: rework recvmsg handler msg_name and msg_namelen logic), we now always take the else branch due to namelen being initialized to 0. Digging in netdev-vger-cvs git repo shows that msg_namelen was initialized with a fixed-size since at least 1995, so the else branch was never taken. Compile tested only. Signed-off-by: Florian Westphal Acked-by: Hannes Frederic Sowa Signed-off-by: David S. Miller diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c index 33af772..62ced65 100644 --- a/net/rose/af_rose.c +++ b/net/rose/af_rose.c @@ -1253,6 +1253,7 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket *sock, if (msg->msg_name) { struct sockaddr_rose *srose; + struct full_sockaddr_rose *full_srose = msg->msg_name; memset(msg->msg_name, 0, sizeof(struct full_sockaddr_rose)); srose = msg->msg_name; @@ -1260,18 +1261,9 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket *sock, srose->srose_addr = rose->dest_addr; srose->srose_call = rose->dest_call; srose->srose_ndigis = rose->dest_ndigis; - if (msg->msg_namelen >= sizeof(struct full_sockaddr_rose)) { - struct full_sockaddr_rose *full_srose = (struct full_sockaddr_rose *)msg->msg_name; - for (n = 0 ; n < rose->dest_ndigis ; n++) - full_srose->srose_digis[n] = rose->dest_digis[n]; - msg->msg_namelen = sizeof(struct full_sockaddr_rose); - } else { - if (rose->dest_ndigis >= 1) { - srose->srose_ndigis = 1; - srose->srose_digi = rose->dest_digis[0]; - } - msg->msg_namelen = sizeof(struct sockaddr_rose); - } + for (n = 0 ; n < rose->dest_ndigis ; n++) + full_srose->srose_digis[n] = rose->dest_digis[n]; + msg->msg_namelen = sizeof(struct full_sockaddr_rose); } skb_free_datagram(sk, skb);