* [PATCH net 1/3] vhost_net: initialize rx_ring in vhost_net_open()
2018-03-09 6:50 [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage Jason Wang
@ 2018-03-09 6:50 ` Jason Wang
2018-03-09 16:55 ` Michael S. Tsirkin
2018-03-09 6:50 ` [PATCH net 2/3] vhost_net: keep private_data and rx_ring synced Jason Wang
` (3 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Jason Wang @ 2018-03-09 6:50 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel; +Cc: glider, dvyukov
From: Alexander Potapenko <glider@google.com>
KMSAN reported a use of uninit memory in vhost_net_buf_unproduce()
while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring:
==================================================================
BUG: KMSAN: use of uninitialized memory in vhost_net_buf_unproduce+0x7bb/0x9a0 drivers/vho
et.c:170
CPU: 0 PID: 3021 Comm: syz-fuzzer Not tainted 4.16.0-rc4+ #3853
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x185/0x1d0 lib/dump_stack.c:53
kmsan_report+0x142/0x1f0 mm/kmsan/kmsan.c:1093
__msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676
vhost_net_buf_unproduce+0x7bb/0x9a0 drivers/vhost/net.c:170
vhost_net_stop_vq drivers/vhost/net.c:974 [inline]
vhost_net_stop+0x146/0x380 drivers/vhost/net.c:982
vhost_net_release+0xb1/0x4f0 drivers/vhost/net.c:1015
__fput+0x49f/0xa00 fs/file_table.c:209
____fput+0x37/0x40 fs/file_table.c:243
task_work_run+0x243/0x2c0 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:191 [inline]
exit_to_usermode_loop arch/x86/entry/common.c:166 [inline]
prepare_exit_to_usermode+0x349/0x3b0 arch/x86/entry/common.c:196
syscall_return_slowpath+0xf3/0x6d0 arch/x86/entry/common.c:265
do_syscall_64+0x34d/0x450 arch/x86/entry/common.c:292
...
origin:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:303 [inline]
kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:213
kmsan_kmalloc_large+0x6f/0xd0 mm/kmsan/kmsan.c:392
kmalloc_large_node_hook mm/slub.c:1366 [inline]
kmalloc_large_node mm/slub.c:3808 [inline]
__kmalloc_node+0x100e/0x1290 mm/slub.c:3818
kmalloc_node include/linux/slab.h:554 [inline]
kvmalloc_node+0x1a5/0x2e0 mm/util.c:419
kvmalloc include/linux/mm.h:541 [inline]
vhost_net_open+0x64/0x5f0 drivers/vhost/net.c:921
misc_open+0x7b5/0x8b0 drivers/char/misc.c:154
chrdev_open+0xc28/0xd90 fs/char_dev.c:417
do_dentry_open+0xccb/0x1430 fs/open.c:752
vfs_open+0x272/0x2e0 fs/open.c:866
do_last fs/namei.c:3378 [inline]
path_openat+0x49ad/0x6580 fs/namei.c:3519
do_filp_open+0x267/0x640 fs/namei.c:3553
do_sys_open+0x6ad/0x9c0 fs/open.c:1059
SYSC_openat+0xc7/0xe0 fs/open.c:1086
SyS_openat+0x63/0x90 fs/open.c:1080
do_syscall_64+0x2f1/0x450 arch/x86/entry/common.c:287
==================================================================
Fixes: c67df11f6e480 ("vhost_net: try batch dequing from skb array")
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 610cba2..60f1080 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -948,6 +948,7 @@ static int vhost_net_open(struct inode *inode, struct file *f)
n->vqs[i].done_idx = 0;
n->vqs[i].vhost_hlen = 0;
n->vqs[i].sock_hlen = 0;
+ n->vqs[i].rx_ring = NULL;
vhost_net_buf_init(&n->vqs[i].rxq);
}
vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX);
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net 1/3] vhost_net: initialize rx_ring in vhost_net_open()
2018-03-09 6:50 ` [PATCH net 1/3] vhost_net: initialize rx_ring in vhost_net_open() Jason Wang
@ 2018-03-09 16:55 ` Michael S. Tsirkin
0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2018-03-09 16:55 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, virtualization, netdev, linux-kernel, glider, dvyukov
On Fri, Mar 09, 2018 at 02:50:32PM +0800, Jason Wang wrote:
> From: Alexander Potapenko <glider@google.com>
>
> KMSAN reported a use of uninit memory in vhost_net_buf_unproduce()
> while trying to access n->vqs[VHOST_NET_VQ_TX].rx_ring:
>
> ==================================================================
> BUG: KMSAN: use of uninitialized memory in vhost_net_buf_unproduce+0x7bb/0x9a0 drivers/vho
> et.c:170
> CPU: 0 PID: 3021 Comm: syz-fuzzer Not tainted 4.16.0-rc4+ #3853
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> Call Trace:
> __dump_stack lib/dump_stack.c:17 [inline]
> dump_stack+0x185/0x1d0 lib/dump_stack.c:53
> kmsan_report+0x142/0x1f0 mm/kmsan/kmsan.c:1093
> __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:676
> vhost_net_buf_unproduce+0x7bb/0x9a0 drivers/vhost/net.c:170
> vhost_net_stop_vq drivers/vhost/net.c:974 [inline]
> vhost_net_stop+0x146/0x380 drivers/vhost/net.c:982
> vhost_net_release+0xb1/0x4f0 drivers/vhost/net.c:1015
> __fput+0x49f/0xa00 fs/file_table.c:209
> ____fput+0x37/0x40 fs/file_table.c:243
> task_work_run+0x243/0x2c0 kernel/task_work.c:113
> tracehook_notify_resume include/linux/tracehook.h:191 [inline]
> exit_to_usermode_loop arch/x86/entry/common.c:166 [inline]
> prepare_exit_to_usermode+0x349/0x3b0 arch/x86/entry/common.c:196
> syscall_return_slowpath+0xf3/0x6d0 arch/x86/entry/common.c:265
> do_syscall_64+0x34d/0x450 arch/x86/entry/common.c:292
> ...
> origin:
> kmsan_save_stack_with_flags mm/kmsan/kmsan.c:303 [inline]
> kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:213
> kmsan_kmalloc_large+0x6f/0xd0 mm/kmsan/kmsan.c:392
> kmalloc_large_node_hook mm/slub.c:1366 [inline]
> kmalloc_large_node mm/slub.c:3808 [inline]
> __kmalloc_node+0x100e/0x1290 mm/slub.c:3818
> kmalloc_node include/linux/slab.h:554 [inline]
> kvmalloc_node+0x1a5/0x2e0 mm/util.c:419
> kvmalloc include/linux/mm.h:541 [inline]
> vhost_net_open+0x64/0x5f0 drivers/vhost/net.c:921
> misc_open+0x7b5/0x8b0 drivers/char/misc.c:154
> chrdev_open+0xc28/0xd90 fs/char_dev.c:417
> do_dentry_open+0xccb/0x1430 fs/open.c:752
> vfs_open+0x272/0x2e0 fs/open.c:866
> do_last fs/namei.c:3378 [inline]
> path_openat+0x49ad/0x6580 fs/namei.c:3519
> do_filp_open+0x267/0x640 fs/namei.c:3553
> do_sys_open+0x6ad/0x9c0 fs/open.c:1059
> SYSC_openat+0xc7/0xe0 fs/open.c:1086
> SyS_openat+0x63/0x90 fs/open.c:1080
> do_syscall_64+0x2f1/0x450 arch/x86/entry/common.c:287
> ==================================================================
>
> Fixes: c67df11f6e480 ("vhost_net: try batch dequing from skb array")
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 610cba2..60f1080 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -948,6 +948,7 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> n->vqs[i].done_idx = 0;
> n->vqs[i].vhost_hlen = 0;
> n->vqs[i].sock_hlen = 0;
> + n->vqs[i].rx_ring = NULL;
> vhost_net_buf_init(&n->vqs[i].rxq);
> }
> vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX);
> --
> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 2/3] vhost_net: keep private_data and rx_ring synced
2018-03-09 6:50 [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage Jason Wang
2018-03-09 6:50 ` [PATCH net 1/3] vhost_net: initialize rx_ring in vhost_net_open() Jason Wang
@ 2018-03-09 6:50 ` Jason Wang
2018-03-09 16:56 ` Michael S. Tsirkin
2018-03-09 6:50 ` [PATCH net 3/3] vhost_net: examine pointer types during un-producing Jason Wang
` (2 subsequent siblings)
4 siblings, 1 reply; 8+ messages in thread
From: Jason Wang @ 2018-03-09 6:50 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel; +Cc: glider, dvyukov
We get pointer ring from the exported sock, this means we should keep
rx_ring and vq->private synced during both vq stop and backend set,
otherwise we may see stale rx_ring.
Fixes: c67df11f6e480 ("vhost_net: try batch dequing from skb array")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 60f1080..efb9306 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -973,6 +973,7 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
vhost_net_disable_vq(n, vq);
vq->private_data = NULL;
vhost_net_buf_unproduce(nvq);
+ nvq->rx_ring = NULL;
mutex_unlock(&vq->mutex);
return sock;
}
@@ -1162,14 +1163,14 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
vhost_net_disable_vq(n, vq);
vq->private_data = sock;
vhost_net_buf_unproduce(nvq);
- if (index == VHOST_NET_VQ_RX)
- nvq->rx_ring = get_tap_ptr_ring(fd);
r = vhost_vq_init_access(vq);
if (r)
goto err_used;
r = vhost_net_enable_vq(n, vq);
if (r)
goto err_used;
+ if (index == VHOST_NET_VQ_RX)
+ nvq->rx_ring = get_tap_ptr_ring(fd);
oldubufs = nvq->ubufs;
nvq->ubufs = ubufs;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net 2/3] vhost_net: keep private_data and rx_ring synced
2018-03-09 6:50 ` [PATCH net 2/3] vhost_net: keep private_data and rx_ring synced Jason Wang
@ 2018-03-09 16:56 ` Michael S. Tsirkin
0 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2018-03-09 16:56 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, virtualization, netdev, linux-kernel, glider, dvyukov
On Fri, Mar 09, 2018 at 02:50:33PM +0800, Jason Wang wrote:
> We get pointer ring from the exported sock, this means we should keep
> rx_ring and vq->private synced during both vq stop and backend set,
> otherwise we may see stale rx_ring.
>
> Fixes: c67df11f6e480 ("vhost_net: try batch dequing from skb array")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/vhost/net.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 60f1080..efb9306 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -973,6 +973,7 @@ static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> vhost_net_disable_vq(n, vq);
> vq->private_data = NULL;
> vhost_net_buf_unproduce(nvq);
> + nvq->rx_ring = NULL;
> mutex_unlock(&vq->mutex);
> return sock;
> }
> @@ -1162,14 +1163,14 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> vhost_net_disable_vq(n, vq);
> vq->private_data = sock;
> vhost_net_buf_unproduce(nvq);
> - if (index == VHOST_NET_VQ_RX)
> - nvq->rx_ring = get_tap_ptr_ring(fd);
> r = vhost_vq_init_access(vq);
> if (r)
> goto err_used;
> r = vhost_net_enable_vq(n, vq);
> if (r)
> goto err_used;
> + if (index == VHOST_NET_VQ_RX)
> + nvq->rx_ring = get_tap_ptr_ring(fd);
>
> oldubufs = nvq->ubufs;
> nvq->ubufs = ubufs;
> --
> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 3/3] vhost_net: examine pointer types during un-producing
2018-03-09 6:50 [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage Jason Wang
2018-03-09 6:50 ` [PATCH net 1/3] vhost_net: initialize rx_ring in vhost_net_open() Jason Wang
2018-03-09 6:50 ` [PATCH net 2/3] vhost_net: keep private_data and rx_ring synced Jason Wang
@ 2018-03-09 6:50 ` Jason Wang
2018-03-09 16:57 ` [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage Michael S. Tsirkin
2018-03-09 17:03 ` David Miller
4 siblings, 0 replies; 8+ messages in thread
From: Jason Wang @ 2018-03-09 6:50 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel; +Cc: glider, dvyukov
After commit fc72d1d54dd9 ("tuntap: XDP transmission"), we can
actually queueing XDP pointers in the pointer ring, so we should
examine the pointer type before freeing the pointer.
Fixes: fc72d1d54dd9 ("tuntap: XDP transmission")
Reported-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/tun.c | 3 ++-
drivers/vhost/net.c | 2 +-
include/linux/if_tun.h | 4 ++++
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 7433bb2..28cfa64 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -655,7 +655,7 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
return tun;
}
-static void tun_ptr_free(void *ptr)
+void tun_ptr_free(void *ptr)
{
if (!ptr)
return;
@@ -667,6 +667,7 @@ static void tun_ptr_free(void *ptr)
__skb_array_destroy_skb(ptr);
}
}
+EXPORT_SYMBOL_GPL(tun_ptr_free);
static void tun_queue_purge(struct tun_file *tfile)
{
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index efb9306..8139bc7 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -170,7 +170,7 @@ static void vhost_net_buf_unproduce(struct vhost_net_virtqueue *nvq)
if (nvq->rx_ring && !vhost_net_buf_is_empty(rxq)) {
ptr_ring_unconsume(nvq->rx_ring, rxq->queue + rxq->head,
vhost_net_buf_get_size(rxq),
- __skb_array_destroy_skb);
+ tun_ptr_free);
rxq->head = rxq->tail = 0;
}
}
diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h
index c5b0a75..fd00170 100644
--- a/include/linux/if_tun.h
+++ b/include/linux/if_tun.h
@@ -25,6 +25,7 @@ struct ptr_ring *tun_get_tx_ring(struct file *file);
bool tun_is_xdp_buff(void *ptr);
void *tun_xdp_to_ptr(void *ptr);
void *tun_ptr_to_xdp(void *ptr);
+void tun_ptr_free(void *ptr);
#else
#include <linux/err.h>
#include <linux/errno.h>
@@ -50,5 +51,8 @@ static inline void *tun_ptr_to_xdp(void *ptr)
{
return NULL;
}
+static inline void tun_ptr_free(void *ptr)
+{
+}
#endif /* CONFIG_TUN */
#endif /* __IF_TUN_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage
2018-03-09 6:50 [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage Jason Wang
` (2 preceding siblings ...)
2018-03-09 6:50 ` [PATCH net 3/3] vhost_net: examine pointer types during un-producing Jason Wang
@ 2018-03-09 16:57 ` Michael S. Tsirkin
2018-03-09 17:03 ` David Miller
4 siblings, 0 replies; 8+ messages in thread
From: Michael S. Tsirkin @ 2018-03-09 16:57 UTC (permalink / raw)
To: Jason Wang
Cc: kvm, netdev, linux-kernel, virtualization, glider, davem, dvyukov
On Fri, Mar 09, 2018 at 02:50:31PM +0800, Jason Wang wrote:
> Hi:
>
> This small series try to fix several bugs of ptr_ring usage in
> vhost_net. Please review.
>
> Thanks
ACK series, thanks Jason!
> Alexander Potapenko (1):
> vhost_net: initialize rx_ring in vhost_net_open()
>
> Jason Wang (2):
> vhost_net: keep private_data and rx_ring synced
> vhost_net: examine pointer types during un-producing
>
> drivers/net/tun.c | 3 ++-
> drivers/vhost/net.c | 8 +++++---
> include/linux/if_tun.h | 4 ++++
> 3 files changed, 11 insertions(+), 4 deletions(-)
>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage
2018-03-09 6:50 [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage Jason Wang
` (3 preceding siblings ...)
2018-03-09 16:57 ` [PATCH net 0/3] Several fixes for vhost_net ptr_ring usage Michael S. Tsirkin
@ 2018-03-09 17:03 ` David Miller
4 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2018-03-09 17:03 UTC (permalink / raw)
To: jasowang; +Cc: kvm, mst, netdev, linux-kernel, virtualization, glider, dvyukov
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 9 Mar 2018 14:50:31 +0800
> This small series try to fix several bugs of ptr_ring usage in
> vhost_net. Please review.
Series applied, thanks Jason.
^ permalink raw reply [flat|nested] 8+ messages in thread