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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sun, 29 Sep 2019 13:05:02 +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 2/2] net: tun: Fix incorrect memory access

This is equivalent commit to tap one, where we fix incorrect memory
access caused by sock_init_data being passed improper socket.

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 tun_file.

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/tun.c | 92 +++++++++++++++++++++++++----------------------
 1 file changed, 50 insertions(+), 42 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index aab0be40d443..60344794579c 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -159,7 +159,7 @@ struct tun_pcpu_stats {
  */
 struct tun_file {
 	struct sock sk;
-	struct socket socket;
+	struct socket *socket;
 	struct tun_struct __rcu *tun;
 	struct fasync_struct *fasync;
 	/* only used for fasnyc */
@@ -753,14 +753,14 @@ static void tun_detach_all(struct net_device *dev)
 		tfile = rtnl_dereference(tun->tfiles[i]);
 		BUG_ON(!tfile);
 		tun_napi_disable(tfile);
-		tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
-		tfile->socket.sk->sk_data_ready(tfile->socket.sk);
+		tfile->socket->sk->sk_shutdown = RCV_SHUTDOWN;
+		tfile->socket->sk->sk_data_ready(tfile->socket->sk);
 		RCU_INIT_POINTER(tfile->tun, NULL);
 		--tun->numqueues;
 	}
 	list_for_each_entry(tfile, &tun->disabled, next) {
-		tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
-		tfile->socket.sk->sk_data_ready(tfile->socket.sk);
+		tfile->socket->sk->sk_shutdown = RCV_SHUTDOWN;
+		tfile->socket->sk->sk_data_ready(tfile->socket->sk);
 		RCU_INIT_POINTER(tfile->tun, NULL);
 	}
 	BUG_ON(tun->numqueues != 0);
@@ -794,7 +794,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
 	struct net_device *dev = tun->dev;
 	int err;
 
-	err = security_tun_dev_attach(tfile->socket.sk, tun->security);
+	err = security_tun_dev_attach(tfile->socket->sk, tun->security);
 	if (err < 0)
 		goto out;
 
@@ -815,9 +815,9 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
 
 	/* Re-attach the filter to persist device */
 	if (!skip_filter && (tun->filter_attached == true)) {
-		lock_sock(tfile->socket.sk);
-		err = sk_attach_filter(&tun->fprog, tfile->socket.sk);
-		release_sock(tfile->socket.sk);
+		lock_sock(tfile->socket->sk);
+		err = sk_attach_filter(&tun->fprog, tfile->socket->sk);
+		release_sock(tfile->socket->sk);
 		if (!err)
 			goto out;
 	}
@@ -830,7 +830,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
 	}
 
 	tfile->queue_index = tun->numqueues;
