[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 15:38:32 UTC 2014
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 <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.
>
> 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 <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.
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 <fw at strlen.de>
Acked-by: Hannes Frederic Sowa <hannes at stressinduktion.org>
Signed-off-by: David S. Miller <davem at davemloft.net>
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);
More information about the kernel-team
mailing list