Skip to content

Commit

Permalink
Change the cleanup process in xeniface
Browse files Browse the repository at this point in the history 
Previously cleaning up granted/mapped memory from a client process was done
through a process notification routine that's called on every process exit.
That ensured that even if the process doesn't clean up properly itself, we
get to clean up the mess. We couldn't just rely on the "device handle closed"
callback because that's only called when the last reference to the object is
closed: the classic "duplicate handle problem" (Process X opens the device,
then maps some memory through xeniface. Process Y duplicates that device
handle. Process X exits without cleaning up. Xeniface doesn't get notified
because there are still references to the device object. The memory manager
BSODs the OS because a process is exiting while still having locked pages
in its address space).

The process notify routine is very convenient (called at PASSIVE_LEVEL, in the
context of the exiting process) but has some disadvantages:
- It requires a global state in the driver because it doesn't have any
  "context" parameter.
- There is a limit of 64 notify routines in a system.
- The notify routine is called for every process creation and destruction in
  the system. Although the overhead is minimal for "not watched" processes,
  we can do better.

Now we're using pending IOCTLs for granting/mapping memory. The client
process issues a grant/map IOCTL and xeniface pends this request until
either the client issues the cleanup request or exits (all pending IOCTLs
are cancelled by the system when the owning thread exits).

This adds some non-trivial complexity to xeniface (maintaining a pending IRP
queue, unmapping user memory from not PASSIVE-guaranteed IRQL and from
arbitrary context) and xencontrol (asynchronous requests, need to keep
OVERLAPPED structures for the whole time an IOCTL is being pended) but is
ultimately the correct way to handle the "duplicate handle problem".
  • Loading branch information
@omeg
omeg committed Sep 28, 2015
1 parent 62867bc commit fa2a1d0
Show file tree
Hide file tree
Showing 14 changed files with 1,037 additions and 568 deletions.
42 changes: 42 additions & 0 deletions include/irp_queue.h
View file
@@ -0,0 +1,42 @@
#ifndef _IRP_QUEUE_H_
#define _IRP_QUEUE_H_

#include <ntddk.h>

VOID CsqInsertIrp(
_In_ PIO_CSQ Csq,
_In_ PIRP Irp
);

VOID CsqRemoveIrp(
_In_ PIO_CSQ Csq,
_In_ PIRP Irp
);

PIRP CsqPeekNextIrp(
_In_ PIO_CSQ Csq,
_In_ PIRP Irp,
_In_ PVOID PeekContext
);

_IRQL_raises_(DISPATCH_LEVEL)
_IRQL_requires_max_(DISPATCH_LEVEL)
_Acquires_lock_(CONTAINING_RECORD(Csq, XENIFACE_FDO, IrpQueue)->IrpQueueLock)
VOID CsqAcquireLock(
_In_ PIO_CSQ Csq,
_Out_ _At_(*Irql, _Post_ _IRQL_saves_) PKIRQL Irql
);

_IRQL_requires_(DISPATCH_LEVEL)
_Releases_lock_(CONTAINING_RECORD(Csq, XENIFACE_FDO, IrpQueue)->IrpQueueLock)
VOID CsqReleaseLock(
_In_ PIO_CSQ Csq,
_In_ _IRQL_restores_ KIRQL Irql
);

VOID CsqCompleteCanceledIrp(
_In_ PIO_CSQ Csq,
_In_ PIRP Irp
);

#endif
6 changes: 2 additions & 4 deletions include/xencontrol.h
View file
Expand Up @@ -89,15 +89,14 @@ DWORD GnttabGrantPages(
IN ULONG notifyOffset,
IN ULONG notifyPort,
IN GNTTAB_GRANT_PAGES_FLAGS flags,
OUT PVOID *handle,
OUT PVOID *address,
OUT ULONG *references
);

XENCONTROL_API
DWORD GnttabUngrantPages(
IN HANDLE iface,
IN PVOID handle
IN PVOID address
);

