[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] More IOCTL parameter checks

  • To: <win-pv-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Owen Smith <owen.smith@xxxxxxxxxx>
  • Date: Mon, 25 Apr 2022 13:14:01 +0100
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Owen Smith <owen.smith@xxxxxxxxxx>
  • Delivery-date: Mon, 25 Apr 2022 12:14:17 +0000
  • Ironport-data: A9a23:XofM361vfYBL2jiK9PbD5Y9xkn2cJEfYwER7XKvMYLTBsI5bpzYDz GEYDG6APv/cM2Wkft4lPt+2pxkPuJCBx9JgHVc6pC1hF35El5HIVI+TRqvS04J+DSFhoGZPt Zh2hgzodZhsJpPkjk7xdOCn9xGQ7InQLlbGILes1htZGEk1EE/NtTo5w7Rj2tIy3IDga++wk YiaT/P3aQfNNwFcagr424rbwP+4lK2v0N+wlgVWicFj5DcypVFMZH4sDfjZw0/DaptVBoaHq 9Prl9lVyI97EyAFUbtJmp6jGqEDryW70QKm0hK6UID66vROS7BbPg/W+5PwZG8O4whlkeydx /1IipaebxkvBpSSv7RMYyVfSy9zZupvreqvzXiX6aR/zmXDenrohf5vEFs3LcsT/eMf7WNmr KJCbmpXN1ba2rzwkOnTpupE36zPKOHsI44Z/GplzC3ZJf0nXYrCU+PB4towMDIY2ZgUQKiGO pdxhTxHRjP+b0FMFFEsMIM/prqvhiXOT35/twfAzUYwyzeKl1EguFT3C/LKe9rPXdsQkkuGq 2bu+2XiHgpcJNGZ0SCC8H+nmqnIhyyTZW4JPOTmrLgw2gTVnzFNTk1NPbemnRWnokuBUfx5N Uosw3QJ/fkCyEvzQNSmGDTt9RZooSUgc9ZXFuQ77iSExazV/xuVCwA4c9JRVDA1nJRoHGJ3j zdli/usXGUy6+PNFRpx45/O9VuP1T4pwXjujMPuZS8M+JHdrY46lXojpf4zQffu3rUZ9dwdq g1mTRTSZZ1O16bnNI3hpDgrZg5AQbCTE2YICv3/BD7N0++ATNfNi3aUwVba9+1cC42SU0OMu nMJ8+DHsr1WUszUzHbcHb9TdF1M2xpjGGeF6bKIN8N/nwlBBlb5JdwAiN2ADBoB3jk4lc/BP xaI5FI5CG57N3q2d65nC79d+OxxpZUM4e/ND6iOBvIXO8AZXFberElGOB7Bt0iwwRNEufxuZ v+mnTOEUC9y5VJPl2HtGY/wENYDm0gD+I8kbc2ln0n/jerCNRZ4i94taTOzUwzw14vcyC29z jqVH5HiJ8l3OAEmXhTqzA==
  • Ironport-hdrordr: A9a23:4htiA6vR/MSj1wYcEbin50Si7skDfNV00zEX/kB9WHVpmszxra +TdZMgpHrJYVcqKRYdcL+7WZVoLUmwyXcX2/hyAV7BZmnbUQKTRekIh7cKqweQfxEWndQy6U 4PScRD4aXLfDtHsfo=
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>
Also expands ASSERT(NT_SUCCESS(function)) calls in cleanup paths

Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
---
 src/xeniface/ioctl_evtchn.c     | 15 ++++++++++-----
 src/xeniface/ioctl_gnttab.c     | 26 ++++++++++++++++----------
 src/xeniface/ioctl_sharedinfo.c |  5 +++++
 src/xeniface/ioctl_store.c      | 17 ++++++++++-------
 src/xeniface/ioctl_suspend.c    | 11 +++++++++--
 src/xeniface/ioctls.c           |  4 ++--
 6 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/src/xeniface/ioctl_evtchn.c b/src/xeniface/ioctl_evtchn.c
index f528485..90e9468 100644
--- a/src/xeniface/ioctl_evtchn.c
+++ b/src/xeniface/ioctl_evtchn.c
@@ -166,7 +166,8 @@ IoctlEvtchnBindUnbound(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_EVTCHN_BIND_UNBOUND_IN) ||
-        OutLen != sizeof(XENIFACE_EVTCHN_BIND_UNBOUND_OUT)) {
+        OutLen != sizeof(XENIFACE_EVTCHN_BIND_UNBOUND_OUT) ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -259,7 +260,8 @@ IoctlEvtchnBindInterdomain(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_EVTCHN_BIND_INTERDOMAIN_IN) ||
-        OutLen != sizeof(XENIFACE_EVTCHN_BIND_INTERDOMAIN_OUT)) {
+        OutLen != sizeof(XENIFACE_EVTCHN_BIND_INTERDOMAIN_OUT) ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -353,7 +355,8 @@ IoctlEvtchnClose(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_EVTCHN_CLOSE_IN) ||
-        OutLen != 0) {
+        OutLen != 0 ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -430,7 +433,8 @@ IoctlEvtchnNotify(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_EVTCHN_NOTIFY_IN) ||
-        OutLen != 0) {
+        OutLen != 0 ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -462,7 +466,8 @@ IoctlEvtchnUnmask(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_EVTCHN_UNMASK_IN) ||
-        OutLen != 0) {
+        OutLen != 0 ||
+        Buffer == NULL) {
         goto fail1;
     }
 
