2018-09-21 07:46:36

by Jorgen Hansen

[permalink] [raw]
Subject: [PATCH] VMCI: Resource wildcard match fixed

When adding a VMCI resource, the check for an existing entry
would ignore that the new entry could be a wildcard. This could
result in multiple resource entries that would match a given
handle. One disastrous outcome of this is that the
refcounting used to ensure that delayed callbacks for VMCI
datagrams have run before the datagram is destroyed can be
wrong, since the refcount could be increased on the duplicate
entry. This in turn leads to a use after free bug. This issue
was discovered by Hangbin Liu using KASAN and syzkaller.

Fixes: bc63dedb7d46 ("VMCI: resource object implementation")
Reported-by: Hangbin Liu <[email protected]>
Reviewed-by: Adit Ranadive <[email protected]>
Reviewed-by: Vishnu Dasa <[email protected]>
Signed-off-by: Jorgen Hansen <[email protected]>
---
drivers/misc/vmw_vmci/vmci_driver.c | 2 +-
drivers/misc/vmw_vmci/vmci_resource.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_driver.c b/drivers/misc/vmw_vmci/vmci_driver.c
index d7eaf1eb11e7..003bfba40758 100644
--- a/drivers/misc/vmw_vmci/vmci_driver.c
+++ b/drivers/misc/vmw_vmci/vmci_driver.c
@@ -113,5 +113,5 @@ module_exit(vmci_drv_exit);

MODULE_AUTHOR("VMware, Inc.");
MODULE_DESCRIPTION("VMware Virtual Machine Communication Interface.");
-MODULE_VERSION("1.1.5.0-k");
+MODULE_VERSION("1.1.6.0-k");
MODULE_LICENSE("GPL v2");
diff --git a/drivers/misc/vmw_vmci/vmci_resource.c b/drivers/misc/vmw_vmci/vmci_resource.c
index 1ab6e8737a5f..da1ee2e1ba99 100644
--- a/drivers/misc/vmw_vmci/vmci_resource.c
+++ b/drivers/misc/vmw_vmci/vmci_resource.c
@@ -57,7 +57,8 @@ static struct vmci_resource *vmci_resource_lookup(struct vmci_handle handle,

if (r->type == type &&
rid == handle.resource &&
- (cid == handle.context || cid == VMCI_INVALID_ID)) {
+ (cid == handle.context || cid == VMCI_INVALID_ID ||
+ handle.context == VMCI_INVALID_ID)) {
resource = r;
break;
}
--
2.17.1



2018-10-02 22:35:35

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] VMCI: Resource wildcard match fixed

On Fri, Sep 21, 2018 at 12:31:05AM -0700, Jorgen Hansen wrote:
> When adding a VMCI resource, the check for an existing entry
> would ignore that the new entry could be a wildcard. This could
> result in multiple resource entries that would match a given
> handle. One disastrous outcome of this is that the
> refcounting used to ensure that delayed callbacks for VMCI
> datagrams have run before the datagram is destroyed can be
> wrong, since the refcount could be increased on the duplicate
> entry. This in turn leads to a use after free bug. This issue
> was discovered by Hangbin Liu using KASAN and syzkaller.
>
> Fixes: bc63dedb7d46 ("VMCI: resource object implementation")
> Reported-by: Hangbin Liu <[email protected]>
> Reviewed-by: Adit Ranadive <[email protected]>
> Reviewed-by: Vishnu Dasa <[email protected]>
> Signed-off-by: Jorgen Hansen <[email protected]>
> ---
> drivers/misc/vmw_vmci/vmci_driver.c | 2 +-
> drivers/misc/vmw_vmci/vmci_resource.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/vmw_vmci/vmci_driver.c b/drivers/misc/vmw_vmci/vmci_driver.c
> index d7eaf1eb11e7..003bfba40758 100644
> --- a/drivers/misc/vmw_vmci/vmci_driver.c
> +++ b/drivers/misc/vmw_vmci/vmci_driver.c
> @@ -113,5 +113,5 @@ module_exit(vmci_drv_exit);
>
> MODULE_AUTHOR("VMware, Inc.");
> MODULE_DESCRIPTION("VMware Virtual Machine Communication Interface.");
> -MODULE_VERSION("1.1.5.0-k");
> +MODULE_VERSION("1.1.6.0-k");
> MODULE_LICENSE("GPL v2");

You do know MODULE_VERSION means nothing, right? Please just remove it.

thanks,

greg k-h

2018-10-09 08:44:43

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] VMCI: Resource wildcard match fixed

On Tue, Oct 09, 2018 at 08:27:41AM +0000, Jorgen S. Hansen wrote:
>
>
> On 3 Oct 2018, at 00:35, Greg KH <[email protected]<mailto:[email protected]>> wrote:
>
> On Fri, Sep 21, 2018 at 12:31:05AM -0700, Jorgen Hansen wrote:
> When adding a VMCI resource, the check for an existing entry
> would ignore that the new entry could be a wildcard. This could
> result in multiple resource entries that would match a given
> handle. One disastrous outcome of this is that the
> refcounting used to ensure that delayed callbacks for VMCI
> datagrams have run before the datagram is destroyed can be
> wrong, since the refcount could be increased on the duplicate
> entry. This in turn leads to a use after free bug. This issue
> was discovered by Hangbin Liu using KASAN and syzkaller.
>
> Fixes: bc63dedb7d46 ("VMCI: resource object implementation")
> Reported-by: Hangbin Liu <[email protected]<mailto:[email protected]>>
> Reviewed-by: Adit Ranadive <[email protected]<mailto:[email protected]>>
> Reviewed-by: Vishnu Dasa <[email protected]<mailto:[email protected]>>
> Signed-off-by: Jorgen Hansen <[email protected]<mailto:[email protected]>>
> ---
> drivers/misc/vmw_vmci/vmci_driver.c | 2 +-
> drivers/misc/vmw_vmci/vmci_resource.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/vmw_vmci/vmci_driver.c b/drivers/misc/vmw_vmci/vmci_driver.c
> index d7eaf1eb11e7..003bfba40758 100644
> --- a/drivers/misc/vmw_vmci/vmci_driver.c
> +++ b/drivers/misc/vmw_vmci/vmci_driver.c
> @@ -113,5 +113,5 @@ module_exit(vmci_drv_exit);
>
> MODULE_AUTHOR("VMware, Inc.");
> MODULE_DESCRIPTION("VMware Virtual Machine Communication Interface.");
> -MODULE_VERSION("1.1.5.0-k");
> +MODULE_VERSION("1.1.6.0-k");
> MODULE_LICENSE("GPL v2");
>
> You do know MODULE_VERSION means nothing, right? Please just remove it.
>
> Sure. Do you want a new version of this patch with it removed ? (The reason for asking is that I already got a couple of notifications about the patch being added to char-misc and char-misc-next).

Just send a follow-on patch that removes it please.

thanks,

greg k-h