From 93ee13245ce7fe78ffbc9694bfac3da30844296a Mon Sep 17 00:00:00 2001 From: Armin Novak Date: Mon, 10 Aug 2020 13:30:41 +0200 Subject: [PATCH] Fixed urbdrc server notification of channel close There was a recursion issue with usb device channel closing and local redirected device removal. If the local redirected device is removed due to hotplug events, the device channel needs to be closed, which in turn checks if the local device list contains the device. Ensure that the channel close code is only executed when not called from the channel side. --- .../urbdrc/client/libusb/libusb_udevice.c | 24 +++++++++++++- .../urbdrc/client/libusb/libusb_udevman.c | 31 +++++++++++++++++++ channels/urbdrc/client/urbdrc_main.c | 6 ++++ channels/urbdrc/client/urbdrc_main.h | 2 ++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/channels/urbdrc/client/libusb/libusb_udevice.c b/channels/urbdrc/client/libusb/libusb_udevice.c index 6f96e447f..bea1cba58 100644 --- a/channels/urbdrc/client/libusb/libusb_udevice.c +++ b/channels/urbdrc/client/libusb/libusb_udevice.c @@ -1088,10 +1088,28 @@ static int libusb_udev_is_already_send(IUDEVICE* idev) return (pdev->status & URBDRC_DEVICE_ALREADY_SEND) ? 1 : 0; } +/* This is called from channel cleanup code. + * Avoid double free, just remove the device and mark the channel closed. */ +static void libusb_udev_mark_channel_closed(IUDEVICE* idev) +{ + UDEVICE* pdev = (UDEVICE*)idev; + if (pdev && ((pdev->status & URBDRC_DEVICE_CHANNEL_CLOSED) == 0)) + { + URBDRC_PLUGIN* urbdrc = pdev->urbdrc; + const uint8_t busNr = idev->get_bus_number(idev); + const uint8_t devNr = idev->get_dev_number(idev); + + pdev->status |= URBDRC_DEVICE_CHANNEL_CLOSED; + urbdrc->udevman->unregister_udevice(urbdrc->udevman, busNr, devNr); + } +} + +/* This is called by local events where the device is removed or in an error + * state. Remove the device from redirection and close the channel. */ static void libusb_udev_channel_closed(IUDEVICE* idev) { UDEVICE* pdev = (UDEVICE*)idev; - if (pdev) + if (pdev && ((pdev->status & URBDRC_DEVICE_CHANNEL_CLOSED) == 0)) { URBDRC_PLUGIN* urbdrc = pdev->urbdrc; const uint8_t busNr = idev->get_bus_number(idev); @@ -1104,6 +1122,9 @@ static void libusb_udev_channel_closed(IUDEVICE* idev) pdev->status |= URBDRC_DEVICE_CHANNEL_CLOSED; + if (channel) + channel->Write(channel, 0, NULL, NULL); + urbdrc->udevman->unregister_udevice(urbdrc->udevman, busNr, devNr); } } @@ -1481,6 +1502,7 @@ static void udev_load_interface(UDEVICE* pdev) pdev->iface.isChannelClosed = libusb_udev_is_channel_closed; pdev->iface.setAlreadySend = libusb_udev_set_already_send; pdev->iface.setChannelClosed = libusb_udev_channel_closed; + pdev->iface.markChannelClosed = libusb_udev_mark_channel_closed; pdev->iface.getPath = libusb_udev_get_path; /* Transfer */ pdev->iface.isoch_transfer = libusb_udev_isoch_transfer; diff --git a/channels/urbdrc/client/libusb/libusb_udevman.c b/channels/urbdrc/client/libusb/libusb_udevman.c index 2eebc1ada..ec7b0b322 100644 --- a/channels/urbdrc/client/libusb/libusb_udevman.c +++ b/channels/urbdrc/client/libusb/libusb_udevman.c @@ -399,6 +399,36 @@ static IUDEVICE* udevman_get_udevice_by_UsbDevice(IUDEVMAN* idevman, UINT32 UsbD return NULL; } +static IUDEVICE* udevman_get_udevice_by_ChannelID(IUDEVMAN* idevman, UINT32 channelID) +{ + UDEVICE* pdev; + URBDRC_PLUGIN* urbdrc; + + if (!idevman || !idevman->plugin) + return NULL; + + /* Mask highest 2 bits, must be ignored */ + urbdrc = (URBDRC_PLUGIN*)idevman->plugin; + idevman->loading_lock(idevman); + idevman->rewind(idevman); + + while (idevman->has_next(idevman)) + { + pdev = (UDEVICE*)idevman->get_next(idevman); + + if (pdev->channelID == channelID) + { + idevman->loading_unlock(idevman); + return (IUDEVICE*)pdev; + } + } + + idevman->loading_unlock(idevman); + WLog_Print(urbdrc->log, WLOG_WARN, "Failed to find a USB device mapped to channelID=%08" PRIx32, + channelID); + return NULL; +} + static void udevman_loading_lock(IUDEVMAN* idevman) { UDEVMAN* udevman = (UDEVMAN*)idevman; @@ -786,6 +816,7 @@ static void udevman_load_interface(UDEVMAN* udevman) udevman->iface.register_udevice = udevman_register_udevice; udevman->iface.unregister_udevice = udevman_unregister_udevice; udevman->iface.get_udevice_by_UsbDevice = udevman_get_udevice_by_UsbDevice; + udevman->iface.get_udevice_by_ChannelID = udevman_get_udevice_by_ChannelID; /* Extension */ udevman->iface.isAutoAdd = udevman_is_auto_add; /* Basic state */ diff --git a/channels/urbdrc/client/urbdrc_main.c b/channels/urbdrc/client/urbdrc_main.c index 2ed6a6f2e..4638bc88f 100644 --- a/channels/urbdrc/client/urbdrc_main.c +++ b/channels/urbdrc/client/urbdrc_main.c @@ -621,6 +621,12 @@ static UINT urbdrc_on_close(IWTSVirtualChannelCallback* pChannelCallback) UINT32 control = callback->channel_mgr->GetChannelId(callback->channel); if (udevman->controlChannelId == control) udevman->status |= URBDRC_DEVICE_CHANNEL_CLOSED; + else + { /* Need to notify the local backend the device is gone */ + IUDEVICE* pdev = udevman->get_udevice_by_ChannelID(udevman, control); + if (pdev) + pdev->markChannelClosed(pdev); + } } } } diff --git a/channels/urbdrc/client/urbdrc_main.h b/channels/urbdrc/client/urbdrc_main.h index 2b643f7cb..3c8bf160a 100644 --- a/channels/urbdrc/client/urbdrc_main.h +++ b/channels/urbdrc/client/urbdrc_main.h @@ -174,6 +174,7 @@ struct _IUDEVICE void (*setAlreadySend)(IUDEVICE* idev); void (*setChannelClosed)(IUDEVICE* idev); + void (*markChannelClosed)(IUDEVICE* idev); char* (*getPath)(IUDEVICE* idev); void (*free)(IUDEVICE* idev); @@ -205,6 +206,7 @@ struct _IUDEVMAN UINT16 idProduct, UINT32 flag); IUDEVICE* (*get_next)(IUDEVMAN* idevman); IUDEVICE* (*get_udevice_by_UsbDevice)(IUDEVMAN* idevman, UINT32 UsbDevice); + IUDEVICE* (*get_udevice_by_ChannelID)(IUDEVMAN* idevman, UINT32 channelID); /* Extension */ int (*isAutoAdd)(IUDEVMAN* idevman);