Stefano Garzarella
2021-Mar-11 13:52 UTC
[PATCH 0/2] vhost-vdpa: fix issues around v->config_ctx handling
While writing a test for a Rust library [1] to handle vhost-vdpa devices, I experienced the 'use-after-free' issue fixed in patch 1, then I discovered the potential issue when eventfd_ctx_fdget() fails fixed in patch 2. Do you think it might be useful to write a vdpa test suite, perhaps using this Rust library [2] ? Could we put it in the kernel tree in tool/testing? Thanks, Stefano [1] https://github.com/stefano-garzarella/vhost/tree/vdpa [2] https://github.com/rust-vmm/vhost Stefano Garzarella (2): vhost-vdpa: fix use-after-free of v->config_ctx vhost-vdpa: set v->config_ctx to NULL if eventfd_ctx_fdget() fails drivers/vhost/vdpa.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) -- 2.29.2
Stefano Garzarella
2021-Mar-11 13:52 UTC
[PATCH 1/2] vhost-vdpa: fix use-after-free of v->config_ctx
When the 'v->config_ctx' eventfd_ctx reference is released we didn't set it to NULL. So if the same character device (e.g. /dev/vhost-vdpa-0) is re-opened, the 'v->config_ctx' is invalid and calling again vhost_vdpa_config_put() causes use-after-free issues like the following refcount_t underflow: refcount_t: underflow; use-after-free. WARNING: CPU: 2 PID: 872 at lib/refcount.c:28 refcount_warn_saturate+0xae/0xf0 RIP: 0010:refcount_warn_saturate+0xae/0xf0 Call Trace: eventfd_ctx_put+0x5b/0x70 vhost_vdpa_release+0xcd/0x150 [vhost_vdpa] __fput+0x8e/0x240 ____fput+0xe/0x10 task_work_run+0x66/0xa0 exit_to_user_mode_prepare+0x118/0x120 syscall_exit_to_user_mode+0x21/0x50 ? __x64_sys_close+0x12/0x40 do_syscall_64+0x45/0x50 entry_SYSCALL_64_after_hwframe+0x44/0xae Fixes: 776f395004d8 ("vhost_vdpa: Support config interrupt in vdpa") Cc: lingshan.zhu at intel.com Cc: stable at vger.kernel.org Signed-off-by: Stefano Garzarella <sgarzare at redhat.com> --- drivers/vhost/vdpa.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index ef688c8c0e0e..00796e4ecfdf 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -308,8 +308,10 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp) static void vhost_vdpa_config_put(struct vhost_vdpa *v) { - if (v->config_ctx) + if (v->config_ctx) { eventfd_ctx_put(v->config_ctx); + v->config_ctx = NULL; + } } static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp) -- 2.29.2
Stefano Garzarella
2021-Mar-11 13:52 UTC
[PATCH 2/2] vhost-vdpa: set v->config_ctx to NULL if eventfd_ctx_fdget() fails
In vhost_vdpa_set_config_call() if eventfd_ctx_fdget() fails the 'v->config_ctx' contains an error instead of a valid pointer. Since we consider 'v->config_ctx' valid if it is not NULL, we should set it to NULL in this case to avoid to use an invalid pointer in other functions such as vhost_vdpa_config_put(). Fixes: 776f395004d8 ("vhost_vdpa: Support config interrupt in vdpa") Cc: lingshan.zhu at intel.com Cc: stable at vger.kernel.org Signed-off-by: Stefano Garzarella <sgarzare at redhat.com> --- drivers/vhost/vdpa.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c index 00796e4ecfdf..f9ecdce5468a 100644 --- a/drivers/vhost/vdpa.c +++ b/drivers/vhost/vdpa.c @@ -331,8 +331,12 @@ static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp) if (!IS_ERR_OR_NULL(ctx)) eventfd_ctx_put(ctx); - if (IS_ERR(v->config_ctx)) - return PTR_ERR(v->config_ctx); + if (IS_ERR(v->config_ctx)) { + long ret = PTR_ERR(v->config_ctx); + + v->config_ctx = NULL; + return ret; + } v->vdpa->config->set_config_cb(v->vdpa, &cb); -- 2.29.2
Michael S. Tsirkin
2021-Mar-11 16:42 UTC
[PATCH 0/2] vhost-vdpa: fix issues around v->config_ctx handling
On Thu, Mar 11, 2021 at 02:52:55PM +0100, Stefano Garzarella wrote:> While writing a test for a Rust library [1] to handle vhost-vdpa devices, > I experienced the 'use-after-free' issue fixed in patch 1, then I > discovered the potential issue when eventfd_ctx_fdget() fails fixed in > patch 2. > > Do you think it might be useful to write a vdpa test suite, perhaps using > this Rust library [2] ? > Could we put it in the kernel tree in tool/testing?I can add tools/vdpa, no problem ...> Thanks, > Stefano > > [1] https://github.com/stefano-garzarella/vhost/tree/vdpa > [2] https://github.com/rust-vmm/vhost > > Stefano Garzarella (2): > vhost-vdpa: fix use-after-free of v->config_ctx > vhost-vdpa: set v->config_ctx to NULL if eventfd_ctx_fdget() fails > > drivers/vhost/vdpa.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > -- > 2.29.2