On 2018年05月11日 02:08, Cong Wang wrote:
On Tue, May 8, 2018 at 11:59 PM, Jason Wang <[email protected]> wrote: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 commit 4df0bfc79904 ("tun: fix a memory leak for tfile->tx_array").Ah, I didn't know ptr_ring_init(0) could work... Nice patch! Except one thing below.diff --git a/drivers/net/tun.c b/drivers/net/tun.c index ef33950..298cb96 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -681,15 +681,6 @@ static void tun_queue_purge(struct tun_file *tfile) 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)); - } -}I don't think you can totally remove ptr_ring_cleanup(), it should be called unconditionally with your ptr_ring_init(0) trick, right?
Right, my bad. Actually I do intend to cleanup it at close() like what commit log said.
Will send v2. Thanks