diff --git a/src/xeniface/ioctl_gnttab.c b/src/xeniface/ioctl_gnttab.c
index c3cf129..5777ac8 100644
--- a/src/xeniface/ioctl_gnttab.c
+++ b/src/xeniface/ioctl_gnttab.c
@@ -174,7 +174,8 @@ IoctlGnttabPermitForeignAccess(
     }
 
     status = STATUS_INVALID_BUFFER_SIZE;
-    if (OutLen != 
(ULONG)FIELD_OFFSET(XENIFACE_GNTTAB_PERMIT_FOREIGN_ACCESS_OUT, 
References[In->NumberPages]))
+    if (OutLen != 
(ULONG)FIELD_OFFSET(XENIFACE_GNTTAB_PERMIT_FOREIGN_ACCESS_OUT, 
References[In->NumberPages]) ||
+        Out == NULL)
         goto fail5;
 
     status = STATUS_NO_MEMORY;
@@ -310,11 +311,12 @@ fail11:
     Error("Fail11: Page = %lu\n", Page);
 
     while (Page > 0) {
-        ASSERT(NT_SUCCESS(XENBUS_GNTTAB(RevokeForeignAccess,
+        NTSTATUS status2 = XENBUS_GNTTAB(RevokeForeignAccess,
                                         &Fdo->GnttabInterface,
                                         Fdo->GnttabCache,
                                         FALSE,
-                                        Context->Grants[Page - 1])));
+                                        Context->Grants[Page - 1]);
+        ASSERT(NT_SUCCESS(status2));
 
         --Page;
     }