XENCONTROL_API
Expand All @@ -109,14 +108,13 @@ DWORD GnttabMapForeignPages(
IN ULONG notifyOffset,
IN ULONG notifyPort,
IN GNTTAB_GRANT_PAGES_FLAGS flags,
OUT PVOID *handle,
OUT PVOID *address
);

XENCONTROL_API
DWORD GnttabUnmapForeignPages(
IN HANDLE iface,
IN PVOID handle
IN PVOID address
);

XENCONTROL_API
Expand Down
49 changes: 33 additions & 16 deletions include/xeniface_ioctls.h
View file
Expand Up @@ -87,6 +87,7 @@ typedef struct _STORE_SET_PERMISSIONS_IN

typedef struct _STORE_ADD_WATCH_IN
{
ULONG Id;
PCHAR Path;
ULONG PathLength; // number of bytes, including the null terminator
HANDLE Event;
Expand All @@ -109,7 +110,7 @@ typedef struct _STORE_REMOVE_WATCH_IN
/* evtchn ioctls */
/************************************************************************/
#define IOCTL_XENIFACE_EVTCHN_BIND_INTERDOMAIN \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x811, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x810, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _EVTCHN_BIND_INTERDOMAIN_IN
{
Expand All @@ -125,7 +126,7 @@ typedef struct _EVTCHN_BIND_INTERDOMAIN_OUT
} EVTCHN_BIND_INTERDOMAIN_OUT, *PEVTCHN_BIND_INTERDOMAIN_OUT;

#define IOCTL_XENIFACE_EVTCHN_BIND_UNBOUND_PORT \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x812, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x811, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _EVTCHN_BIND_UNBOUND_PORT_IN
{
Expand All @@ -140,23 +141,23 @@ typedef struct _EVTCHN_BIND_UNBOUND_PORT_OUT
} EVTCHN_BIND_UNBOUND_PORT_OUT, *PEVTCHN_BIND_UNBOUND_PORT_OUT;

#define IOCTL_XENIFACE_EVTCHN_CLOSE \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x813, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x812, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _EVTCHN_CLOSE_IN
{
ULONG LocalPort;
} EVTCHN_CLOSE_IN, *PEVTCHN_CLOSE_IN;

#define IOCTL_XENIFACE_EVTCHN_NOTIFY \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x814, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x813, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _EVTCHN_NOTIFY_IN
{
ULONG LocalPort;
} EVTCHN_NOTIFY_IN, *PEVTCHN_NOTIFY_IN;

#define IOCTL_XENIFACE_EVTCHN_UNMASK \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x815, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x814, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _EVTCHN_UNMASK_IN
{
Expand All @@ -178,38 +179,47 @@ typedef enum _GNTTAB_GRANT_PAGES_FLAGS

typedef struct _GNTTAB_GRANT_PAGES_IN
{
ULONG RequestId;
USHORT RemoteDomain;
ULONG NumberPages;
GNTTAB_GRANT_PAGES_FLAGS Flags;
ULONG NotifyOffset;
ULONG NotifyPort;
} GNTTAB_GRANT_PAGES_IN, *PGNTTAB_GRANT_PAGES_IN;

#define IOCTL_XENIFACE_GNTTAB_GET_GRANTS \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x821, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _GNTTAB_GET_GRANTS_IN
{
ULONG RequestId;
} GNTTAB_GET_GRANTS_IN, *PGNTTAB_GET_GRANTS_IN;

#pragma warning(push)
#pragma warning(disable:4200) // nonstandard extension used : zero-sized array in struct/union
typedef struct _GNTTAB_GRANT_PAGES_OUT
typedef struct _GNTTAB_GET_GRANTS_OUT
{
PVOID Address;
PVOID Context;
ULONG References[0];
} GNTTAB_GRANT_PAGES_OUT, *PGNTTAB_GRANT_PAGES_OUT;
} GNTTAB_GET_GRANTS_OUT, *PGNTTAB_GET_GRANTS_OUT;
#pragma warning(pop)

#define IOCTL_XENIFACE_GNTTAB_UNGRANT_PAGES \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x821, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x822, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _GNTTAB_UNGRANT_PAGES_IN
{
PVOID Context;
ULONG RequestId;
} GNTTAB_UNGRANT_PAGES_IN, *PGNTTAB_UNGRANT_PAGES_IN;

#define IOCTL_XENIFACE_GNTTAB_MAP_FOREIGN_PAGES \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x822, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x823, METHOD_BUFFERED, FILE_ANY_ACCESS)

#pragma warning(push)
#pragma warning(disable:4200) // nonstandard extension used : zero-sized array in struct/union
typedef struct _GNTTAB_MAP_FOREIGN_PAGES_IN
{
ULONG RequestId;
USHORT RemoteDomain;
ULONG NumberPages;
GNTTAB_GRANT_PAGES_FLAGS Flags;
Expand All @@ -219,18 +229,25 @@ typedef struct _GNTTAB_MAP_FOREIGN_PAGES_IN
} GNTTAB_MAP_FOREIGN_PAGES_IN, *PGNTTAB_MAP_FOREIGN_PAGES_IN;
#pragma warning(pop)

typedef struct _GNTTAB_MAP_FOREIGN_PAGES_OUT
#define IOCTL_XENIFACE_GNTTAB_GET_MAP \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x824, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _GNTTAB_GET_MAP_IN
{
ULONG RequestId;
} GNTTAB_GET_MAP_IN, *PGNTTAB_GET_MAP_IN;

typedef struct _GNTTAB_GET_MAP_OUT
{
PVOID Address;
PVOID Context;
} GNTTAB_MAP_FOREIGN_PAGES_OUT, *PGNTTAB_MAP_FOREIGN_PAGES_OUT;
} GNTTAB_GET_MAP_OUT, *PGNTTAB_GET_MAP_OUT;