-	tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN;
+	tfile->socket->sk->sk_shutdown &= ~RCV_SHUTDOWN;
 
 	if (tfile->detached) {
 		/* Re-attach detached tfile, updating XDP queue_index */
@@ -1086,8 +1086,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (!check_filter(&tun->txflt, skb))
 		goto drop;
 
-	if (tfile->socket.sk->sk_filter &&
-	    sk_filter(tfile->socket.sk, skb))
+	if (tfile->socket->sk->sk_filter &&
+	    sk_filter(tfile->socket->sk, skb))
 		goto drop;
 
 	len = run_ebpf_filter(tun, skb, len);
@@ -1112,7 +1112,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Notify and wake up reader process */
 	if (tfile->flags & TUN_FASYNC)
 		kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
-	tfile->socket.sk->sk_data_ready(tfile->socket.sk);
+	tfile->socket->sk->sk_data_ready(tfile->socket->sk);
 
 	rcu_read_unlock();
 	return NETDEV_TX_OK;
@@ -1275,7 +1275,7 @@ static void __tun_xdp_flush_tfile(struct tun_file *tfile)
 	/* Notify and wake up reader process */
 	if (tfile->flags & TUN_FASYNC)
 		kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
-	tfile->socket.sk->sk_data_ready(tfile->socket.sk);
+	tfile->socket->sk->sk_data_ready(tfile->socket->sk);
 }
 
 static int tun_xdp_xmit(struct net_device *dev, int n,
@@ -1415,7 +1415,7 @@ static void tun_net_init(struct net_device *dev)
 
 static bool tun_sock_writeable(struct tun_struct *tun, struct tun_file *tfile)
 {
-	struct sock *sk = tfile->socket.sk;
+	struct sock *sk = tfile->socket->sk;
 
 	return (tun->dev->flags & IFF_UP) && sock_writeable(sk);
 }
@@ -1433,7 +1433,7 @@ static __poll_t tun_chr_poll(struct file *file, poll_table *wait)
 	if (!tun)
 		return EPOLLERR;
 
-	sk = tfile->socket.sk;
+	sk = tfile->socket->sk;
 
 	tun_debug(KERN_INFO, tun, "tun_chr_poll\n");
 
@@ -1518,7 +1518,7 @@ static struct sk_buff *tun_alloc_skb(struct tun_file *tfile,
 				     size_t prepad, size_t len,
 				     size_t linear, int noblock)
 {
-	struct sock *sk = tfile->socket.sk;
+	struct sock *sk = tfile->socket->sk;
 	struct sk_buff *skb;
 	int err;
 
@@ -1585,7 +1585,7 @@ static bool tun_can_build_skb(struct tun_struct *tun, struct tun_file *tfile,
 	if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
 		return false;
 
-	if (tfile->socket.sk->sk_sndbuf != INT_MAX)
+	if (tfile->socket->sk->sk_sndbuf != INT_MAX)
 		return false;
 
 	if (!noblock)
@@ -1612,7 +1612,7 @@ static struct sk_buff *__tun_build_skb(struct tun_file *tfile,
 
 	skb_reserve(skb, pad);
 	skb_put(skb, len);
-	skb_set_owner_w(skb, tfile->socket.sk);
+	skb_set_owner_w(skb, tfile->socket->sk);
 
 	get_page(alloc_frag->page);
 	alloc_frag->offset += buflen;
@@ -2169,7 +2169,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
 		goto out;
 	}
 
-	add_wait_queue(&tfile->socket.wq.wait, &wait);
+	add_wait_queue(&tfile->socket->wq.wait, &wait);
 
 	while (1) {
 		set_current_state(TASK_INTERRUPTIBLE);
@@ -2180,7 +2180,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
 			error = -ERESTARTSYS;
 			break;
 		}
-		if (tfile->socket.sk->sk_shutdown & RCV_SHUTDOWN) {
+		if (tfile->socket->sk->sk_shutdown & RCV_SHUTDOWN) {
 			error = -EFAULT;
 			break;
 		}
@@ -2189,7 +2189,7 @@ static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err)
 	}
 
 	__set_current_state(TASK_RUNNING);
-	remove_wait_queue(&tfile->socket.wq.wait, &wait);
+	remove_wait_queue(&tfile->socket->wq.wait, &wait);
 
 out:
 	*err = error;
@@ -2519,7 +2519,7 @@ static int tun_xdp_one(struct tun_struct *tun,
 static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
 {
 	int ret, i;
-	struct tun_file *tfile = container_of(sock, struct tun_file, socket);
+	struct tun_file *tfile = container_of(sock->sk, struct tun_file, sk);
 	struct tun_struct *tun = tun_get(tfile);
 	struct tun_msg_ctl *ctl = m->msg_control;
 	struct xdp_buff *xdp;
@@ -2565,7 +2565,7 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len)
 static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len,
 		       int flags)
 {
-	struct tun_file *tfile = container_of(sock, struct tun_file, socket);
+	struct tun_file *tfile = container_of(sock->sk, struct tun_file, sk);
 	struct tun_struct *tun = tun_get(tfile);
 	void *ptr = m->msg_control;
 	int ret;
@@ -2616,7 +2616,7 @@ static int tun_ptr_peek_len(void *ptr)
 
 static int tun_peek_len(struct socket *sock)
 {
-	struct tun_file *tfile = container_of(sock, struct tun_file, socket);
+	struct tun_file *tfile = container_of(sock->sk, struct tun_file, sk);
 	struct tun_struct *tun;
 	int ret = 0;
 
@@ -2799,7 +2799,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 
 		tun->align = NET_SKB_PAD;
 		tun->filter_attached = false;
-		tun->sndbuf = tfile->socket.sk->sk_sndbuf;
+		tun->sndbuf = tfile->socket->sk->sk_sndbuf;
 		tun->rx_batched = 0;
 		RCU_INIT_POINTER(tun->steering_prog, NULL);
 
@@ -2927,9 +2927,9 @@ static void tun_detach_filter(struct tun_struct *tun, int n)
 
 	for (i = 0; i < n; i++) {
 		tfile = rtnl_dereference(tun->tfiles[i]);
-		lock_sock(tfile->socket.sk);
-		sk_detach_filter(tfile->socket.sk);
-		release_sock(tfile->socket.sk);
+		lock_sock(tfile->socket->sk);
+		sk_detach_filter(tfile->socket->sk);
+		release_sock(tfile->socket->sk);
 	}
 
 	tun->filter_attached = false;
@@ -2942,9 +2942,9 @@ static int tun_attach_filter(struct tun_struct *tun)
 
 	for (i = 0; i < tun->numqueues; i++) {
 		tfile = rtnl_dereference(tun->tfiles[i]);
-		lock_sock(tfile->socket.sk);
-		ret = sk_attach_filter(&tun->fprog, tfile->socket.sk);
-		release_sock(tfile->socket.sk);
+		lock_sock(tfile->socket->sk);
+		ret = sk_attach_filter(&tun->fprog, tfile->socket->sk);
+		release_sock(tfile->socket->sk);
 		if (ret) {
 			tun_detach_filter(tun, i);
 			return ret;
@@ -2962,7 +2962,7 @@ static void tun_set_sndbuf(struct tun_struct *tun)
 
 	for (i = 0; i < tun->numqueues; i++) {
 		tfile = rtnl_dereference(tun->tfiles[i]);
-		tfile->socket.sk->sk_sndbuf = tun->sndbuf;
+		tfile->socket->sk->sk_sndbuf = tun->sndbuf;
 	}
 }
 
@@ -3109,7 +3109,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 
 		if (tfile->detached)
 			ifr.ifr_flags |= IFF_DETACH_QUEUE;
-		if (!tfile->socket.sk->sk_filter)
+		if (!tfile->socket->sk->sk_filter)
 			ifr.ifr_flags |= IFF_NOFILTER;
 
 		if (copy_to_user(argp, &ifr, ifreq_len))
@@ -3217,7 +3217,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		break;
 
 	case TUNGETSNDBUF:
-		sndbuf = tfile->socket.sk->sk_sndbuf;
+		sndbuf = tfile->socket->sk->sk_sndbuf;
 		if (copy_to_user(argp, &sndbuf, sizeof(sndbuf)))
 			ret = -EFAULT;
 		break;
@@ -3405,6 +3405,7 @@ static int tun_chr_fasync(int fd, struct file *file, int on)
 static int tun_chr_open(struct inode *inode, struct file * file)
 {
 	struct net *net = current->nsproxy->net_ns;
+	struct socket *sock;
 	struct tun_file *tfile;
 
 	DBG1(KERN_INFO, "tunX: tun_chr_open\n");
@@ -3413,7 +3414,16 @@ static int tun_chr_open(struct inode *inode, struct file * file)
 					    &tun_proto, 0);
 	if (!tfile)
 		return -ENOMEM;
+
+	sock = sock_alloc();
+	if (!sock) {
+		sk_free(&tfile->sk);
+		return -ENOMEM;
+	}
+	tfile->socket = sock;
+
 	if (ptr_ring_init(&tfile->tx_ring, 0, GFP_KERNEL)) {
+		sock_release(tfile->socket);
 		sk_free(&tfile->sk);
 		return -ENOMEM;
 	}
@@ -3423,12 +3433,10 @@ static int tun_chr_open(struct inode *inode, struct file * file)
 	tfile->flags = 0;
 	tfile->ifindex = 0;
 
-	init_waitqueue_head(&tfile->socket.wq.wait);
-
-	tfile->socket.file = file;
-	tfile->socket.ops = &tun_socket_ops;
+	sock->file = file;
+	sock->ops = &tun_socket_ops;
 
-	sock_init_data(&tfile->socket, &tfile->sk);
+	sock_init_data(sock, &tfile->sk);
 
 	tfile->sk.sk_write_space = tun_sock_write_space;
 	tfile->sk.sk_sndbuf = INT_MAX;
@@ -3646,7 +3654,7 @@ static int tun_device_event(struct notifier_block *unused,
 			struct tun_file *tfile;
 
 			tfile = rtnl_dereference(tun->tfiles[i]);
-			tfile->socket.sk->sk_write_space(tfile->socket.sk);
+			tfile->socket->sk->sk_write_space(tfile->socket->sk);
 		}
 		break;
 	default:
@@ -3713,7 +3721,7 @@ struct socket *tun_get_socket(struct file *file)
 	tfile = file->private_data;
 	if (!tfile)
 		return ERR_PTR(-EBADFD);
-	return &tfile->socket;
+	return tfile->socket;
 }
 EXPORT_SYMBOL_GPL(tun_get_socket);
 
-- 
2.23.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