[Spice-devel] [PATCH] usbclerk: add temporary driver install per session
Arnon Gilboa
agilboa at redhat.com
Wed Oct 10 02:09:46 PDT 2012
Thanks Uri, see below
Uri Lublin wrote:
> 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.
sure
> 2. Do we want to limit the number of concurrent connected clients
> (e.g. 32) ?
sure
> 3. If we keep the request type too, we can know if we need to
> uninstall-driver upon disconnect.
sure
> 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 ?
>
supporting the old USB_CLERK_DRIVER_INSTALL will become more complex,
since the client used to close the pipe right after that, and connect
with a new pipe instance for sending USB_CLERK_DRIVER_REMOVE.
> 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