From 4b8c41ce622049486ca5753178ddf9d7f415ecf4 Mon Sep 17 00:00:00 2001 From: iflyhere <57563846+iflyhere@users.noreply.github.com> Date: Thu, 27 Aug 2026 20:28:30 +0200 Subject: [PATCH] Harden the USB adapter lifecycle Four separate ways the adapter path can take the app down or wedge it. All of them are easy to hit on a powered hub that re-enumerates the dongle, which is how a lot of ground stations are wired. 1. Deliberate null deref. WfbngLink::stop() ran a CRASH() macro (`int *i = 0; *i = 42;`) when the fd was no longer in rtl_devices. That is a recoverable state - the adapter was already gone - and it killed the process. Removed, now a warning and return. 2. NPE on openDevice(). UsbManager.openDevice() returns null when the permission was revoked or the device disappeared between the permission check and the open; getFileDescriptor() was called on it unconditionally. start() now returns false instead, WfbLinkManager reports it and leaves the adapter out of activeWifiAdapters so the next refresh retries it. Before, a failed adapter was recorded as active and never retried. 3. Leaked usbfs descriptors. UsbDeviceConnection was never closed and linkConns was never cleared, so every attach/detach cycle leaked one fd plus the map entry. 4. USB permission dialog on Android 14. requestPermission() got a PendingIntent built from an implicit Intent. Android 14 refuses to deliver those to a runtime registered receiver, so the result never arrived and the app sat on "No permission for wifi adapter(s)". setPackage() added. Also: refreshAdapters() dereferenced getAttachedAdapters() without checking for the null it returns when the device filter fails to parse, and the wfb thread name indexed split()[1] without checking the device name matched /dev/bus/usb/. --- .../openipc/pixelpilot/WfbLinkManager.java | 23 +++++++++++--- app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp | 11 ++----- .../com/openipc/wfbngrtl8812/WfbNgLink.java | 30 +++++++++++++++++-- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/com/openipc/pixelpilot/WfbLinkManager.java b/app/src/main/java/com/openipc/pixelpilot/WfbLinkManager.java index 8580be69..0c0390e0 100644 --- a/app/src/main/java/com/openipc/pixelpilot/WfbLinkManager.java +++ b/app/src/main/java/com/openipc/pixelpilot/WfbLinkManager.java @@ -120,6 +120,10 @@ public Map getAttachedAdapters() { public synchronized void refreshAdapters() { Map attachedAdapters = getAttachedAdapters(); + if (attachedAdapters == null) { + Log.e(TAG, "Could not read the usb device filter, skipping adapter refresh."); + return; + } boolean missingPermissions = false; android.hardware.usb.UsbManager usbManager = @@ -128,8 +132,13 @@ public synchronized void refreshAdapters() { if (!usbManager.hasPermission(entry.getValue())) { binding.tvMessage.setVisibility(View.VISIBLE); binding.tvMessage.setText("No permission for wifi adapter(s) " + entry.getValue().getDeviceName()); + // Android 14 refuses to deliver a PendingIntent built from an implicit + // intent to a runtime registered receiver, so the permission result never + // arrives unless the package is set explicitly. + Intent permissionIntent = new Intent(WfbLinkManager.ACTION_USB_PERMISSION); + permissionIntent.setPackage(context.getPackageName()); PendingIntent pendingIntent = PendingIntent.getBroadcast(context, 0, - new Intent(WfbLinkManager.ACTION_USB_PERMISSION), PendingIntent.FLAG_IMMUTABLE); + permissionIntent, PendingIntent.FLAG_IMMUTABLE); usbManager.requestPermission(entry.getValue(), pendingIntent); missingPermissions = true; } @@ -155,8 +164,11 @@ public synchronized void refreshAdapters() { if (activeWifiAdapters.containsKey(entry.getKey())) { continue; } - startAdapter(entry.getValue()); - activeWifiAdapters.put(entry.getKey(), entry.getValue()); + // Only track it as active if it actually came up, otherwise a failed adapter + // is never retried on the next refresh. + if (startAdapter(entry.getValue())) { + activeWifiAdapters.put(entry.getKey(), entry.getValue()); + } } if (activeWifiAdapters.isEmpty()) { @@ -204,7 +216,10 @@ public synchronized boolean startAdapter(UsbDevice dev) { String text = "Starting wfb-ng channel " + wifiChannel + " with " + String.format( "[%04X", dev.getVendorId()) + ":" + String.format("%04X]", dev.getProductId()); binding.tvMessage.setText(text); - wfbLink.start(wifiChannel, bandWidth.getValue(), dev); + if (!wfbLink.start(wifiChannel, bandWidth.getValue(), dev)) { + binding.tvMessage.setText("Could not open wifi adapter " + dev.getDeviceName()); + return false; + } return true; } } diff --git a/app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp b/app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp index e8d9c14c..a3046b07 100644 --- a/app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp +++ b/app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp @@ -34,12 +34,6 @@ #undef TAG #define TAG "pixelpilot" -#define CRASH() \ - do { \ - int *i = 0; \ - *i = 42; \ - } while (0) - std::string generate_random_string(size_t length) { const std::string characters = "abcdefghijklmnopqrstuvwxyz"; std::random_device rd; @@ -283,8 +277,9 @@ int WfbngLink::run(JNIEnv *env, jobject context, jint wifiChannel, jint bw, jint void WfbngLink::stop(JNIEnv *env, jobject context, jint fd) { if (rtl_devices.find(fd) == rtl_devices.end()) { - __android_log_print(ANDROID_LOG_ERROR, TAG, "rtl_devices.find(%d) == rtl_devices.end()", fd); - CRASH(); + // Happens when the adapter was already gone by the time the stop arrived, e.g. it + // was unplugged or the hub re-enumerated it. Nothing left to stop. + __android_log_print(ANDROID_LOG_WARN, TAG, "stop: no rtl device for fd=%d, already gone", fd); return; } auto dev = rtl_devices.at(fd).get(); diff --git a/app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java b/app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java index ea0347de..39dca17d 100644 --- a/app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java +++ b/app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java @@ -92,17 +92,35 @@ public void nativeSetUseStbc(int use) { nativeSetUseStbc(nativeWfbngLink, use); } - public synchronized void start(int wifiChannel, int bandWidth, UsbDevice usbDevice) { + public synchronized boolean start(int wifiChannel, int bandWidth, UsbDevice usbDevice) { Log.d(TAG, "wfb-ng monitoring on " + usbDevice.getDeviceName() + " using wifi channel " + wifiChannel); UsbManager usbManager = (UsbManager) context.getSystemService(Context.USB_SERVICE); + // Returns null when the permission was revoked or the device disappeared between + // the permission check and here, which is easy to hit on a re-enumerating hub. UsbDeviceConnection usbDeviceConnection = usbManager.openDevice(usbDevice); + if (usbDeviceConnection == null) { + Log.e(TAG, "Could not open " + usbDevice.getDeviceName() + " (no permission or already gone)"); + return false; + } int fd = usbDeviceConnection.getFileDescriptor(); + if (fd < 0) { + Log.e(TAG, "Invalid file descriptor for " + usbDevice.getDeviceName()); + usbDeviceConnection.close(); + return false; + } Thread t = new Thread(() -> nativeRun(nativeWfbngLink, context, wifiChannel, bandWidth, fd)); - t.setName("wfb-" + usbDevice.getDeviceName().split("/dev/bus/usb/")[1]); + t.setName(threadNameFor(usbDevice)); linkThreads.put(usbDevice, t); linkConns.put(usbDevice, usbDeviceConnection); - linkThreads.get(usbDevice).start(); + t.start(); Log.d(TAG, "wfb-ng thread on " + usbDevice.getDeviceName() + " started."); + return true; + } + + private static String threadNameFor(UsbDevice usbDevice) { + String name = usbDevice.getDeviceName(); + String[] parts = name.split("/dev/bus/usb/"); + return "wfb-" + (parts.length > 1 ? parts[1] : name); } public synchronized void stopAll() throws InterruptedException { @@ -114,9 +132,13 @@ public synchronized void stopAll() throws InterruptedException { if (t != null) { t.join(); } + // The connection holds a dup of the usbfs fd. Without close() every + // attach/detach cycle leaks one, until the process runs out. + entry.getValue().close(); Log.d(TAG, "wfb-ng thread on " + entry.getKey().getDeviceName() + " done."); } linkThreads.clear(); + linkConns.clear(); } public synchronized void stop(UsbDevice dev) throws InterruptedException { @@ -131,6 +153,8 @@ public synchronized void stop(UsbDevice dev) throws InterruptedException { t.join(); } linkThreads.remove(dev); + linkConns.remove(dev); + conn.close(); } public void SetWfbNGStatsChanged(final WfbNGStatsChanged callback) {