Skip to content

Commit ab75811

Browse files
jasowanggregkh
authored andcommitted
tun: fix use after free for ptr_ring
commit b196d88 upstream. 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 4df0bfc ("tun: fix a memory leak for tfile->tx_array"). Backport Note :- Comparison with the upstream patch: [1] A "semantic revert" of the changes made in 4df0bfc("tun: fix a memory leak for tfile->tx_array"). 4df0bfc was applied upstream, and then skb array was changed to use ptr_ring. The upstream patch then removes the changes introduced by 4df0bfc. This backport does the same; "revert" the changes made by 4df0bfc. [2] xdp_rxq_info_unreg() being called in relevant locations As xdp_rxq_info related patches are not present in 4.14, these changes are not needed in the backport. [3] An instance of ptr_ring_init needs to be replaced by skb_array_init Inside tun_attach() [4] ptr_ring_cleanup needs to be replaced by skb_array_cleanup Inside tun_chr_close() Note that the backport for 7063efd ("tuntap: fix use after free during release") needs to be applied on top of this patch. 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: 1576d98 ("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> Signed-off-by: Zubin Mithra <zsm@chromium.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 8626c40 commit ab75811

1 file changed

Lines changed: 7 additions & 14 deletions

File tree

drivers/net/tun.c

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -534,14 +534,6 @@ static void tun_queue_purge(struct tun_file *tfile)
534534
skb_queue_purge(&tfile->sk.sk_error_queue);
535535
}
536536

537-
static void tun_cleanup_tx_array(struct tun_file *tfile)
538-
{
539-
if (tfile->tx_array.ring.queue) {
540-
skb_array_cleanup(&tfile->tx_array);
541-
memset(&tfile->tx_array, 0, sizeof(tfile->tx_array));
542-
}
543-
}
544-
545537
static void __tun_detach(struct tun_file *tfile, bool clean)
546538
{
547539
struct tun_file *ntfile;
@@ -583,7 +575,6 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
583575
tun->dev->reg_state == NETREG_REGISTERED)
584576
unregister_netdevice(tun->dev);
585577
}
586-
tun_cleanup_tx_array(tfile);
587578
sock_put(&tfile->sk);
588579
}
589580
}
@@ -623,13 +614,11 @@ static void tun_detach_all(struct net_device *dev)
623614
/* Drop read queue */
624615
tun_queue_purge(tfile);
625616
sock_put(&tfile->sk);
626-
tun_cleanup_tx_array(tfile);
627617
}
628618
list_for_each_entry_safe(tfile, tmp, &tun->disabled, next) {
629619
tun_enable_queue(tfile);
630620
tun_queue_purge(tfile);
631621
sock_put(&tfile->sk);
632-
tun_cleanup_tx_array(tfile);
633622
}
634623
BUG_ON(tun->numdisabled != 0);
635624

@@ -675,7 +664,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte
675664
}
676665

677666
if (!tfile->detached &&
678-
skb_array_init(&tfile->tx_array, dev->tx_queue_len, GFP_KERNEL)) {
667+
skb_array_resize(&tfile->tx_array, dev->tx_queue_len, GFP_KERNEL)) {
679668
err = -ENOMEM;
680669
goto out;
681670
}
@@ -2624,6 +2613,11 @@ static int tun_chr_open(struct inode *inode, struct file * file)
26242613
&tun_proto, 0);
26252614
if (!tfile)
26262615
return -ENOMEM;
2616+
if (skb_array_init(&tfile->tx_array, 0, GFP_KERNEL)) {
2617+
sk_free(&tfile->sk);
2618+
return -ENOMEM;
2619+
}
2620+
26272621
RCU_INIT_POINTER(tfile->tun, NULL);
26282622
tfile->flags = 0;
26292623
tfile->ifindex = 0;
@@ -2644,8 +2638,6 @@ static int tun_chr_open(struct inode *inode, struct file * file)
26442638

26452639
sock_set_flag(&tfile->sk, SOCK_ZEROCOPY);
26462640

2647-
memset(&tfile->tx_array, 0, sizeof(tfile->tx_array));
2648-
26492641
return 0;
26502642
}
26512643

@@ -2654,6 +2646,7 @@ static int tun_chr_close(struct inode *inode, struct file *file)
26542646
struct tun_file *tfile = file->private_data;
26552647

26562648
tun_detach(tfile, true);
2649+
skb_array_cleanup(&tfile->tx_array);
26572650

26582651
return 0;
26592652
}

0 commit comments

Comments
 (0)