The connect() error path frees vif->queues without first waiting for
RCU readers, unlike backend_disconnect() which publishes num_queues = 0,
calls synchronize_net() and only then frees the array.
The net device is registered in xenvif_alloc() during probe, so it is
already live when connect() runs. xenvif_get_stats64() and
xenvif_get_ethtool_stats() read it through the usual stats paths, taking
rcu_read_lock(), sampling num_queues with READ_ONCE() and then
dereferencing vif->queues[i].
A guest can drive connect() into the error path by handing the backend a
bad ring reference or event channel for one of its queues, which makes
connect_data_rings() fail and connect() jump to err: with num_queues
still counting the queues already set up. The error path calls
vfree(vif->queues) without a preceding synchronize_net(), so a reader
that has already sampled a non-zero num_queues and loaded the vif->queues
pointer goes on to dereference the freed vmalloc array.
KASAN catches the read of the freed queue array in xenvif_get_stats()
while a guest repeatedly forces the error path and dom0 reads
/proc/net/dev. Generic KASAN labels a freed vmalloc region
vmalloc-out-of-bounds, since the use-after-free label is reserved for
slab and page objects:
[ 160.524029] ==================================================================
[ 160.527557] BUG: KASAN: vmalloc-out-of-bounds in xenvif_get_stats+0x28f/0x390
[ 160.530848] Read of size 8 at addr ffffc90005de06a8 by task cat/10519
[ 160.533831]
[ 160.534844] CPU: 1 UID: 0 PID: 10519 Comm: cat Not tainted 7.3.0-kCTLwide2-15482-g21a7f44fd076-dirty #2 PREEMPT(full)
[ 160.534849] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 160.534855] Call Trace:
[ 160.534868]
[ 160.534870] dump_stack_lvl+0x64/0x80
[ 160.534885] print_report+0xd0/0x630
[ 160.534918] ? __pfx__raw_spin_lock_irqsave+0x10/0x10
[ 160.534933] ? xenvif_get_stats+0x28f/0x390
[ 160.534938] kasan_report+0xce/0x100
[ 160.534943] ? xenvif_get_stats+0x28f/0x390
[ 160.534947] xenvif_get_stats+0x28f/0x390
[ 160.534952] dev_get_stats+0x2db/0xb50
[ 160.534961] ? dev_seq_printf_stats+0x446/0x7b0
[ 160.534979] dev_seq_printf_stats+0x9e/0x7b0
[ 160.534982] ? __pfx_dev_seq_printf_stats+0x10/0x10
[ 160.534987] ? dev_seq_next+0xe0/0x1b0
[ 160.534990] ? __pfx_dev_seq_next+0x10/0x10
[ 160.534993] ? __asan_memcpy+0x3c/0x60
[ 160.534997] dev_seq_show+0xf/0x30
[ 160.535000] seq_read_iter+0x8f3/0x1190
[ 160.535018] ? alloc_pipe_info+0x2c2/0x430
[ 160.535030] seq_read+0x267/0x3d0
[ 160.535033] ? __pfx_seq_read+0x10/0x10
[ 160.535038] ? __pfx_do_mmap+0x10/0x10
[ 160.535054] proc_reg_read+0x1ab/0x270
[ 160.535077] vfs_read+0x177/0xa20
[ 160.535085] ? __pfx_vfs_read+0x10/0x10
[ 160.535089] ? vm_mmap_pgoff+0x24b/0x310
[ 160.535108] ? __pfx_vm_mmap_pgoff+0x10/0x10
[ 160.535116] ? fdget_pos+0x1ca/0x4b0
[ 160.535124] ksys_read+0xf7/0x1c0
[ 160.535128] ? __pfx_ksys_read+0x10/0x10
[ 160.535132] do_syscall_64+0xe2/0x4b0
[ 160.535141] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 160.535149] RIP: 0033:0x463aa1
[ 160.535156] Code: 00 4c 89 d2 4c 89 c6 e9 fd fd ff ff 0f 1f 44 00 00 31 c0 c3 0f 1f 44 00 00 f3 0f 1e fa 80 3d ec 22 1a 00 00 74 13 31 c0 0f 05 <48> 3d 00 f0 ff ff 77 4f c3 66 0f 1f 44 00 00 55 48 89 e5 48 83 ec
[ 160.535168] RSP: 002b:00007fff303dbeb8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
[ 160.535176] RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000463aa1
[ 160.535178] RDX: 0000000000010000 RSI: 00007f9becf9f000 RDI: 0000000000000003
[ 160.535180] RBP: 00007fff303dbee0 R08: ffffffffffffffff R09: 0000000000000000
[ 160.535182] R10: 0000000000000022 R11: 0000000000000246 R12: 00007f9becf9f000
[ 160.535183] R13: 0000000000010000 R14: 0000000000000001 R15: 0000000000000000
[ 160.535186]
[ 160.535188]
[ 160.636633] The buggy address belongs to a vmalloc virtual mapping
[ 160.639416] Memory state around the buggy address:
[ 160.641738] ffffc90005de0580: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
[ 160.662534] ffffc90005de0600: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
[ 160.665865] >ffffc90005de0680: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
[ 160.669222] ^
[ 160.671430] ffffc90005de0700: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
[ 160.674771] ffffc90005de0780: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
[ 160.678102] ==================================================================
Mirror the teardown that backend_disconnect() already performs. Snapshot
num_queues, publish num_queues = 0, call synchronize_net() to wait for
in-flight readers, and only then deinit and free the array using the
snapshot.
Fixes: b17075d5c198 ("xen-netback: fix race condition on XenBus disconnect")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee
---
drivers/net/xen-netback/xenbus.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index e020f44ac2c717..d1c9e38cfb8ef2 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -835,10 +835,21 @@ static void connect(struct backend_info *be)
return;
err:
- if (be->vif->num_queues > 0)
+ if (be->vif->num_queues > 0) {
+ unsigned int num_queues = be->vif->num_queues;
+
xenvif_disconnect_data(be->vif); /* Clean up existing queues */
- for (queue_index = 0; queue_index < be->vif->num_queues; ++queue_index)
- xenvif_deinit_queue(&be->vif->queues[queue_index]);
+
+ /*
+ * Mirror backend_disconnect(): publish num_queues == 0 and
+ * synchronize_net() so RCU stats readers finish before the free.
+ */
+ be->vif->num_queues = 0;
+ synchronize_net();
+
+ for (queue_index = 0; queue_index < num_queues; ++queue_index)
+ xenvif_deinit_queue(&be->vif->queues[queue_index]);
+ }
vfree(be->vif->queues);
be->vif->queues = NULL;
be->vif->num_queues = 0;
--
2.43.0