Skip to content

[transport] usb: Fix htool crash on USB disconnect #249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wak-google wants to merge 1 commit into
base: main
Choose a base branch
from
+33 −9
Open

[transport] usb: Fix htool crash on USB disconnect #249

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 33 additions & 9 deletions transports/libhoth_usb_fifo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,31 +29,52 @@

static int libhoth_usb_fifo_run_transfers(struct libhoth_usb_device* dev,
bool out, bool in) {
int status;
struct libhoth_usb_fifo* drvdata = &dev->driver_data.fifo;
drvdata->all_transfers_completed = 0;
drvdata->out_transfer_completed = !out;
drvdata->in_transfer_completed = !in;
drvdata->all_transfers_completed = 1;
drvdata->out_transfer_completed = true;
drvdata->in_transfer_completed = true;

if (in) {
int status = libusb_submit_transfer(drvdata->in_transfer);
status = libusb_submit_transfer(drvdata->in_transfer);
if (status != LIBUSB_SUCCESS) {
return status;
goto out;
}
drvdata->all_transfers_completed = 0;
drvdata->in_transfer_completed = false;
}
if (out) {
int status = libusb_submit_transfer(drvdata->out_transfer);
if (status != LIBUSB_SUCCESS) {
return status;
goto out;
}
drvdata->all_transfers_completed = 0;
drvdata->out_transfer_completed = false;
}
while (drvdata->all_transfers_completed == 0) {
int status = libusb_handle_events_completed(
dev->ctx, &drvdata->all_transfers_completed);
if (status == LIBUSB_ERROR_INTERRUPTED) {
return status;
if (status != LIBUSB_SUCCESS && status != LIBUSB_ERROR_INTERRUPTED) {
goto out;
}
}
return LIBHOTH_OK;
drvdata->in_transfer_completed = true;
drvdata->out_transfer_completed = true;
status = LIBHOTH_OK;

out:
if (!drvdata->in_transfer_completed) {
libusb_cancel_transfer(drvdata->in_transfer);
drvdata->in_transfer_completed = true;
Copy link
Copy Markdown
Collaborator

@xorptr xorptr May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be handled in fifo_transfer_callback itself? I am assuming that libusb_cancel_transfer will cause callbacks to occur with status LIBUSB_TRANSFER_CANCELLED. Marking in_transfer_completed and out_transfer_completed here might cause fifo_transfer_callback to set all_transfers_completed before the both the transfers are actually cancelled.

In the previous implementation, this is my current understanding of the flow (assuming both transfers are submitted successfully, but one of them had an issue):

  1. IN transfer has some error
  2. fifo_transfer_callback is called with transfer = drvdata->in_transfer (assumed for this example), and transfer->status = LIBUSB_TRANSFER_ERROR (assumed for this example).
  3. fifo_transfer_callback sets drvdata->in_transfer_completed = true
  4. fifo_transfer_callback calls libusb_cancel_transfer(drvdata->out_transfer)
  5. fifo_transfer_callback returns without setting all_transfers_completed to true
  6. Out transfer is cancellation processing is finished
  7. fifo_transfer_callback is called with transfer = drvdata->out_transfer (assumed for this example), and transfer->status = LIBUSB_TRANSFER_CANCELLED
  8. fifo_transfer_callback sets drvdata->out_transfer_completed = true
  9. fifo_transfer_callback sets drvdata->all_transfers_completed = 1
  10. Execution breaks out of loop with condition drvdata->all_transfers_completed

By setting both drvdata->in_transfer_completed and drvdata->out_transfer_completed just after submitting the transfer cancellation request, fifo_transfer_callback will set drvdata->all_transfers_completed = 1 even if cancellation for just one of them is finished.

}
if (!drvdata->out_transfer_completed) {
libusb_cancel_transfer(drvdata->out_transfer);
drvdata->out_transfer_completed = true;
}
while (drvdata->all_transfers_completed == 0) {
libusb_handle_events_completed(dev->ctx, &drvdata->all_transfers_completed);
}
return status;
}

static void fifo_transfer_callback(struct libusb_transfer* transfer) {
Expand Down Expand Up @@ -159,6 +180,9 @@ int libhoth_usb_fifo_open(struct libhoth_usb_device* dev,
goto err_out;
}
drvdata->prng_state = prng_seed;
drvdata->in_transfer_completed = true;
Comment thread
wak-google marked this conversation as resolved.
drvdata->out_transfer_completed = true;
drvdata->all_transfers_completed = 1;
return LIBHOTH_OK;
err_out:
if (drvdata->in_buffer != NULL) free(drvdata->in_buffer);
Expand Down
Loading