From febcdae1ba1ac3c0e10e2bd9c3f53f8d53af8a24 Mon Sep 17 00:00:00 2001 From: Dave Platt Date: Thu, 13 Mar 2014 17:27:03 -0700 Subject: [PATCH] Harden NsdManager against null-dereference crashes Due to race conditions or programming errors, the NsdManager can attempt to process an asynchronous status message (and issue a callback to the listener) after the listener has already been removed from the NsdManager state. This causes dereferencing of null objects, and a crash. Split out the three async-queue message cases: these are ones in which message.arg2 does not hold an NsdManager array index and the code should not interpret this field as if it were. Add an explicit check for "null listener" (the array index in the message has already been released), log a warning, and exit early. Safeguard accesses to the "NSD service type" string from a possibly null) NsdServiceInfo object... return a constant "?" string rather than crashing. Bug: 9016259 Manual cherrypick of commit b1fbb14122a99c62363a949dd634294f5e887ef, change-ID I7a6ff6842cf035cefbafe2a023ae1fd43734081e in master. Change-Id: I8d9b7a1763d47d061a0f46b3cb453de4bdb8c2ed --- core/java/android/net/nsd/NsdManager.java | 47 ++++++++++++++--------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/core/java/android/net/nsd/NsdManager.java b/core/java/android/net/nsd/NsdManager.java index 9c3e405f44..7b2c623140 100644 --- a/core/java/android/net/nsd/NsdManager.java +++ b/core/java/android/net/nsd/NsdManager.java @@ -301,27 +301,36 @@ public final class NsdManager { @Override public void handleMessage(Message message) { - Object listener = getListener(message.arg2); - boolean listenerRemove = true; switch (message.what) { case AsyncChannel.CMD_CHANNEL_HALF_CONNECTED: mAsyncChannel.sendMessage(AsyncChannel.CMD_CHANNEL_FULL_CONNECTION); - break; + return; case AsyncChannel.CMD_CHANNEL_FULLY_CONNECTED: mConnected.countDown(); - break; + return; case AsyncChannel.CMD_CHANNEL_DISCONNECTED: Log.e(TAG, "Channel lost"); + return; + default: break; + } + Object listener = getListener(message.arg2); + if (listener == null) { + Log.d(TAG, "Stale key " + message.arg2); + return; + } + boolean listenerRemove = true; + NsdServiceInfo ns = getNsdService(message.arg2); + switch (message.what) { case DISCOVER_SERVICES_STARTED: - String s = ((NsdServiceInfo) message.obj).getServiceType(); + String s = getNsdServiceInfoType((NsdServiceInfo) message.obj); ((DiscoveryListener) listener).onDiscoveryStarted(s); // Keep listener until stop discovery listenerRemove = false; break; case DISCOVER_SERVICES_FAILED: - ((DiscoveryListener) listener).onStartDiscoveryFailed( - getNsdService(message.arg2).getServiceType(), message.arg1); + ((DiscoveryListener) listener).onStartDiscoveryFailed(getNsdServiceInfoType(ns), + message.arg1); break; case SERVICE_FOUND: ((DiscoveryListener) listener).onServiceFound((NsdServiceInfo) message.obj); @@ -334,16 +343,14 @@ public final class NsdManager { listenerRemove = false; break; case STOP_DISCOVERY_FAILED: - ((DiscoveryListener) listener).onStopDiscoveryFailed( - getNsdService(message.arg2).getServiceType(), message.arg1); + ((DiscoveryListener) listener).onStopDiscoveryFailed(getNsdServiceInfoType(ns), + message.arg1); break; case STOP_DISCOVERY_SUCCEEDED: - ((DiscoveryListener) listener).onDiscoveryStopped( - getNsdService(message.arg2).getServiceType()); + ((DiscoveryListener) listener).onDiscoveryStopped(getNsdServiceInfoType(ns)); break; case REGISTER_SERVICE_FAILED: - ((RegistrationListener) listener).onRegistrationFailed( - getNsdService(message.arg2), message.arg1); + ((RegistrationListener) listener).onRegistrationFailed(ns, message.arg1); break; case REGISTER_SERVICE_SUCCEEDED: ((RegistrationListener) listener).onServiceRegistered( @@ -352,16 +359,13 @@ public final class NsdManager { listenerRemove = false; break; case UNREGISTER_SERVICE_FAILED: - ((RegistrationListener) listener).onUnregistrationFailed( - getNsdService(message.arg2), message.arg1); + ((RegistrationListener) listener).onUnregistrationFailed(ns, message.arg1); break; case UNREGISTER_SERVICE_SUCCEEDED: - ((RegistrationListener) listener).onServiceUnregistered( - getNsdService(message.arg2)); + ((RegistrationListener) listener).onServiceUnregistered(ns); break; case RESOLVE_SERVICE_FAILED: - ((ResolveListener) listener).onResolveFailed( - getNsdService(message.arg2), message.arg1); + ((ResolveListener) listener).onResolveFailed(ns, message.arg1); break; case RESOLVE_SERVICE_SUCCEEDED: ((ResolveListener) listener).onServiceResolved((NsdServiceInfo) message.obj); @@ -421,6 +425,11 @@ public final class NsdManager { } + private String getNsdServiceInfoType(NsdServiceInfo s) { + if (s == null) return "?"; + return s.getServiceType(); + } + /** * Initialize AsyncChannel */