diff options
author | Prashant Bhole | 2018-12-03 18:09:24 +0900 |
---|---|---|
committer | David S. Miller | 2018-12-03 14:10:27 -0800 |
commit | 4e4b08e55889da97dec750759f3ade8cc92b4644 (patch) | |
tree | 9dd902827780d99b486602739bd607813df84ddb /drivers/net/tun.c | |
parent | a5d4a89245ead1f37ed135213653c5beebea4237 (diff) |
tun: remove skb access after netif_receive_skb
In tun.c skb->len was accessed while doing stats accounting after a
call to netif_receive_skb. We can not access skb after this call
because buffers may be dropped.
The fix for this bug would be to store skb->len in local variable and
then use it after netif_receive_skb(). IMO using xdp data size for
accounting bytes will be better because input for tun_xdp_one() is
xdp_buff.
Hence this patch:
- fixes a bug by removing skb access after netif_receive_skb()
- uses xdp data size for accounting bytes
[613.019057] BUG: KASAN: use-after-free in tun_sendmsg+0x77c/0xc50 [tun]
[613.021062] Read of size 4 at addr ffff8881da9ab7c0 by task vhost-1115/1155
[613.023073]
[613.024003] CPU: 0 PID: 1155 Comm: vhost-1115 Not tainted 4.20.0-rc3-vm+ #232
[613.026029] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
[613.029116] Call Trace:
[613.031145] dump_stack+0x5b/0x90
[613.032219] print_address_description+0x6c/0x23c
[613.034156] ? tun_sendmsg+0x77c/0xc50 [tun]
[613.036141] kasan_report.cold.5+0x241/0x308
[613.038125] tun_sendmsg+0x77c/0xc50 [tun]
[613.040109] ? tun_get_user+0x1960/0x1960 [tun]
[613.042094] ? __isolate_free_page+0x270/0x270
[613.045173] vhost_tx_batch.isra.14+0xeb/0x1f0 [vhost_net]
[613.047127] ? peek_head_len.part.13+0x90/0x90 [vhost_net]
[613.049096] ? get_tx_bufs+0x5a/0x2c0 [vhost_net]
[613.051106] ? vhost_enable_notify+0x2d8/0x420 [vhost]
[613.053139] handle_tx_copy+0x2d0/0x8f0 [vhost_net]
[613.053139] ? vhost_net_buf_peek+0x340/0x340 [vhost_net]
[613.053139] ? __mutex_lock+0x8d9/0xb30
[613.053139] ? finish_task_switch+0x8f/0x3f0
[613.053139] ? handle_tx+0x32/0x120 [vhost_net]
[613.053139] ? mutex_trylock+0x110/0x110
[613.053139] ? finish_task_switch+0xcf/0x3f0
[613.053139] ? finish_task_switch+0x240/0x3f0
[613.053139] ? __switch_to_asm+0x34/0x70
[613.053139] ? __switch_to_asm+0x40/0x70
[613.053139] ? __schedule+0x506/0xf10
[613.053139] handle_tx+0xc7/0x120 [vhost_net]
[613.053139] vhost_worker+0x166/0x200 [vhost]
[613.053139] ? vhost_dev_init+0x580/0x580 [vhost]
[613.053139] ? __kthread_parkme+0x77/0x90
[613.053139] ? vhost_dev_init+0x580/0x580 [vhost]
[613.053139] kthread+0x1b1/0x1d0
[613.053139] ? kthread_park+0xb0/0xb0
[613.053139] ret_from_fork+0x35/0x40
[613.088705]
[613.088705] Allocated by task 1155:
[613.088705] kasan_kmalloc+0xbf/0xe0
[613.088705] kmem_cache_alloc+0xdc/0x220
[613.088705] __build_skb+0x2a/0x160
[613.088705] build_skb+0x14/0xc0
[613.088705] tun_sendmsg+0x4f0/0xc50 [tun]
[613.088705] vhost_tx_batch.isra.14+0xeb/0x1f0 [vhost_net]
[613.088705] handle_tx_copy+0x2d0/0x8f0 [vhost_net]
[613.088705] handle_tx+0xc7/0x120 [vhost_net]
[613.088705] vhost_worker+0x166/0x200 [vhost]
[613.088705] kthread+0x1b1/0x1d0
[613.088705] ret_from_fork+0x35/0x40
[613.088705]
[613.088705] Freed by task 1155:
[613.088705] __kasan_slab_free+0x12e/0x180
[613.088705] kmem_cache_free+0xa0/0x230
[613.088705] ip6_mc_input+0x40f/0x5a0
[613.088705] ipv6_rcv+0xc9/0x1e0
[613.088705] __netif_receive_skb_one_core+0xc1/0x100
[613.088705] netif_receive_skb_internal+0xc4/0x270
[613.088705] br_pass_frame_up+0x2b9/0x2e0
[613.088705] br_handle_frame_finish+0x2fb/0x7a0
[613.088705] br_handle_frame+0x30f/0x6c0
[613.088705] __netif_receive_skb_core+0x61a/0x15b0
[613.088705] __netif_receive_skb_one_core+0x8e/0x100
[613.088705] netif_receive_skb_internal+0xc4/0x270
[613.088705] tun_sendmsg+0x738/0xc50 [tun]
[613.088705] vhost_tx_batch.isra.14+0xeb/0x1f0 [vhost_net]
[613.088705] handle_tx_copy+0x2d0/0x8f0 [vhost_net]
[613.088705] handle_tx+0xc7/0x120 [vhost_net]
[613.088705] vhost_worker+0x166/0x200 [vhost]
[613.088705] kthread+0x1b1/0x1d0
[613.088705] ret_from_fork+0x35/0x40
[613.088705]
[613.088705] The buggy address belongs to the object at ffff8881da9ab740
[613.088705] which belongs to the cache skbuff_head_cache of size 232
Fixes: 043d222f93ab ("tuntap: accept an array of XDP buffs through sendmsg()")
Reviewed-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'drivers/net/tun.c')
-rw-r--r-- | drivers/net/tun.c | 3 |
1 files changed, 2 insertions, 1 deletions
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index cf349e65a66b..005020042be9 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -2385,6 +2385,7 @@ static int tun_xdp_one(struct tun_struct *tun, struct tun_file *tfile, struct xdp_buff *xdp, int *flush) { + unsigned int datasize = xdp->data_end - xdp->data; struct tun_xdp_hdr *hdr = xdp->data_hard_start; struct virtio_net_hdr *gso = &hdr->gso; struct tun_pcpu_stats *stats; @@ -2461,7 +2462,7 @@ build: stats = get_cpu_ptr(tun->pcpu_stats); u64_stats_update_begin(&stats->syncp); stats->rx_packets++; - stats->rx_bytes += skb->len; + stats->rx_bytes += datasize; u64_stats_update_end(&stats->syncp); put_cpu_ptr(stats); |