Skip to content

Commit dc19682

Browse files
cezannecezanne
committed
Fix bugs that vhci is unable to handle usbip commands.
vhci did not work at all due to following bugs. - wrong state management for a usbip command with 2 read IRP's (OUT direction usbip command) - interface selection problem Now my usb stick is working but does not seem to be unplugged perfectly.
1 parent 9d72a1a commit dc19682

File tree

5 files changed

+39
-41
lines changed

5 files changed

+39
-41
lines changed

‎driver/vhci/usbreq.c

+12-13
Original file line numberDiff line numberDiff line change
@@ -193,23 +193,22 @@ submit_urbr(PPDO_DEVICE_DATA pdodata, PIRP irp)
193193
if (status == STATUS_SUCCESS) {
194194
if (pdodata->len_sent_partial == 0) {
195195
pdodata->urbr_sent_partial = NULL;
196-
if (insert_pending_or_sent_urbr(pdodata, urbr, FALSE)) {
197-
InsertTailList(&pdodata->head_urbr, &urbr->list_all);
198-
status = STATUS_PENDING;
199-
pdodata->pending_read_irp = NULL;
200-
KeReleaseSpinLock(&pdodata->lock_urbr, oldirql);
201-
202-
read_irp->IoStatus.Status = STATUS_SUCCESS;
203-
IoCompleteRequest(read_irp, IO_NO_INCREMENT);
204-
}
205-
else {
206-
KeReleaseSpinLock(&pdodata->lock_urbr, oldirql);
196+
if (!insert_pending_or_sent_urbr(pdodata, urbr, FALSE))
207197
status = STATUS_CANCELLED;
208-
ExFreeToNPagedLookasideList(&g_lookaside, urbr);
209-
}
198+
}
199+
200+
if (status == STATUS_SUCCESS) {
201+
InsertTailList(&pdodata->head_urbr, &urbr->list_all);
202+
pdodata->pending_read_irp = NULL;
203+
KeReleaseSpinLock(&pdodata->lock_urbr, oldirql);
204+
205+
read_irp->IoStatus.Status = STATUS_SUCCESS;
206+
IoCompleteRequest(read_irp, IO_NO_INCREMENT);
207+
status = STATUS_PENDING;
210208
}
211209
else {
212210
KeReleaseSpinLock(&pdodata->lock_urbr, oldirql);
211+
ExFreeToNPagedLookasideList(&g_lookaside, urbr);
213212
}
214213
}
215214
else {

‎driver/vhci/vhci_devconf.c

+9-18
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
((info_intf)->NumberOfPipes - 1) * sizeof(USBD_PIPE_INFORMATION))
1111

1212
#define MAKE_PIPE(ep, type, interval) ((USBD_PIPE_HANDLE)((ep) | ((interval) << 8) | ((type) << 16)))
13+
#define TO_INTF_HANDLE(intf_num, altsetting) ((USBD_INTERFACE_HANDLE)((intf_num << 8) + altsetting))
14+
#define TO_INTF_NUM(handle) (UCHAR)(((UINT_PTR)(handle)) >> 8)
15+
#define TO_INTF_ALTSETTING(handle) (UCHAR)((UINT_PTR)(handle) & 0xff)
1316

1417
void
1518
show_pipe(unsigned int num, PUSBD_PIPE_INFORMATION pipe)
@@ -48,21 +51,6 @@ set_pipe(PUSBD_PIPE_INFORMATION pipe, PUSB_ENDPOINT_DESCRIPTOR ep_desc, unsigned
4851
pipe->PipeHandle = MAKE_PIPE(ep_desc->bEndpointAddress, pipe->PipeType, ep_desc->bInterval);
4952
}
5053

