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

[Owen Smith <owen.smith@xxxxxxxxxx>]

Reviewed-by: Owen Smith <owen.smith@xxxxxxxxx>

On Thu, Mar 7, 2024 at 10:19 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.

GnttabStopSharing() as the final step in the failure path takes care
of cleaning up the contents of the (possibly partially initialized) context.

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

diff --git a/src/xeniface/ioctl_gnttab.c b/src/xeniface/ioctl_gnttab.c
index 8ab2099..026f29b 100644
--- a/src/xeniface/ioctl_gnttab.c
+++ b/src/xeniface/ioctl_gnttab.c
@@ -304,12 +304,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