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
next prev parent 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).