51-
static PUSB_INTERFACE_DESCRIPTOR
52-
find_dsc_intf_by_handle(PUSB_CONFIGURATION_DESCRIPTOR dsc_conf, USBD_INTERFACE_HANDLE handle)
53-
{
54-
PUSB_INTERFACE_DESCRIPTOR dsc_intf = (PUSB_INTERFACE_DESCRIPTOR)dsc_conf;
55-
unsigned int i;
56-
57-
for (i = 0; i < (UINT_PTR)handle; i++) {
58-
dsc_intf = (PUSB_INTERFACE_DESCRIPTOR)USBD_ParseDescriptors(dsc_conf, dsc_conf->wTotalLength, dsc_intf, USB_INTERFACE_DESCRIPTOR_TYPE);
59-
if (dsc_intf == NULL)
60-
return NULL;
61-
dsc_intf = NEXT_DESC_INTF(dsc_intf);
62-
}
63-
return (PUSB_INTERFACE_DESCRIPTOR)USBD_ParseDescriptors(dsc_conf, dsc_conf->wTotalLength, dsc_intf, USB_INTERFACE_DESCRIPTOR_TYPE);
64-
}
65-
6654
static NTSTATUS
6755
setup_endpoints(USBD_INTERFACE_INFORMATION *intf, PUSB_CONFIGURATION_DESCRIPTOR dsc_conf, PVOID start, UCHAR speed)
6856
{
@@ -141,7 +129,7 @@ select_config(struct _URB_SELECT_CONFIGURATION *urb_selc, UCHAR speed)
141129
if ((status = setup_intf(info_intf, dsc_conf, speed)) != STATUS_SUCCESS)
142130
return status;
143131

144-
info_intf->InterfaceHandle = (USBD_INTERFACE_HANDLE)(i + 1);
132+
info_intf->InterfaceHandle = TO_INTF_HANDLE(info_intf->InterfaceNumber, info_intf->AlternateSetting);
145133
info_intf = NEXT_USBD_INTERFACE_INFO(info_intf);
146134
}
147135

@@ -154,12 +142,15 @@ select_interface(struct _URB_SELECT_INTERFACE *urb_seli, PUSB_CONFIGURATION_DESC
154142
{
155143
PUSB_INTERFACE_DESCRIPTOR dsc_intf;
156144
PUSBD_INTERFACE_INFORMATION info_intf;
145+
UCHAR intf_num, altsetting;
157146

158147
info_intf = &urb_seli->Interface;
159148

160-
dsc_intf = find_dsc_intf_by_handle(dsc_conf, info_intf->InterfaceHandle);
149+
intf_num = TO_INTF_NUM(info_intf->InterfaceHandle);
150+
altsetting = TO_INTF_ALTSETTING(info_intf->InterfaceHandle);
151+
dsc_intf = dsc_find_intf(dsc_conf, intf_num, altsetting);
161152
if (dsc_intf == NULL) {
162-
DBGW(DBG_URB, "non-existent interface: handle: %d", (int)(UINT_PTR)info_intf->InterfaceHandle);
153+
DBGW(DBG_URB, "non-existent interface: intf_num: %hhu %hhu\n", intf_num, altsetting);
163154
return STATUS_INVALID_PARAMETER;
164155
}
165156
return setup_intf(info_intf, dsc_conf, speed);

‎driver/vhci/vhci_ioctl.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ Bus_Internal_IoCtl(__in PDEVICE_OBJECT DeviceObject, __in PIRP Irp)
134134
status = submit_urbr(pdoData, Irp);
135135
break;
136136
default:
137-
DBGE(DBG_IOCTL, "unhandled internal ioctl: %s", dbg_vhci_ioctl_code(ioctl_code));
137+
DBGE(DBG_IOCTL, "unhandled internal ioctl: %s\n", dbg_vhci_ioctl_code(ioctl_code));
138138
status = STATUS_INVALID_PARAMETER;
139139
break;
140140
}

‎driver/vhci/vhci_pdo.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,9 @@ Return Value:
531531
PAGED_CODE ();
532532

533533
UNREFERENCED_PARAMETER(DeviceData);
534+
UNREFERENCED_PARAMETER(Irp);
534535

535-
return Irp->IoStatus.Status;
536+
return STATUS_SUCCESS;
536537

537538
//
538539
// Following code shows how to provide

‎driver/vhci/vhci_read.c

+15-8
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ store_urb_get_intf_desc(PIRP irp, PURB urb, struct urb_req *urbr)
153153
}
154154

155155
static NTSTATUS
156-
store_urb_class_vendor_partial(PIRP irp, PURB urb)
156+
store_urb_class_vendor_partial(PPDO_DEVICE_DATA pdodata, PIRP irp, PURB urb)
157157
{
158158
struct _URB_CONTROL_VENDOR_OR_CLASS_REQUEST *urb_vc = &urb->UrbControlVendorClassRequest;
159159
PVOID dst;
@@ -164,6 +164,7 @@ store_urb_class_vendor_partial(PIRP irp, PURB urb)
164164

165165
RtlCopyMemory(dst, urb_vc->TransferBuffer, urb_vc->TransferBufferLength);
166166
irp->IoStatus.Information = urb_vc->TransferBufferLength;
167+
pdodata->len_sent_partial = 0;
167168

168169
return STATUS_SUCCESS;
169170
}
@@ -292,7 +293,7 @@ store_urb_select_interface(PIRP irp, PURB urb, struct urb_req *urbr)
292293
}
293294

