mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	tun: fix use after free for ptr_ring
We used to initialize ptr_ring during TUNSETIFF, this is because its size depends on the tx_queue_len of netdevice. And we try to clean it up when socket were detached from netdevice. A race were spotted when trying to do uninit during a read which will lead a use after free for pointer ring. Solving this by always initialize a zero size ptr_ring in open() and do resizing during TUNSETIFF, and then we can safely do cleanup during close(). With this, there's no need for the workaround that was introduced by commit4df0bfc799("tun: fix a memory leak for tfile->tx_array"). Reported-by: syzbot+e8b902c3c3fadf0a9dba@syzkaller.appspotmail.com Cc: Eric Dumazet <eric.dumazet@gmail.com> Cc: Cong Wang <xiyou.wangcong@gmail.com> Cc: Michael S. Tsirkin <mst@redhat.com> Fixes:1576d98605("tun: switch to use skb array for tx") Signed-off-by: Jason Wang <jasowang@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
		
							parent
							
								
									9d6b4bfb59
								
							
						
					
					
						commit
						b196d88aba
					
				
					 1 changed files with 12 additions and 15 deletions
				
			
		| 
						 | 
					@ -681,15 +681,6 @@ static void tun_queue_purge(struct tun_file *tfile)
 | 
				
			||||||
	skb_queue_purge(&tfile->sk.sk_error_queue);
 | 
						skb_queue_purge(&tfile->sk.sk_error_queue);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static void tun_cleanup_tx_ring(struct tun_file *tfile)
 | 
					 | 
				
			||||||
{
 | 
					 | 
				
			||||||
	if (tfile->tx_ring.queue) {
 | 
					 | 
				
			||||||
		ptr_ring_cleanup(&tfile->tx_ring, tun_ptr_free);
 | 
					 | 
				
			||||||
		xdp_rxq_info_unreg(&tfile->xdp_rxq);
 | 
					 | 
				
			||||||
		memset(&tfile->tx_ring, 0, sizeof(tfile->tx_ring));
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
}
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
static void __tun_detach(struct tun_file *tfile, bool clean)
 | 
					static void __tun_detach(struct tun_file *tfile, bool clean)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
	struct tun_file *ntfile;
 | 
						struct tun_file *ntfile;
 | 
				
			||||||
| 
						 | 
					@ -736,7 +727,8 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 | 
				
			||||||
			    tun->dev->reg_state == NETREG_REGISTERED)
 | 
								    tun->dev->reg_state == NETREG_REGISTERED)
 | 
				
			||||||
				unregister_netdevice(tun->dev);
 | 
									unregister_netdevice(tun->dev);
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
		tun_cleanup_tx_ring(tfile);
 | 
							if (tun)
 | 
				
			||||||
 | 
								xdp_rxq_info_unreg(&tfile->xdp_rxq);
 | 
				
			||||||
		sock_put(&tfile->sk);
 | 
							sock_put(&tfile->sk);
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					@ -783,14 +775,14 @@ static void tun_detach_all(struct net_device *dev)
 | 
				
			||||||
		tun_napi_del(tun, tfile);
 | 
							tun_napi_del(tun, tfile);
 | 
				
			||||||
		/* Drop read queue */
 | 
							/* Drop read queue */
 | 
				
			||||||
		tun_queue_purge(tfile);
 | 
							tun_queue_purge(tfile);
 | 
				
			||||||
 | 
							xdp_rxq_info_unreg(&tfile->xdp_rxq);
 | 
				
			||||||
		sock_put(&tfile->sk);
 | 
							sock_put(&tfile->sk);
 | 
				
			||||||
		tun_cleanup_tx_ring(tfile);
 | 
					 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	list_for_each_entry_safe(tfile, tmp, &tun->disabled, next) {
 | 
						list_for_each_entry_safe(tfile, tmp, &tun->disabled, next) {
 | 
				
			||||||
		tun_enable_queue(tfile);
 | 
							tun_enable_queue(tfile);
 | 
				
			||||||
		tun_queue_purge(tfile);
 | 
							tun_queue_purge(tfile);
 | 
				
			||||||
 | 
							xdp_rxq_info_unreg(&tfile->xdp_rxq);
 | 
				
			||||||
		sock_put(&tfile->sk);
 | 
							sock_put(&tfile->sk);
 | 
				
			||||||
		tun_cleanup_tx_ring(tfile);
 | 
					 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	BUG_ON(tun->numdisabled != 0);
 | 
						BUG_ON(tun->numdisabled != 0);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -834,7 +826,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (!tfile->detached &&
 | 
						if (!tfile->detached &&
 | 
				
			||||||
	    ptr_ring_init(&tfile->tx_ring, dev->tx_queue_len, GFP_KERNEL)) {
 | 
						    ptr_ring_resize(&tfile->tx_ring, dev->tx_queue_len,
 | 
				
			||||||
 | 
								    GFP_KERNEL, tun_ptr_free)) {
 | 
				
			||||||
		err = -ENOMEM;
 | 
							err = -ENOMEM;
 | 
				
			||||||
		goto out;
 | 
							goto out;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					@ -3219,6 +3212,11 @@ static int tun_chr_open(struct inode *inode, struct file * file)
 | 
				
			||||||
					    &tun_proto, 0);
 | 
										    &tun_proto, 0);
 | 
				
			||||||
	if (!tfile)
 | 
						if (!tfile)
 | 
				
			||||||
		return -ENOMEM;
 | 
							return -ENOMEM;
 | 
				
			||||||
 | 
						if (ptr_ring_init(&tfile->tx_ring, 0, GFP_KERNEL)) {
 | 
				
			||||||
 | 
							sk_free(&tfile->sk);
 | 
				
			||||||
 | 
							return -ENOMEM;
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	RCU_INIT_POINTER(tfile->tun, NULL);
 | 
						RCU_INIT_POINTER(tfile->tun, NULL);
 | 
				
			||||||
	tfile->flags = 0;
 | 
						tfile->flags = 0;
 | 
				
			||||||
	tfile->ifindex = 0;
 | 
						tfile->ifindex = 0;
 | 
				
			||||||
| 
						 | 
					@ -3239,8 +3237,6 @@ static int tun_chr_open(struct inode *inode, struct file * file)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	sock_set_flag(&tfile->sk, SOCK_ZEROCOPY);
 | 
						sock_set_flag(&tfile->sk, SOCK_ZEROCOPY);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	memset(&tfile->tx_ring, 0, sizeof(tfile->tx_ring));
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
	return 0;
 | 
						return 0;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -3249,6 +3245,7 @@ static int tun_chr_close(struct inode *inode, struct file *file)
 | 
				
			||||||
	struct tun_file *tfile = file->private_data;
 | 
						struct tun_file *tfile = file->private_data;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	tun_detach(tfile, true);
 | 
						tun_detach(tfile, true);
 | 
				
			||||||
 | 
						ptr_ring_cleanup(&tfile->tx_ring, tun_ptr_free);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return 0;
 | 
						return 0;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue