[Spice-devel] [PATCH] usbclerk: add temporary driver install per session
Uri Lublin
uril at redhat.com
Tue Oct 9 07:17:59 PDT 2012
Hi Arnon,
Thanks for this patch.
1. I think we need to remove the device from the list upon
USB_CLERK_DRIVER_REMOVE
For example to prevent the following scenario: client1
install+uninstall, client2 install, client1 disconnect.
2. Do we want to limit the number of concurrent connected clients (e.g.
32) ?
3. If we keep the request type too, we can know if we need to
uninstall-driver upon disconnect.
4. We can prevent uninstall for a device not installed by a client, by
searching for the device
to remove in appropriated devs list. Do we want to do that ?
Thanks,
Uri.
On 09/16/2012 02:01 PM, Arnon Gilboa wrote:
> -add message type USB_CLERK_DRIVER_SESSION_INSTALL, bump version to 3
> -handle multiple pipe connections concurrently with thread for each one
> -keep pipe open, and on pipe close - cleanup the session-specific devices
> -add test for temporary driver install and multiple devices
>
> rhbz#845216
> ---
> usbclerk.cpp | 101 ++++++++++++++++++++++++++++++++++-------------------
> usbclerk.h | 3 +-
> usbclerktest.cpp | 79 ++++++++++++++++++++++++++++--------------
> 3 files changed, 120 insertions(+), 63 deletions(-)
>
> diff --git a/usbclerk.cpp b/usbclerk.cpp
> index 9ce12b8..464c0cb 100644
> --- a/usbclerk.cpp
> +++ b/usbclerk.cpp
> @@ -4,6 +4,7 @@
> #include<stdio.h>
> #include<string.h>
> #include<tchar.h>
> +#include<list>
> #include "usbclerk.h"
> #include "usbredirfilter.h"
> #include "libwdi.h"
> @@ -26,6 +27,13 @@
> #define MAX_DEVICE_HCID_LEN 1024
> #define MAX_DEVICE_FILTER_LEN 1024
>
> +typedef struct USBDev {
> + UINT16 vid;
> + UINT16 pid;
> +} USBDev;
> +
> +typedef std::list<USBDev> USBDevs;
> +
> class USBClerk {
> public:
> static USBClerk* get();
> @@ -37,7 +45,7 @@ public:
> private:
> USBClerk();
> bool execute();
> - bool dispatch_message(CHAR *buffer, DWORD bytes, USBClerkReply *reply);
> + bool dispatch_message(CHAR *buffer, DWORD bytes, USBClerkReply *reply, USBDevs *devs);
> bool install_winusb_driver(int vid, int pid);
> bool remove_winusb_driver(int vid, int pid);
> bool uninstall_inf(HDEVINFO devs, PSP_DEVINFO_DATA dev_info);
> @@ -51,6 +59,7 @@ private:
> bool dev_filter_check(int vid, int pid, bool *has_winusb);
> static DWORD WINAPI control_handler(DWORD control, DWORD event_type,
> LPVOID event_data, LPVOID context);
> + static DWORD WINAPI pipe_thread(LPVOID param);
> static VOID WINAPI main(DWORD argc, TCHAR * argv[]);
>
> private:
> @@ -60,7 +69,6 @@ private:
> struct usbredirfilter_rule *_filter_rules;
> int _filter_count;
> char _wdi_path[MAX_PATH];
> - HANDLE _pipe;
> bool _running;
> VDLog* _log;
> };
> @@ -276,11 +284,9 @@ bool USBClerk::execute()
> {
> SECURITY_ATTRIBUTES sec_attr;
> SECURITY_DESCRIPTOR* sec_desr;
> - USBClerkReply reply = {{USB_CLERK_MAGIC, USB_CLERK_VERSION,
> - USB_CLERK_REPLY, sizeof(USBClerkReply)}};
> CHAR filter_str[MAX_DEVICE_FILTER_LEN];
> - CHAR buffer[USB_CLERK_PIPE_BUF_SIZE];
> - DWORD bytes;
> + HANDLE pipe, thread;
> + DWORD tid;
> HKEY hkey;
> LONG ret;
>
> @@ -296,14 +302,6 @@ bool USBClerk::execute()
> sec_attr.nLength = sizeof(sec_attr);
> sec_attr.bInheritHandle = TRUE;
> sec_attr.lpSecurityDescriptor = sec_desr;
> - _pipe = CreateNamedPipe(USB_CLERK_PIPE_NAME, PIPE_ACCESS_DUPLEX |
> - FILE_FLAG_FIRST_PIPE_INSTANCE,
> - PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT, 1,
> - USB_CLERK_PIPE_BUF_SIZE, USB_CLERK_PIPE_BUF_SIZE, 0,&sec_attr);
> - if (_pipe == INVALID_HANDLE_VALUE) {
> - vd_printf("CreatePipe() failed: %u", GetLastError());
> - return false;
> - }
>
> /* Read filter rules from registry */
> ret = RegOpenKeyEx(HKEY_LOCAL_MACHINE, L"Software\\USBClerk", 0, KEY_READ,&hkey);
> @@ -323,34 +321,60 @@ bool USBClerk::execute()
> RegCloseKey(hkey);
> }
> while (_running) {
> - if (!ConnectNamedPipe(_pipe, NULL)&& GetLastError() != ERROR_PIPE_CONNECTED) {
> - vd_printf("ConnectNamedPipe() failed: %u", GetLastError());
> + pipe = CreateNamedPipe(USB_CLERK_PIPE_NAME, PIPE_ACCESS_DUPLEX,
> + PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
> + PIPE_UNLIMITED_INSTANCES, USB_CLERK_PIPE_BUF_SIZE,
> + USB_CLERK_PIPE_BUF_SIZE, 0,&sec_attr);
> + if (pipe == INVALID_HANDLE_VALUE) {
> + vd_printf("CreatePipe() failed: %u", GetLastError());
> break;
> }
> - if (!ReadFile(_pipe,&buffer, sizeof(buffer),&bytes, NULL)) {
> - vd_printf("ReadFile() failed: %d", GetLastError());
> - goto disconnect;
> - }
> - if (!dispatch_message(buffer, bytes,&reply)) {
> - goto disconnect;
> + if (!ConnectNamedPipe(pipe, NULL)&& GetLastError() != ERROR_PIPE_CONNECTED) {
> + vd_printf("ConnectNamedPipe() failed: %u", GetLastError());
> + CloseHandle(pipe);
> + break;
> }
> - if (!WriteFile(_pipe,&reply, sizeof(reply),&bytes, NULL)) {
> - vd_printf("WriteFile() failed: %d", GetLastError());
> - goto disconnect;
> + thread = CreateThread(NULL, 0, pipe_thread, (LPVOID)pipe, 0,&tid);
> + if (thread == NULL) {
> + vd_printf("CreateThread() failed: %u", GetLastError());
> + break;
> }
> - FlushFileBuffers(_pipe);
> -disconnect:
> - DisconnectNamedPipe(_pipe);
> + CloseHandle(thread);
> }
> free(_filter_rules);
> - CloseHandle(_pipe);
> return true;
> }
>
> -bool USBClerk::dispatch_message(CHAR *buffer, DWORD bytes, USBClerkReply *reply)
> +DWORD WINAPI USBClerk::pipe_thread(LPVOID param)
> +{
> + USBClerkReply reply = {{USB_CLERK_MAGIC, USB_CLERK_VERSION,
> + USB_CLERK_REPLY, sizeof(USBClerkReply)}};
> + CHAR buffer[USB_CLERK_PIPE_BUF_SIZE];
> + HANDLE pipe = (HANDLE)param;
> + USBClerk* usbclerk = get();
> + USBDevs devs;
> + DWORD bytes;
> +
> + while (usbclerk->_running) {
> + if (!ReadFile(pipe,&buffer, sizeof(buffer),&bytes, NULL) ||
> + !usbclerk->dispatch_message(buffer, bytes,&reply,&devs) ||
> + !WriteFile(pipe,&reply, sizeof(reply),&bytes, NULL)) {
> + break;
> + }
> + FlushFileBuffers(pipe);
> + }
> + DisconnectNamedPipe(pipe);
> + CloseHandle(pipe);
> + for (USBDevs::iterator dev = devs.begin(); dev != devs.end(); dev++) {
> + usbclerk->remove_winusb_driver(dev->vid, dev->pid);
> + }
> + return 0;
> +}
> +
> +bool USBClerk::dispatch_message(CHAR *buffer, DWORD bytes, USBClerkReply *reply, USBDevs *devs)
> {
> USBClerkHeader *hdr = (USBClerkHeader *)buffer;
> - USBClerkDriverOp *dev;
> + USBClerkDriverOp *op;
>
> if (hdr->magic != USB_CLERK_MAGIC) {
> vd_printf("Bad message received, magic %u", hdr->magic);
> @@ -360,15 +384,20 @@ bool USBClerk::dispatch_message(CHAR *buffer, DWORD bytes, USBClerkReply *reply)
> vd_printf("Wrong mesage size %u type %u", hdr->size, hdr->type);
> return false;
> }
> - dev = (USBClerkDriverOp *)buffer;
> + op = (USBClerkDriverOp *)buffer;
> + USBDev dev = {op->vid, op->pid};
> switch (hdr->type) {
> + case USB_CLERK_DRIVER_SESSION_INSTALL:
> case USB_CLERK_DRIVER_INSTALL:
> - vd_printf("Installing winusb driver for %04x:%04x", dev->vid, dev->pid);
> - reply->status = install_winusb_driver(dev->vid, dev->pid);
> + vd_printf("Installing winusb driver for %04x:%04x", dev.vid, dev.pid);
> + reply->status = install_winusb_driver(dev.vid, dev.pid);
> + if (hdr->type == USB_CLERK_DRIVER_SESSION_INSTALL&& reply->status) {
> + devs->push_back(dev);
> + }
> break;
> case USB_CLERK_DRIVER_REMOVE:
> - vd_printf("Removing winusb driver for %04x:%04x", dev->vid, dev->pid);
> - reply->status = remove_winusb_driver(dev->vid, dev->pid);
> + vd_printf("Removing winusb driver for %04x:%04x", dev.vid, dev.pid);
> + reply->status = remove_winusb_driver(dev.vid, dev.pid);
> break;
> default:
> vd_printf("Unknown message received, type %u", hdr->type);
> diff --git a/usbclerk.h b/usbclerk.h
> index 5b1e3cf..24da3b4 100644
> --- a/usbclerk.h
> +++ b/usbclerk.h
> @@ -5,7 +5,7 @@
>
> #define USB_CLERK_PIPE_NAME TEXT("\\\\.\\pipe\\usbclerkpipe")
> #define USB_CLERK_MAGIC 0xDADA
> -#define USB_CLERK_VERSION 0x0002
> +#define USB_CLERK_VERSION 0x0003
>
> typedef struct USBClerkHeader {
> UINT16 magic;
> @@ -18,6 +18,7 @@ enum {
> USB_CLERK_DRIVER_INSTALL = 1,
> USB_CLERK_DRIVER_REMOVE,
> USB_CLERK_REPLY,
> + USB_CLERK_DRIVER_SESSION_INSTALL,
> USB_CLERK_END_MESSAGE,
> };
>
> diff --git a/usbclerktest.cpp b/usbclerktest.cpp
> index 712fe96..efaf3df 100644
> --- a/usbclerktest.cpp
> +++ b/usbclerktest.cpp
> @@ -1,19 +1,35 @@
> #include<stdio.h>
> +#include<conio.h>
> #include<tchar.h>
> #include "usbclerk.h"
>
> int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
> {
> HANDLE pipe;
> - USBClerkDriverOp dev = {{USB_CLERK_MAGIC, USB_CLERK_VERSION, 0, sizeof(USBClerkDriverOp)}};
> + USBClerkDriverOp dev = {{USB_CLERK_MAGIC, USB_CLERK_VERSION,
> + USB_CLERK_DRIVER_INSTALL, sizeof(USBClerkDriverOp)}};
> USBClerkReply reply;
> DWORD pipe_mode;
> DWORD bytes = 0;
> - bool do_remove = false;
> + bool err = false;
> + int i, devs = 0;
>
> - if (argc< 2 || swscanf_s(argv[argc - 1], L"%hx:%hx",&dev.vid,&dev.pid)< 2 ||
> - (argc == 3&& !(do_remove = !wcscmp(argv[1], L"/u")))) {
> - printf("Usage: usbclerktest [/u] vid:pid\n/u - uninstall driver\nvid:pid in hex\n");
> + for (i = 1; i< argc&& !err; i++) {
> + if (wcscmp(argv[i], L"/t") == 0) {
> + dev.hdr.type = USB_CLERK_DRIVER_SESSION_INSTALL;
> + } else if (wcscmp(argv[i], L"/u") == 0) {
> + dev.hdr.type = USB_CLERK_DRIVER_REMOVE;
> + } else if (swscanf_s(argv[i], L"%hx:%hx",&dev.vid,&dev.pid) == 2) {
> + devs++;
> + } else {
> + err = true;
> + }
> + }
> + if (argc< 2 || err || devs< argc - 2) {
> + printf("Usage: usbclerktest [/t][/u] vid:pid [vid1:pid1...]\n"
> + "default - install driver for device vid:pid (in hex)\n"
> + "/t - temporary install until session terminated\n"
> + "/u - uninstall driver\n");
> return 1;
> }
> pipe = CreateFile(USB_CLERK_PIPE_NAME, GENERIC_READ | GENERIC_WRITE,
> @@ -27,29 +43,40 @@ int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
> printf("SetNamedPipeHandleState() failed: %d\n", GetLastError());
> return 1;
> }
> - if (do_remove) {
> - printf("Removing %04x:%04x\n", dev.vid, dev.pid);
> - dev.hdr.type = USB_CLERK_DRIVER_REMOVE;
> - } else {
> - printf("Signing& installing %04x:%04x\n", dev.vid, dev.pid);
> - dev.hdr.type = USB_CLERK_DRIVER_INSTALL;
> +
> + for (i = 1; i< argc&& !err; i++) {
> + if (swscanf_s(argv[i], L"%hx:%hx",&dev.vid,&dev.pid)< 2) continue;
> + switch (dev.hdr.type) {
> + case USB_CLERK_DRIVER_SESSION_INSTALL:
> + case USB_CLERK_DRIVER_INSTALL:
> + printf("Signing& installing %04x:%04x...", dev.vid, dev.pid);
> + break;
> + case USB_CLERK_DRIVER_REMOVE:
> + printf("Removing %04x:%04x...", dev.vid, dev.pid);
> + break;
> + }
> + if (!TransactNamedPipe(pipe,&dev, sizeof(dev),&reply, sizeof(reply),&bytes, NULL)) {
> + printf("TransactNamedPipe() failed: %d\n", GetLastError());
> + CloseHandle(pipe);
> + return 1;
> + }
> + if (reply.hdr.magic != USB_CLERK_MAGIC || reply.hdr.type != USB_CLERK_REPLY ||
> + reply.hdr.size != sizeof(USBClerkReply)) {
> + printf("Unknown message received, magic 0x%x type %u size %u\n",
> + reply.hdr.magic, reply.hdr.type, reply.hdr.size);
> + return 1;
> + }
> + if (reply.status) {
> + printf("Completed successfully\n");
> + } else {
> + printf("Failed\n");
> + }
> }
> - if (!TransactNamedPipe(pipe,&dev, sizeof(dev),&reply, sizeof(reply),&bytes, NULL)) {
> - printf("TransactNamedPipe() failed: %d\n", GetLastError());
> - CloseHandle(pipe);
> - return 1;
> +
> + if (dev.hdr.type == USB_CLERK_DRIVER_SESSION_INSTALL) {
> + printf("Hit any key to terminate session\n");
> + _getch();
> }
> CloseHandle(pipe);
> - if (reply.hdr.magic != USB_CLERK_MAGIC || reply.hdr.type != USB_CLERK_REPLY ||
> - reply.hdr.size != sizeof(USBClerkReply)) {
> - printf("Unknown message received, magic 0x%x type %u size %u\n",
> - reply.hdr.magic, reply.hdr.type, reply.hdr.size);
> - return 1;
> - }
> - if (reply.status) {
> - printf("Completed successfully\n");
> - } else {
> - printf("Failed\n");
> - }
> return 0;
> }
More information about the Spice-devel
mailing list