294295
static NTSTATUS
295-
store_urb_bulk_partial(PIRP irp, PURB urb)
296+
store_urb_bulk_partial(PPDO_DEVICE_DATA pdodata, PIRP irp, PURB urb)
296297
{
297298
struct _URB_BULK_OR_INTERRUPT_TRANSFER *urb_bi = &urb->UrbBulkOrInterruptTransfer;
298299
PVOID dst, src;
@@ -306,6 +307,7 @@ store_urb_bulk_partial(PIRP irp, PURB urb)
306307
return STATUS_INSUFFICIENT_RESOURCES;
307308
RtlCopyMemory(dst, src, urb_bi->TransferBufferLength);
308309
irp->IoStatus.Information = urb_bi->TransferBufferLength;
310+
pdodata->len_sent_partial = 0;
309311

310312
return STATUS_SUCCESS;
311313
}
@@ -315,14 +317,17 @@ store_urb_bulk(PIRP irp, PURB urb, struct urb_req *urbr)
315317
{
316318
struct _URB_BULK_OR_INTERRUPT_TRANSFER *urb_bi = &urb->UrbBulkOrInterruptTransfer;
317319
struct usbip_header *hdr;
318-
int type;
319-
int in = urb_bi->TransferFlags & USBD_TRANSFER_DIRECTION_IN ? 1 : 0;
320+
int in, type;
320321

321322
hdr = get_usbip_hdr_from_read_irp(irp);
322323
if (hdr == NULL) {
323324
return STATUS_BUFFER_TOO_SMALL;
324325
}
325326

327+
/* Sometimes, direction in TransferFlags of _URB_BULK_OR_INTERRUPT_TRANSFER is not consistent with PipeHandle.
328+
* Use a direction flag in pipe handle.
329+
*/
330+
in = PIPE2DIRECT(urb_bi->PipeHandle);
326331
type = PIPE2TYPE(urb_bi->PipeHandle);
327332
if (type != USB_ENDPOINT_TYPE_BULK && type != USB_ENDPOINT_TYPE_INTERRUPT) {
328333
DBGE(DBG_READ, "Error, not a bulk pipe\n");
@@ -384,7 +389,7 @@ copy_iso_data(PVOID dst, struct _URB_ISOCH_TRANSFER *urb_iso)
384389
}
385390

386391
static NTSTATUS
387-
store_urb_iso_partial(PIRP irp, PURB urb)
392+
store_urb_iso_partial(PPDO_DEVICE_DATA pdodata, PIRP irp, PURB urb)
388393
{
389394
struct _URB_ISOCH_TRANSFER *urb_iso = &urb->UrbIsochronousTransfer;
390395
ULONG len_iso = urb_iso->TransferBufferLength + urb_iso->NumberOfPackets * sizeof(struct usbip_iso_packet_descriptor);
@@ -396,6 +401,8 @@ store_urb_iso_partial(PIRP irp, PURB urb)
396401

397402
copy_iso_data(dst, urb_iso);
398403
irp->IoStatus.Information = len_iso;
404+
pdodata->len_sent_partial = 0;
405+
399406
return STATUS_SUCCESS;
400407
}
401408

@@ -517,10 +524,10 @@ store_urbr_partial(PIRP irp, struct urb_req *urbr)
517524

518525
switch (code_func) {
519526
case URB_FUNCTION_BULK_OR_INTERRUPT_TRANSFER:
520-
status = store_urb_bulk_partial(irp, urb);
527+
status = store_urb_bulk_partial(urbr->pdodata, irp, urb);
521528
break;
522529
case URB_FUNCTION_ISOCH_TRANSFER:
523-
status = store_urb_iso_partial(irp, urb);
530+
status = store_urb_iso_partial(urbr->pdodata, irp, urb);
524531
break;
525532
case URB_FUNCTION_CLASS_DEVICE:
526533
case URB_FUNCTION_CLASS_INTERFACE:
@@ -529,7 +536,7 @@ store_urbr_partial(PIRP irp, struct urb_req *urbr)
529536
case URB_FUNCTION_VENDOR_DEVICE:
530537
case URB_FUNCTION_VENDOR_INTERFACE:
531538
case URB_FUNCTION_VENDOR_ENDPOINT:
532-
status = store_urb_class_vendor_partial(irp, urb);
539+
status = store_urb_class_vendor_partial(urbr->pdodata, irp, urb);
533540
break;
534541
default:
535542
irp->IoStatus.Information = 0;

0 commit comments

Comments
 (0)