netdev - [RFC PATCH 1/2] net: tap: Fix incorrect memory access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190929110502.2284-2-amade@asmblr.net>
Date:   Sun, 29 Sep 2019 13:05:01 +0200
From:   Amadeusz Sławiński <amade@...blr.net>
To:     "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Cc:     Amadeusz Sławiński <amade@...blr.net>
Subject: [RFC PATCH 1/2] net: tap: Fix incorrect memory access

KASAN reported incorrect memory access when sock_init_data() was called.
This happens due to the fact that if sock_init_data() is called with
sock argument being not NULL, it goes into path using SOCK_INODE macro.
SOCK_INODE itself is just a wrapper around
container_of(socket, struct socket_alloc, socket).

As can be seen from that flow sock_init_data, should be called with
sock, being part of struct socket_alloc, instead of being part of
struct tap_queue.

Refactor code to follow flow similar in other places where sock is
allocated correctly.

Signed-off-by: Amadeusz Sławiński <amade@...blr.net>
---
 drivers/net/tap.c      | 34 +++++++++++++++++++++-------------
 include/linux/if_tap.h |  2 +-
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index dd614c2cd994..d1f59bad599f 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -501,6 +501,7 @@ static void tap_sock_destruct(struct sock *sk)
 static int tap_open(struct inode *inode, struct file *file)
 {
 	struct net *net = current->nsproxy->net_ns;
+	struct socket *sock;
 	struct tap_dev *tap;
 	struct tap_queue *q;
 	int err = -ENODEV;
@@ -515,17 +516,25 @@ static int tap_open(struct inode *inode, struct file *file)
 					     &tap_proto, 0);
 	if (!q)
 		goto err;
+
+	sock = sock_alloc();
+	if (!sock) {
+		sk_free(&q->sk);
+		goto err;
+	}
+	q->sock = sock;
+
 	if (ptr_ring_init(&q->ring, tap->dev->tx_queue_len, GFP_KERNEL)) {
+		sock_release(q->sock);
 		sk_free(&q->sk);
 		goto err;
 	}
 
-	init_waitqueue_head(&q->sock.wq.wait);
-	q->sock.type = SOCK_RAW;
-	q->sock.state = SS_CONNECTED;
-	q->sock.file = file;
-	q->sock.ops = &tap_socket_ops;
-	sock_init_data(&q->sock, &q->sk);
+	sock->type = SOCK_RAW;
+	sock->state = SS_CONNECTED;
+	sock->file = file;
+	sock->ops = &tap_socket_ops;
+	sock_init_data(sock, &q->sk);
 	q->sk.sk_write_space = tap_sock_write_space;
 	q->sk.sk_destruct = tap_sock_destruct;
 	q->flags = IFF_VNET_HDR | IFF_NO_PI | IFF_TAP;
@@ -578,13 +587,13 @@ static __poll_t tap_poll(struct file *file, poll_table *wait)
 		goto out;
 
 	mask = 0;
-	poll_wait(file, &q->sock.wq.wait, wait);
+	poll_wait(file, &q->sock->wq.wait, wait);
 
 	if (!ptr_ring_empty(&q->ring))
 		mask |= EPOLLIN | EPOLLRDNORM;
 
 	if (sock_writeable(&q->sk) ||
-	    (!test_and_set_bit(SOCKWQ_ASYNC_NOSPACE, &q->sock.flags) &&
+	    (!test_and_set_bit(SOCKWQ_ASYNC_NOSPACE, &q->sock->flags) &&
 	     sock_writeable(&q->sk)))
 		mask |= EPOLLOUT | EPOLLWRNORM;
 
@@ -1210,7 +1219,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp)
 static int tap_sendmsg(struct socket *sock, struct msghdr *m,
 		       size_t total_len)
 {
-	struct tap_queue *q = container_of(sock, struct tap_queue, sock);
+	struct tap_queue *q = container_of(sock->sk, struct tap_queue, sk);
 	struct tun_msg_ctl *ctl = m->msg_control;
 	struct xdp_buff *xdp;
 	int i;
@@ -1230,7 +1239,7 @@ static int tap_sendmsg(struct socket *sock, struct msghdr *m,
 static int tap_recvmsg(struct socket *sock, struct msghdr *m,
 		       size_t total_len, int flags)
 {
-	struct tap_queue *q = container_of(sock, struct tap_queue, sock);
+	struct tap_queue *q = container_of(sock->sk, struct tap_queue, sk);
 	struct sk_buff *skb = m->msg_control;
 	int ret;
 	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
@@ -1247,8 +1256,7 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
 
 static int tap_peek_len(struct socket *sock)
 {
-	struct tap_queue *q = container_of(sock, struct tap_queue,
-					       sock);
+	struct tap_queue *q = container_of(sock->sk, struct tap_queue, sk);
 	return PTR_RING_PEEK_CALL(&q->ring, __skb_array_len_with_tag);
 }
 
@@ -1271,7 +1279,7 @@ struct socket *tap_get_socket(struct file *file)
 	q = file->private_data;
 	if (!q)
 		return ERR_PTR(-EBADFD);
-	return &q->sock;
+	return q->sock;
 }
 EXPORT_SYMBOL_GPL(tap_get_socket);
 
diff --git a/include/linux/if_tap.h b/include/linux/if_tap.h
index 915a187cfabd..60d00609d6ed 100644
--- a/include/linux/if_tap.h
+++ b/include/linux/if_tap.h
@@ -61,7 +61,7 @@ struct tap_dev {
 
 struct tap_queue {
 	struct sock sk;
-	struct socket sock;
+	struct socket *sock;
 	int vnet_hdr_sz;
 	struct tap_dev __rcu *tap;
 	struct file *file;
-- 
2.23.0