netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Amadeusz Sławiński" <amade@asmblr.net>
To: "David S . Miller" <davem@davemloft.net>, netdev@vger.kernel.org
Cc: "Amadeusz Sławiński" <amade@asmblr.net>
Subject: [RFC PATCH 1/2] net: tap: Fix incorrect memory access
Date: Sun, 29 Sep 2019 13:05:01 +0200	[thread overview]
Message-ID: <20190929110502.2284-2-amade@asmblr.net> (raw)
In-Reply-To: <20190929110502.2284-1-amade@asmblr.net>

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@asmblr.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


  reply	other threads:[~2019-09-29 12:09 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-29 11:05 [RFC PATCH 0/2] Incorrect memory access when using TAP Amadeusz Sławiński
2019-09-29 11:05 ` Amadeusz Sławiński [this message]
2019-09-29 11:05 ` [RFC PATCH 2/2] net: tun: Fix incorrect memory access Amadeusz Sławiński

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190929110502.2284-2-amade@asmblr.net \
    --to=amade@asmblr.net \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).