diff options
Diffstat (limited to 'gitlab/issues_text/target_missing/host_missing/accel_missing/1180')
| -rw-r--r-- | gitlab/issues_text/target_missing/host_missing/accel_missing/1180 | 166 |
1 files changed, 166 insertions, 0 deletions
diff --git a/gitlab/issues_text/target_missing/host_missing/accel_missing/1180 b/gitlab/issues_text/target_missing/host_missing/accel_missing/1180 new file mode 100644 index 000000000..6a2961f27 --- /dev/null +++ b/gitlab/issues_text/target_missing/host_missing/accel_missing/1180 @@ -0,0 +1,166 @@ +Assertion failure in usb_cancel_packet() +Description of problem: +When I ran hcd-ohci with dev-storage, I found an assertion failure in +usb_cancel_packet() [1] due to p->state == USB_PACKET_COMPLETE. This is due to +the inconsistency when resetting device. + +``` c +static inline bool usb_packet_is_inflight(USBPacket *p) +{ + return (p->state == USB_PACKET_QUEUED || + p->state == USB_PACKET_ASYNC); +} + +void usb_cancel_packet(USBPacket * p) +{ + bool callback = (p->state == USB_PACKET_ASYNC); + assert(usb_packet_is_inflight(p)); // <------------------------------- [1] + usb_packet_set_state(p, USB_PACKET_CANCELED); + QTAILQ_REMOVE(&p->ep->queue, p, queue); + if (callback) { + usb_device_cancel_packet(p->ep->dev, p); + } +} +``` +Steps to reproduce: +Step 1: download the prepared rootfs and the image. + +https://drive.google.com/file/d/1B95zWWcomvZt1wms31Ddc9Xwlq-bfqhq/view?usp=sharing + +https://drive.google.com/file/d/1pxFzn49MKYmMMIIsaL9aUkzebRSYfq3J/view?usp=sharing + +Step 2: run the following script. + +``` bash +QEMU_PATH=../../../qemu/build/qemu-system-x86_64 +KERNEL_PATH=./bzImage +ROOTFS_PATH=./rootfs.ext2 +$QEMU_PATH \ + -M q35 -m 1G \ + -kernel $KERNEL_PATH \ + -drive file=$ROOTFS_PATH,if=virtio,format=raw \ + -append "root=/dev/vda console=ttyS0" \ + -net nic,model=virtio -net user \ + -usb \ + -device pci-ohci,num-ports=6 \ + -drive file=null-co://,if=none,format=raw,id=disk0 \ + -device usb-storage,port=1,drive=disk0 \ + -nographic +``` + +Step 3: with spawned shell (the user is root and the password is empty), run +`ohci-03`. +Additional information: +1 With crafted ED and TD, we can have the ohci->usb_packet's status to be +USB_RET_ASYNC [5]. And thus ohci->async_td is not NULL anymore [2]. + +``` +ed0 = { flags = 0x685f0900, tail = 0x0, head = &td0, next = 0 } + +td0 = { flags = 0x0, cbp = 0x1b8ffc, next = 0, be = 0x1b901a } +# data from cbp to be +55 53 42 43 00 00 00 00 00 00 00 00 00 00 00 03 USBC............ +e8 56 20 40 e8 56 20 40 e8 56 20 40 e8 56 20 + +ed1 = { flags = 0x08303080, tail = 0x0, head = &td1, next = 0 } + +td1 = { flags = 0x90000000, cbp = 0x19affc, next = 0, be = 0x19b01a } +# data from cbp to be +55 53 42 43 00 00 00 00 00 00 00 00 00 00 00 03 USBC............ +e8 56 20 40 e8 56 20 40 e8 56 20 40 e8 56 20 +``` + +``` c +static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed) +{ + // ... + usb_handle_packet(dev, &ohci->usb_packet); // <------------------- [4] + if (ohci->usb_packet.status == USB_RET_ASYNC) { + usb_device_flush_ep_queue(dev, ep); + ohci->async_td = addr; // <----------------------------------- [2] + return 1; + } +``` + +At the same time, the dev-storage will ref the current usb_packet +(ohci->usb_packet) [4][3]. + +``` +static void usb_msd_handle_data(USBDevice *dev, USBPacket *p) { + // ... + s->packet = p; // <----------------------------------------------- [3] + p->status = USB_RET_ASYNC; // <----------------------------------- [5] + // ... +} +``` + +2 We can first issue `MMIO_WRITE, 0xe0000054, 0x4, 0x4e33b4bf` to reset +the dev-storage device. This will mark the state of ohci->usb_packet to +USB_PACKET_COMPLETE and clear s->packet. + +``` +ohci_mem_write + ohci_port_set_status + usb_device_reset + usb_device_handle_reset + usb_msd_handle_reset + usb_msd_packet_complete + usb_packet_complete +``` + +3 We can then issue `MMIO_WRITE, 0xe0000004, 0x4, 0x3d8d323a` to reset the +roothub and this will invoke ohci_stop_endpoints() where usb_cancel_packet() +is invoked and thus [1] fails as the state of ohci->usb_packet has been changed +to USB_PACKET_COMPLETE. + +``` +ohci_set_ctl + ohci_roothub_reset + ohci_stop_endpoints + if (ohci->async_td != NULL) usb_cancel_packet(&ohci->usb_packet); + assert(usb_packet_is_inflight(p)); // boom +``` + +The above callstack are simplified. The complete callstack is in the following. + +``` +ohci_set_ctl + ohci_roothub_reset + usb_port_reset + usb_detach + ohci_detach + ohci_child_detach // <-------------------------------- [8] + usb_device_reset // <----------------------------------------- [6] + usb_device_handle_reset + usb_msd_handle_reset + usb_msd_packet_complete + usb_packet_complete + ohci_stop_endpoints // <------------------------------------------ [7] + if (ohci->async_td != NULL) usb_cancel_packet(&ohci->usb_packet); + assert(usb_packet_is_inflight(p)); // boom +``` + +Interestingly, in ohci_roothub_reset(), usb_device_reset() is also invoked [6] +just like what in step 2. I adjusted my PoC by removing step 2. However, I +cannot reproduce this assertion failure. Therefore, there is something different +bewteen [6] and step 2. + +Then, I found at [8], ohci_child_detach() cancels the ohci->usb_packet and reset +ohci->async_td. With step 2, as the status of the ohci->usb_packet has changed +to USB_PACKET_COMPLETE, usb_cancel_packet() will not be invoked. Without step 2, +as the status of the ohci->usb_packet is still USB_PACKET_ASYNC, +usb_cancel_packet() will be invoked and thus everything goes fine. + +``` +static void ohci_child_detach(USBPort *port1, USBDevice *dev) +{ + OHCIState *ohci = port1->opaque; + + if (ohci->async_td && + usb_packet_is_inflight(&ohci->usb_packet) && + ohci->usb_packet.ep->dev == dev) { + usb_cancel_packet(&ohci->usb_packet); + ohci->async_td = 0; + } +} +``` |