#define IOCTL_XENIFACE_GNTTAB_UNMAP_FOREIGN_PAGES \
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x823, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, 0x825, METHOD_BUFFERED, FILE_ANY_ACCESS)

typedef struct _GNTTAB_UNMAP_FOREIGN_PAGES_IN
{
PVOID Context;
ULONG RequestId;
} GNTTAB_UNMAP_FOREIGN_PAGES_IN, *PGNTTAB_UNMAP_FOREIGN_PAGES_IN;

#endif // _XENIFACE_IOCTLS_H_
24 changes: 10 additions & 14 deletions src/xencontrol-test/xencontrol-test.c
View file
Expand Up @@ -212,7 +212,7 @@ void XifLogger(XENIFACE_LOG_LEVEL level, PCHAR function, PWCHAR format, va_list

void Usage(WCHAR *exe)
{
wprintf(L"Usage:\n", exe);
wprintf(L"Usage:\n");
wprintf(L"server: %s server <remote domain id> [number of loops]\n", exe);
wprintf(L"client: %s <remote domain id> <shared page ref> [number of loops]\n", exe);
}
Expand All @@ -224,7 +224,6 @@ int __cdecl wmain(int argc, WCHAR *argv[])
EVT_CTX ctx;
SHARED_MEM *shm;
ULONG localPort;
PVOID mapHandle;
PVOID watchHandle;
CHAR storePath[256];
ULONG seed;
Expand Down Expand Up @@ -302,7 +301,6 @@ int __cdecl wmain(int argc, WCHAR *argv[])
FIELD_OFFSET(SHARED_MEM, ServerFlag),
localPort,
GNTTAB_GRANT_PAGES_USE_NOTIFY_OFFSET | GNTTAB_GRANT_PAGES_USE_NOTIFY_PORT,
&mapHandle,
&shm,
refs);
if (status != ERROR_SUCCESS)
Expand All @@ -315,7 +313,7 @@ int __cdecl wmain(int argc, WCHAR *argv[])
shm->EventPort = localPort;
shm->NumberPages = numPages;

wprintf(L"[*] grant ok, va=%p, context %p, refs: ", shm, mapHandle);
wprintf(L"[*] grant ok, va=%p, refs: ", shm);
for (i = 0; i < numPages; i++)
{
shm->References[i] = refs[i];
Expand Down Expand Up @@ -362,8 +360,8 @@ int __cdecl wmain(int argc, WCHAR *argv[])
}
ReadShm(shm);

wprintf(L"[*] ungranting address %p, context %p\n", shm, mapHandle);
status = GnttabUngrantPages(xif, mapHandle);
wprintf(L"[*] ungranting address %p\n", shm);
status = GnttabUngrantPages(xif, shm);
if (status != ERROR_SUCCESS)
{
wprintf(L"[!] GnttabUngrantPages failed: 0x%x\n", status);
Expand All @@ -383,7 +381,6 @@ int __cdecl wmain(int argc, WCHAR *argv[])
0,
0,
0,
&mapHandle,
&shm);
if (status != ERROR_SUCCESS)
{
Expand All @@ -392,8 +389,8 @@ int __cdecl wmain(int argc, WCHAR *argv[])
}

numPages = shm->NumberPages;
wprintf(L"[*] initial map ok: va=%p, context %p, remote event port %lu, remote pid %lu, %lu refs: ",
shm, mapHandle, shm->EventPort, shm->ServerPid, numPages);
wprintf(L"[*] initial map ok: va=%p, remote event port %lu, remote pid %lu, %lu refs: ",
shm, shm->EventPort, shm->ServerPid, numPages);
for (i = 0; i < numPages; i++)
{
refs[i] = shm->References[i]; // read refs for all pages and store locally since we're remapping the shared memory
Expand All @@ -419,7 +416,7 @@ int __cdecl wmain(int argc, WCHAR *argv[])
wprintf(L"[*] local event port: %lu, remapping the full region\n", localPort);

// unmap
status = GnttabUnmapForeignPages(xif, mapHandle);
status = GnttabUnmapForeignPages(xif, shm);
if (status != ERROR_SUCCESS)
{
wprintf(L"[!] GnttabUnmapForeignPages failed: 0x%x\n", status);
Expand All @@ -434,15 +431,14 @@ int __cdecl wmain(int argc, WCHAR *argv[])
FIELD_OFFSET(SHARED_MEM, ClientFlag),
localPort,
GNTTAB_GRANT_PAGES_USE_NOTIFY_OFFSET | GNTTAB_GRANT_PAGES_USE_NOTIFY_PORT,
&mapHandle,
&shm);
if (status != ERROR_SUCCESS)
{
wprintf(L"[!] GnttabMapForeignPages failed: 0x%x\n", status);
return 1;
}

wprintf(L"[*] full map ok, va=%p, context %p\n", shm, mapHandle);
wprintf(L"[*] full map ok, va=%p\n", shm);
ReadShm(shm);

// setup xenstore watch
Expand Down Expand Up @@ -480,8 +476,8 @@ int __cdecl wmain(int argc, WCHAR *argv[])
ReadShm(shm);

// final unmap
wprintf(L"[*] unmapping address %p, context %p\n", shm, mapHandle);;
status = GnttabUnmapForeignPages(xif, mapHandle);
wprintf(L"[*] unmapping address %p\n", shm);;
status = GnttabUnmapForeignPages(xif, shm);
if (status != ERROR_SUCCESS)
{
wprintf(L"[!] GnttabUnmapForeignPages failed: 0x%x\n", status);
Expand Down
16 changes: 0 additions & 16 deletions src/xencontrol/main.c
View file

This file was deleted.

0 comments on commit fa2a1d0

Please sign in to comment.