Re: [PATCH] Fix double-free on error condition in GnttabPermitForeignAccess()

[Owen Smith <owen.smith@xxxxxxxxxx>]

Is there other cleanup needed in other fail cases?

Context->Grants, Context->KernelVa, Context->Mdl are allocated and pages could need revoking access during the cleanup operation in a failure case.

Calling GnttabStopSharing() should do this, but its not called in the case where GnttabPermitForeignAccess returns a failure code.

Owen

On Thu, Mar 7, 2024 at 8:51 AM Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx> wrote:

XENIFACE_GNTTAB_CONTEXT associated with the request was incorrectly freed
by GnttabPermitForeignAccess() when a failure occured. The context is also
freed by the parent function, IoctlGnttabPermitForeignAccess(), which led
to a double-free and kernel heap corruption.

Signed-off-by: Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx>
---
 src/xeniface/ioctl_gnttab.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/src/xeniface/ioctl_gnttab.c b/src/xeniface/ioctl_gnttab.c
index 8ab2099..083c29d 100644
--- a/src/xeniface/ioctl_gnttab.c
+++ b/src/xeniface/ioctl_gnttab.c
@@ -303,13 +303,6 @@ fail2:

 fail1:
     Error("Fail1\n");
-    GnttabStopSharing(Fdo, Context, Page);
-
-    if (Context != NULL) {
-        RtlZeroMemory(Context, sizeof(*Context));
-        __FreePoolWithTag(Context, XENIFACE_POOL_TAG);
-    }
-
     return Status;
 }

--
2.43.0.windows.1