@@ -425,7 +427,7 @@ IoctlGnttabRevokeForeignAccess(
     PXENIFACE_CONTEXT_ID ContextId;
 
     status = STATUS_INVALID_BUFFER_SIZE;
-    if (InLen != sizeof(XENIFACE_GNTTAB_REVOKE_FOREIGN_ACCESS_IN))
+    if (InLen != sizeof(XENIFACE_GNTTAB_REVOKE_FOREIGN_ACCESS_IN) || Buffer == 
NULL)
         goto fail1;
 
     Id.Type = XENIFACE_CONTEXT_GRANT;
@@ -476,7 +478,8 @@ IoctlGnttabMapForeignPages(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen < sizeof(XENIFACE_GNTTAB_MAP_FOREIGN_PAGES_IN) ||
-        OutLen != sizeof(XENIFACE_GNTTAB_MAP_FOREIGN_PAGES_OUT)) {
+        OutLen != sizeof(XENIFACE_GNTTAB_MAP_FOREIGN_PAGES_OUT) ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -618,10 +621,12 @@ fail10:
 
 fail9:
     Error("Fail9\n");
-    ASSERT(NT_SUCCESS(XENBUS_GNTTAB(UnmapForeignPages,
-                                    &Fdo->GnttabInterface,
-                                    Context->Address
-                                    )));
+    {
+        NTSTATUS status2 = XENBUS_GNTTAB(UnmapForeignPages,
+                                         &Fdo->GnttabInterface,
+                                         Context->Address);
+        ASSERT(NT_SUCCESS(status2));
+    }
 
 fail8:
     Error("Fail8\n");
@@ -714,7 +719,8 @@ IoctlGnttabUnmapForeignPages(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_GNTTAB_UNMAP_FOREIGN_PAGES_IN) ||
-        OutLen != 0) {
+        OutLen != 0 ||
+        Buffer == NULL) {
         goto fail1;
     }
 
diff --git a/src/xeniface/ioctl_sharedinfo.c b/src/xeniface/ioctl_sharedinfo.c
index 7870732..eab35a5 100644
--- a/src/xeniface/ioctl_sharedinfo.c
+++ b/src/xeniface/ioctl_sharedinfo.c
@@ -56,6 +56,9 @@ IoctlSharedInfoGetTime(
     if (OutLen != sizeof(XENIFACE_SHAREDINFO_GET_TIME_OUT))
         goto fail2;
 
+    if (Buffer == NULL)
+        goto fail3;
+
     Out = (PXENIFACE_SHAREDINFO_GET_TIME_OUT)Buffer;
     XENBUS_SHARED_INFO(GetTime, &Fdo->SharedInfoInterface, &Time,
                        &Local);
@@ -66,6 +69,8 @@ IoctlSharedInfoGetTime(
 
     return STATUS_SUCCESS;
 
+fail3:
+    Error("fail3\n");
 fail2:
     Error("Fail2\n");
 fail1:
diff --git a/src/xeniface/ioctl_store.c b/src/xeniface/ioctl_store.c
index 97062df..3964662 100644
--- a/src/xeniface/ioctl_store.c
+++ b/src/xeniface/ioctl_store.c
@@ -105,7 +105,7 @@ IoctlStoreRead(
     BOOLEAN     SquashError = FALSE;
 
     status = STATUS_INVALID_BUFFER_SIZE;
-    if (InLen == 0)
+    if (InLen == 0 || Buffer == NULL)
         goto fail1;
 
     status = STATUS_INVALID_PARAMETER;
@@ -173,7 +173,7 @@ IoctlStoreWrite(
     ULONG       Length;
 
     status = STATUS_INVALID_BUFFER_SIZE;
-    if (InLen == 0 || OutLen != 0)
+    if (InLen == 0 || OutLen != 0 || Buffer == NULL)
         goto fail1;
 
     status = STATUS_INVALID_PARAMETER;
@@ -221,7 +221,7 @@ IoctlStoreDirectory(
     BOOLEAN     SquashError = FALSE;
 
     status = STATUS_INVALID_BUFFER_SIZE;
-    if (InLen == 0)
+    if (InLen == 0 || Buffer == NULL)
         goto fail1;
 
     status = STATUS_INVALID_PARAMETER;
@@ -289,7 +289,7 @@ IoctlStoreRemove(
     NTSTATUS    status;
 
     status = STATUS_INVALID_BUFFER_SIZE;
-    if (InLen == 0 || OutLen != 0)
+    if (InLen == 0 || OutLen != 0 || Buffer == NULL)
         goto fail1;
 
     status = STATUS_INVALID_PARAMETER;
@@ -392,7 +392,8 @@ IoctlStoreSetPermissions(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen < sizeof(XENIFACE_STORE_SET_PERMISSIONS_IN) ||
-        OutLen != 0) {
+        OutLen != 0 ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -508,7 +509,8 @@ IoctlStoreAddWatch(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_STORE_ADD_WATCH_IN) ||
-        OutLen != sizeof(XENIFACE_STORE_ADD_WATCH_OUT)) {
+        OutLen != sizeof(XENIFACE_STORE_ADD_WATCH_OUT) ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -648,7 +650,8 @@ IoctlStoreRemoveWatch(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_STORE_REMOVE_WATCH_IN) ||
-        OutLen != 0) {
+        OutLen != 0 ||
+        Buffer == NULL) {
         goto fail1;
     }
 
diff --git a/src/xeniface/ioctl_suspend.c b/src/xeniface/ioctl_suspend.c
index 6289a94..e7e8437 100644
--- a/src/xeniface/ioctl_suspend.c
+++ b/src/xeniface/ioctl_suspend.c
@@ -55,6 +55,9 @@ IoctlSuspendGetCount(
     if (OutLen != sizeof(ULONG))
         goto fail2;
 
+    if (Buffer == NULL)
+        goto fail3;
+
     Value = (PULONG)Buffer;
     *Value = XENBUS_SUSPEND(GetCount, &Fdo->SuspendInterface); 
     *Info = (ULONG_PTR)sizeof(ULONG);
@@ -62,6 +65,8 @@ IoctlSuspendGetCount(
 
     return status;
 
+fail3:
+    Error("Fail3\n");
 fail2:
     Error("Fail2\n");
 fail1:
@@ -87,7 +92,8 @@ IoctlSuspendRegister(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_SUSPEND_REGISTER_IN) ||
-        OutLen != sizeof(XENIFACE_SUSPEND_REGISTER_OUT)) {
+        OutLen != sizeof(XENIFACE_SUSPEND_REGISTER_OUT) ||
+        Buffer == NULL) {
         goto fail1;
     }
 
@@ -163,7 +169,8 @@ IoctlSuspendDeregister(
 
     status = STATUS_INVALID_BUFFER_SIZE;
     if (InLen != sizeof(XENIFACE_SUSPEND_REGISTER_OUT) ||
-        OutLen != 0) {
+        OutLen != 0 ||
+        Buffer == NULL) {
         goto fail1;
     }
 
diff --git a/src/xeniface/ioctls.c b/src/xeniface/ioctls.c
index a624bd1..20a7669 100644
--- a/src/xeniface/ioctls.c
+++ b/src/xeniface/ioctls.c
@@ -48,9 +48,9 @@ __CaptureUserBuffer(
     NTSTATUS Status;
     PVOID TempBuffer = NULL;
 
-    if (Length == 0) {
+    if (Length == 0 || Buffer == NULL) {
         *CapturedBuffer = NULL;
-        return STATUS_SUCCESS;
+        return STATUS_INVALID_PARAMETER;
     }
 
     Status = STATUS_NO_MEMORY;
-- 
2.32.0.windows.1

 

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.

Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.