Messages in this thread | | | Date | Thu, 26 Oct 2023 00:19:32 -0700 | Subject | Re: [PATCH] vhost-vdpa: fix use-after-free in _compat_vdpa_reset | From | Si-Wei Liu <> |
| |
On 10/25/2023 11:55 PM, Si-Wei Liu wrote: > > > On 10/25/2023 10:26 PM, Michael S. Tsirkin wrote: >> On Wed, Oct 25, 2023 at 04:13:14PM -0700, Si-Wei Liu wrote: >>> When the vhost-vdpa device is being closed, vhost_vdpa_cleanup() >>> doesn't >>> clean up the vqs pointer after free. This could lead to use-after-tree >>> when _compat_vdpa_reset() tries to access the vqs that are freed >>> already. >>> Fix is to set vqs pointer to NULL at the end of vhost_vdpa_cleanup() >>> after getting freed, which is guarded by atomic opened state. >>> >>> BUG: unable to handle page fault for address: 00000001005b4af4 >>> #PF: supervisor read access in kernel mode >>> #PF: error_code(0x0000) - not-present page >>> PGD 16a80a067 P4D 0 >>> Oops: 0000 [#1] PREEMPT SMP NOPTI >>> CPU: 4 PID: 40387 Comm: qemu-kvm Not tainted 6.6.0-rc7+ #3 >>> Hardware name: Dell Inc. PowerEdge R750/0PJ80M, BIOS 1.8.2 >>> 09/14/2022 >>> RIP: 0010:_compat_vdpa_reset.isra.0+0x27/0xb0 [vhost_vdpa] >>> Code: 90 90 90 0f 1f 44 00 00 41 55 4c 8d ae 08 03 00 00 41 54 55 48 >>> 89 f5 53 4c 8b a6 00 03 00 00 48 85 ff 74 49 48 8b 07 4c 89 ef >>> <48> 8b >>> 80 88 45 00 00 48 c1 e8 08 48 83 f0 01 89 c3 e8 73 5e 9b dc >>> RSP: 0018:ff73a85762073ba0 EFLAGS: 00010286 >>> RAX: 00000001005b056c RBX: ff32b13ca6994c68 RCX: 0000000000000002 >>> RDX: 0000000000000001 RSI: ff32b13c07559000 RDI: ff32b13c07559308 >>> RBP: ff32b13c07559000 R08: 0000000000000000 R09: ff32b12ca497c0f0 >>> R10: ff73a85762073c58 R11: 0000000c106f9de3 R12: ff32b12c95b1d050 >>> R13: ff32b13c07559308 R14: ff32b12d0ddc5100 R15: 0000000000008002 >>> FS: 00007fec5b8cbf80(0000) GS:ff32b13bbfc80000(0000) >>> knlGS:0000000000000000 >>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> CR2: 00000001005b4af4 CR3: 000000015644a003 CR4: 0000000000773ee0 >>> PKRU: 55555554 >>> Call Trace: >>> <TASK> >>> ? __die+0x20/0x70 >>> ? page_fault_oops+0x76/0x170 >>> ? exc_page_fault+0x65/0x150 >>> ? asm_exc_page_fault+0x22/0x30 >>> ? _compat_vdpa_reset.isra.0+0x27/0xb0 [vhost_vdpa] >>> vhost_vdpa_open+0x57/0x280 [vhost_vdpa] >>> ? __pfx_chrdev_open+0x10/0x10 >>> chrdev_open+0xc6/0x260 >>> ? __pfx_chrdev_open+0x10/0x10 >>> do_dentry_open+0x16e/0x530 >>> do_open+0x21c/0x400 >>> path_openat+0x111/0x290 >>> do_filp_open+0xb2/0x160 >>> ? __check_object_size.part.0+0x5e/0x140 >>> do_sys_openat2+0x96/0xd0 >>> __x64_sys_openat+0x53/0xa0 >>> do_syscall_64+0x59/0x90 >>> ? syscall_exit_to_user_mode+0x22/0x40 >>> ? do_syscall_64+0x69/0x90 >>> ? syscall_exit_to_user_mode+0x22/0x40 >>> ? do_syscall_64+0x69/0x90 >>> ? do_syscall_64+0x69/0x90 >>> ? syscall_exit_to_user_mode+0x22/0x40 >>> ? do_syscall_64+0x69/0x90 >>> ? exc_page_fault+0x65/0x150 >>> entry_SYSCALL_64_after_hwframe+0x6e/0xd8 >>> >>> Fixes: 10cbf8dfaf93 ("vhost-vdpa: clean iotlb map during reset for >>> older userspace") >>> Fixes: ac7e98c73c05 ("vhost-vdpa: fix NULL pointer deref in >>> _compat_vdpa_reset") >> So these two are all in next correct? >> >> I really do not like it how 10cbf8dfaf936e3ef1f5d7fdc6e9dada268ba6bb >> introduced a regression and then apparently we keep fixing things up? > Sorry my bad. The latest one should be all of it. > >> >> Can I squash these 3 commits? > Sure. Or if you want me to send a v5 with all 3 commits squashed in, I > can do for sure. Saw you squashed it with the 2 fixups in place, thank you! Sent a v5 anyway, just in case if you need a fresh series.
Thanks, -Siwei
> > Thanks, > -Siwei >> >> >>> Reported-by: Lei Yang <leiyang@redhat.com> >>> Closes: >>> https://lore.kernel.org/all/CAPpAL=yHDqn1AztEcN3MpS8o4M+BL_HVy02FdpiHN7DWd91HwQ@mail.gmail.com/ >>> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com> >>> --- >>> drivers/vhost/vdpa.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c >>> index 9a2343c45df0..30df5c58db73 100644 >>> --- a/drivers/vhost/vdpa.c >>> +++ b/drivers/vhost/vdpa.c >>> @@ -1355,6 +1355,7 @@ static void vhost_vdpa_cleanup(struct >>> vhost_vdpa *v) >>> vhost_vdpa_free_domain(v); >>> vhost_dev_cleanup(&v->vdev); >>> kfree(v->vdev.vqs); >>> + v->vdev.vqs = NULL; >>> } >>> static int vhost_vdpa_open(struct inode *inode, struct file *filep) >>> -- >>> 2.39.3 >
| |