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")
Reported-by: Lei Yang <[email protected]>
Closes: https://lore.kernel.org/all/CAPpAL=yHDqn1AztEcN3MpS8o4M+BL_HVy02FdpiHN7DWd91HwQ@mail.gmail.com/
Signed-off-by: Si-Wei Liu <[email protected]>
---
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
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?
Can I squash these 3 commits?
> Reported-by: Lei Yang <[email protected]>
> Closes: https://lore.kernel.org/all/CAPpAL=yHDqn1AztEcN3MpS8o4M+BL_HVy02FdpiHN7DWd91HwQ@mail.gmail.com/
> Signed-off-by: Si-Wei Liu <[email protected]>
> ---
> 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
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.
Thanks,
-Siwei
>
>
>> Reported-by: Lei Yang <[email protected]>
>> Closes: https://lore.kernel.org/all/CAPpAL=yHDqn1AztEcN3MpS8o4M+BL_HVy02FdpiHN7DWd91HwQ@mail.gmail.com/
>> Signed-off-by: Si-Wei Liu <[email protected]>
>> ---
>> 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
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 <[email protected]>
>>> Closes:
>>> https://lore.kernel.org/all/CAPpAL=yHDqn1AztEcN3MpS8o4M+BL_HVy02FdpiHN7DWd91HwQ@mail.gmail.com/
>>> Signed-off-by: Si-Wei Liu <[email protected]>
>>> ---
>>> 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
>