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

Change-Id: I40aabdfc65d86fdd0eaac7a1e7e56e6ff69796cf
This commit is contained in:
Dave Platt
2014-02-27 16:16:20 -08:00
parent 8e53441de7
commit a6dc93df7b

View File

@@ -301,27 +301,36 @@ public final class NsdManager {
@Override @Override
public void handleMessage(Message message) { public void handleMessage(Message message) {
Object listener = getListener(message.arg2);
boolean listenerRemove = true;
switch (message.what) { switch (message.what) {
case AsyncChannel.CMD_CHANNEL_HALF_CONNECTED: case AsyncChannel.CMD_CHANNEL_HALF_CONNECTED:
mAsyncChannel.sendMessage(AsyncChannel.CMD_CHANNEL_FULL_CONNECTION); mAsyncChannel.sendMessage(AsyncChannel.CMD_CHANNEL_FULL_CONNECTION);
break; return;
case AsyncChannel.CMD_CHANNEL_FULLY_CONNECTED: case AsyncChannel.CMD_CHANNEL_FULLY_CONNECTED:
mConnected.countDown(); mConnected.countDown();
break; return;
case AsyncChannel.CMD_CHANNEL_DISCONNECTED: case AsyncChannel.CMD_CHANNEL_DISCONNECTED:
Log.e(TAG, "Channel lost"); Log.e(TAG, "Channel lost");
return;
default:
break; 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: case DISCOVER_SERVICES_STARTED:
String s = ((NsdServiceInfo) message.obj).getServiceType(); String s = getNsdServiceInfoType((NsdServiceInfo) message.obj);
((DiscoveryListener) listener).onDiscoveryStarted(s); ((DiscoveryListener) listener).onDiscoveryStarted(s);
// Keep listener until stop discovery // Keep listener until stop discovery
listenerRemove = false; listenerRemove = false;
break; break;
case DISCOVER_SERVICES_FAILED: case DISCOVER_SERVICES_FAILED:
((DiscoveryListener) listener).onStartDiscoveryFailed( ((DiscoveryListener) listener).onStartDiscoveryFailed(getNsdServiceInfoType(ns),
getNsdService(message.arg2).getServiceType(), message.arg1); message.arg1);
break; break;
case SERVICE_FOUND: case SERVICE_FOUND:
((DiscoveryListener) listener).onServiceFound((NsdServiceInfo) message.obj); ((DiscoveryListener) listener).onServiceFound((NsdServiceInfo) message.obj);
@@ -334,16 +343,14 @@ public final class NsdManager {
listenerRemove = false; listenerRemove = false;
break; break;
case STOP_DISCOVERY_FAILED: case STOP_DISCOVERY_FAILED:
((DiscoveryListener) listener).onStopDiscoveryFailed( ((DiscoveryListener) listener).onStopDiscoveryFailed(getNsdServiceInfoType(ns),
getNsdService(message.arg2).getServiceType(), message.arg1); message.arg1);
break; break;
case STOP_DISCOVERY_SUCCEEDED: case STOP_DISCOVERY_SUCCEEDED:
((DiscoveryListener) listener).onDiscoveryStopped( ((DiscoveryListener) listener).onDiscoveryStopped(getNsdServiceInfoType(ns));
getNsdService(message.arg2).getServiceType());
break; break;
case REGISTER_SERVICE_FAILED: case REGISTER_SERVICE_FAILED:
((RegistrationListener) listener).onRegistrationFailed( ((RegistrationListener) listener).onRegistrationFailed(ns, message.arg1);
getNsdService(message.arg2), message.arg1);
break; break;
case REGISTER_SERVICE_SUCCEEDED: case REGISTER_SERVICE_SUCCEEDED:
((RegistrationListener) listener).onServiceRegistered( ((RegistrationListener) listener).onServiceRegistered(
@@ -352,16 +359,13 @@ public final class NsdManager {
listenerRemove = false; listenerRemove = false;
break; break;
case UNREGISTER_SERVICE_FAILED: case UNREGISTER_SERVICE_FAILED:
((RegistrationListener) listener).onUnregistrationFailed( ((RegistrationListener) listener).onUnregistrationFailed(ns, message.arg1);
getNsdService(message.arg2), message.arg1);
break; break;
case UNREGISTER_SERVICE_SUCCEEDED: case UNREGISTER_SERVICE_SUCCEEDED:
((RegistrationListener) listener).onServiceUnregistered( ((RegistrationListener) listener).onServiceUnregistered(ns);
getNsdService(message.arg2));
break; break;
case RESOLVE_SERVICE_FAILED: case RESOLVE_SERVICE_FAILED:
((ResolveListener) listener).onResolveFailed( ((ResolveListener) listener).onResolveFailed(ns, message.arg1);
getNsdService(message.arg2), message.arg1);
break; break;
case RESOLVE_SERVICE_SUCCEEDED: case RESOLVE_SERVICE_SUCCEEDED:
((ResolveListener) listener).onServiceResolved((NsdServiceInfo) message.obj); ((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 * Initialize AsyncChannel
*/ */