Skip to content

Commit

Permalink
Interrupt endpoint threads on interface altsetting change
Browse files Browse the repository at this point in the history 
Proxyied devices that switch between interface altsettings do not work
properly in certain cases: the endpoint handling threads might fail to be
joined, as they might be blocked on libusb or Raw Gadget transfers.

To interrupt Raw Gadget transfers, send the SIGUSR1 signal to the endpoint
threads via pthread_kill and add a no-op handler for this signal.

To avoid the endpoints threads being blocked forever on libusb sync I/O
functions, add a timeout of 1 second for them.

As a side-effect, this change makes the proxy print "Operation timed out"
messages every second when there's no activity on an endpoint. We can remove
them, but I think they might be valuable for getting more visibility
into what's happening.

Fixes AristoChen#13.
  • Loading branch information
@xairy
xairy committed Jun 15, 2024
1 parent 01a09c8 commit 4a48d46
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 9 deletions.
7 changes: 3 additions & 4 deletions device-libusb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ int control_request(const usb_ctrlrequest *setup_packet, int *nbytes,
}

int send_data(uint8_t endpoint, uint8_t attributes, uint8_t *dataptr,
int length) {
int length, int timeout) {
int transferred;
int attempt = 0;
int result = LIBUSB_SUCCESS;
Expand All @@ -262,7 +262,7 @@ int send_data(uint8_t endpoint, uint8_t attributes, uint8_t *dataptr,
break;
case USB_ENDPOINT_XFER_BULK:
do {
result = libusb_bulk_transfer(dev_handle, endpoint, dataptr, length, &transferred, 0);
result = libusb_bulk_transfer(dev_handle, endpoint, dataptr, length, &transferred, timeout);
//TODO retry transfer if incomplete
if (transferred != length) {
fprintf(stderr, "Incomplete Bulk transfer on EP%02x for attempt %d. length(%d), transferred(%d)\n",
Expand All @@ -284,7 +284,7 @@ int send_data(uint8_t endpoint, uint8_t attributes, uint8_t *dataptr,
&& attempt < MAX_ATTEMPTS);
break;
case USB_ENDPOINT_XFER_INT:
result = libusb_interrupt_transfer(dev_handle, endpoint, dataptr, length, &transferred, 0);
result = libusb_interrupt_transfer(dev_handle, endpoint, dataptr, length, &transferred, timeout);

if (transferred != length)
fprintf(stderr, "Incomplete Interrupt transfer on EP%02x\n", endpoint);
Expand All @@ -302,7 +302,6 @@ int send_data(uint8_t endpoint, uint8_t attributes, uint8_t *dataptr,
int receive_data(uint8_t endpoint, uint8_t attributes, uint16_t maxPacketSize,
uint8_t **dataptr, int *length, int timeout) {
int result = LIBUSB_SUCCESS;
timeout = 0;

int attempt = 0;
switch (attributes & USB_ENDPOINT_XFERTYPE_MASK) {
Expand Down
4 changes: 3 additions & 1 deletion device-libusb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include "misc.h"

#define USB_REQUEST_TIMEOUT 1000

#define MAX_ATTEMPTS 5

extern libusb_device **devs;
Expand All @@ -23,6 +25,6 @@ void set_interface_alt_setting(int interface, int altsetting);
int control_request(const usb_ctrlrequest *setup_packet, int *nbytes,
unsigned char **dataptr, int timeout);
int send_data(uint8_t endpoint, uint8_t attributes, uint8_t *dataptr,
int length);
int length, int timeout);
int receive_data(uint8_t endpoint, uint8_t attributes, uint16_t maxPacketSize,
uint8_t **dataptr, int *length, int timeout);
10 changes: 10 additions & 0 deletions host-raw-gadget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ int usb_raw_ep_read(int fd, struct usb_raw_ep_io *io) {
// Ignore failures caused by device reset.
return rv;
}
else if (errno == EINTR) {
// Ignore failures caused by sending a signal to the
// endpoint threads when shutting them down.
return rv;
}
else if (errno == EBUSY)
return rv;
perror("ioctl(USB_RAW_IOCTL_EP_READ)");
Expand All @@ -130,6 +135,11 @@ int usb_raw_ep_write(int fd, struct usb_raw_ep_io *io) {
// Ignore failures caused by device reset.
return rv;
}
else if (errno == EINTR) {
// Ignore failures caused by sending a signal to the
// endpoint threads when shutting them down.
return rv;
}
else if (errno == EBUSY)
return rv;
perror("ioctl(USB_RAW_IOCTL_EP_WRITE)");
Expand Down
34 changes: 30 additions & 4 deletions proxy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ void printData(struct usb_raw_transfer_io io, __u8 bEndpointAddress, std::string
printf("\n");
}

void noop_signal_handler(int) { }

void *ep_loop_write(void *arg) {
struct thread_info thread_info = *((struct thread_info*) arg);
int fd = thread_info.fd;
Expand All @@ -112,6 +114,10 @@ void *ep_loop_write(void *arg) {
printf("Start writing thread for EP%02x, thread id(%d)\n",
ep.bEndpointAddress, gettid());

// Set a no-op handler for SIGUSR1. Sending this signal to the thread
// will thus interrupt a blocking ioctl call without other side-effects.
signal(SIGUSR1, noop_signal_handler);

while (!please_stop_eps) {
assert(ep_num != -1);
if (data_queue->size() == 0) {
Expand All @@ -134,6 +140,11 @@ void *ep_loop_write(void *arg) {
ep.bEndpointAddress, transfer_type.c_str(), dir.c_str());
break;
}
if (rv < 0 && errno == EINTR) {
printf("EP%x(%s_%s): interface likely changing, stopping thread\n",
ep.bEndpointAddress, transfer_type.c_str(), dir.c_str());
break;
}
else if (rv < 0) {
perror("usb_raw_ep_write()");
exit(EXIT_FAILURE);
Expand All @@ -147,7 +158,7 @@ void *ep_loop_write(void *arg) {
int length = io.inner.length;
unsigned char *data = new unsigned char[length];
memcpy(data, io.data, length);
int rv = send_data(ep.bEndpointAddress, ep.bmAttributes, data, length);
int rv = send_data(ep.bEndpointAddress, ep.bmAttributes, data, length, USB_REQUEST_TIMEOUT);
if (rv == LIBUSB_ERROR_NO_DEVICE) {
printf("EP%x(%s_%s): device likely reset, stopping thread\n",
ep.bEndpointAddress, transfer_type.c_str(), dir.c_str());
Expand Down Expand Up @@ -177,6 +188,10 @@ void *ep_loop_read(void *arg) {
printf("Start reading thread for EP%02x, thread id(%d)\n",
ep.bEndpointAddress, gettid());

// Set a no-op handler for SIGUSR1. Sending this signal to the thread
// will thus interrupt a blocking ioctl call without other side-effects.
signal(SIGUSR1, noop_signal_handler);

while (!please_stop_eps) {
assert(ep_num != -1);
struct usb_raw_transfer_io io;
Expand All @@ -190,7 +205,8 @@ void *ep_loop_read(void *arg) {
continue;
}

int rv = receive_data(ep.bEndpointAddress, ep.bmAttributes, ep.wMaxPacketSize, &data, &nbytes, 0);
int rv = receive_data(ep.bEndpointAddress, ep.bmAttributes, usb_endpoint_maxp(&ep),
&data, &nbytes, USB_REQUEST_TIMEOUT);
if (rv == LIBUSB_ERROR_NO_DEVICE) {
printf("EP%x(%s_%s): device likely reset, stopping thread\n",
ep.bEndpointAddress, transfer_type.c_str(), dir.c_str());
Expand Down Expand Up @@ -319,6 +335,16 @@ void terminate_eps(int fd, int config, int interface, int altsetting) {
for (int i = 0; i < alt->interface.bNumEndpoints; i++) {
struct raw_gadget_endpoint *ep = &alt->endpoints[i];

// Endpoint threads might be blocked either on a Raw Gadget
// ioctl or on a libusb transfer handling. To interrupt the
// former, we send the SIGUSR1 signal to the threads. The
// threads have a no-op handler set for this signal, so the
// ioctl gets interrupted with no other side-effects.
// The libusb transfer handling does get interrupted directly
// and instead times out.
pthread_kill(ep->thread_read, SIGUSR1);
pthread_kill(ep->thread_write, SIGUSR1);

if (ep->thread_read && pthread_join(ep->thread_read, NULL)) {
fprintf(stderr, "Error join thread_read\n");
}
Expand Down Expand Up @@ -405,7 +431,7 @@ void ep0_loop(int fd) {

int rv = -1;
if (event.ctrl.bRequestType & USB_DIR_IN) {
result = control_request(&event.ctrl, &nbytes, &control_data, 1000);
result = control_request(&event.ctrl, &nbytes, &control_data, USB_REQUEST_TIMEOUT);
if (result == 0) {
memcpy(&io.data[0], control_data, nbytes);
io.inner.length = nbytes;
Expand Down Expand Up @@ -580,7 +606,7 @@ void ep0_loop(int fd) {
if (verbose_level >= 2)
printData(io, 0x00, "control", "out");

result = control_request(&event.ctrl, &nbytes, &control_data, 1000);
result = control_request(&event.ctrl, &nbytes, &control_data, USB_REQUEST_TIMEOUT);
if (result == 0) {
printf("ep0: transferred %d bytes (out)\n", rv);
}
Expand Down

0 comments on commit 4a48d46

Please sign in to comment.