joy/android_packages_modules_Connectivity
Watch
1
Fork
You've already forked android_packages_modules_Connectivity
 0
Code Issues Pull requests Projects Releases Wiki Activity

Use the binder token to reference AutoKI in alarm

Browse source 
This patch uses the binder token represented by the callback
to communicate the identity of the AutoOnOffKeepalive through
the alarm mechanism. This is a lot more robust and easy to
understand.

Addressing this, on top of the robustness and readability
advantages, corrects three bugs.

• The |obj| in the message to CMD_MONITOR_AUTOMATIC_KEEPALIVE
  is now the binder token both on the sender side and the
  receiver side, while the previous code had a Network in the
  sender while the receiver expected AutoOnOffKeepalive,
  crashing the system server with a wrong cast.
• The intent sent in the alarm would have a yet-uninitialized
  value for the slot, so it would not be possible to find the
  auto keepalive when the alarm fires.
• When the slot is reassigned, the alarm continued to fire
  with the same slot value, so the auto keepalive would no
  longer be found after a pause/resume cycle that would
  change the slot.

Test: FrameworksNetTests
Change-Id: Ibdbfcd884e0d3559206cbaae7b6b7a524972c3ca
This commit is contained in:
Chalard Jean 2023-02-03 21:26:59 +09:00 • committed by chiachangwang
commit 98732dbd6d
4 changed files with 38 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

2
framework/src/android/net/NetworkAgent.java
View file

@ -491,7 +491,7 @@ public abstract class NetworkAgent {
* TCP sockets are open over a VPN. The system will check periodically for presence of
* such open sockets, and this message is what triggers the re-evaluation.
*
* obj = AutomaticOnOffKeepaliveObject.
* obj = A Binder object associated with the keepalive.
* @hide
*/
public static final int CMD_MONITOR_AUTOMATIC_KEEPALIVE = BASE + 30;

4
service/src/com/android/server/ConnectivityService.java
View file

@ -5547,7 +5547,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
break;
}
case NetworkAgent.CMD_MONITOR_AUTOMATIC_KEEPALIVE: {
final AutomaticOnOffKeepalive ki = (AutomaticOnOffKeepalive) msg.obj;
final AutomaticOnOffKeepalive ki =
mKeepaliveTracker.getKeepaliveForBinder((IBinder) msg.obj);
if (null == ki) return; // The callback was unregistered before the alarm fired
final Network network = ki.getNetwork();
boolean networkFound = false;

46
service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java
View file

@ -45,6 +45,7 @@ import android.net.MarkMaskParcel;
import android.net.Network;
import android.net.NetworkAgent;
import android.net.SocketKeepalive.InvalidSocketException;
import android.os.Bundle;
import android.os.FileUtils;
import android.os.Handler;
import android.os.IBinder;
@ -60,6 +61,7 @@ import android.util.SparseArray;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.IndentingPrintWriter;
import com.android.modules.utils.build.SdkLevel;
import com.android.net.module.util.BinderUtils;
import com.android.net.module.util.CollectionUtils;
import com.android.net.module.util.DeviceConfigUtils;
import com.android.net.module.util.HexDump;
@ -92,8 +94,7 @@ public class AutomaticOnOffKeepaliveTracker {
private static final int[] ADDRESS_FAMILIES = new int[] {AF_INET6, AF_INET};
private static final String ACTION_TCP_POLLING_ALARM =
"com.android.server.connectivity.KeepaliveTracker.TCP_POLLING_ALARM";
private static final String EXTRA_NETWORK = "network_id";
private static final String EXTRA_SLOT = "slot";
private static final String EXTRA_BINDER_TOKEN = "token";
private static final long DEFAULT_TCP_POLLING_INTERVAL_MS = 120_000L;
private static final String AUTOMATIC_ON_OFF_KEEPALIVE_VERSION =
"automatic_on_off_keepalive_version";
@ -159,11 +160,10 @@ public class AutomaticOnOffKeepaliveTracker {
public void onReceive(Context context, Intent intent) {
if (ACTION_TCP_POLLING_ALARM.equals(intent.getAction())) {
Log.d(TAG, "Received TCP polling intent");
final Network network = intent.getParcelableExtra(EXTRA_NETWORK);
final int slot = intent.getIntExtra(EXTRA_SLOT, -1);
final IBinder token = intent.getBundleExtra(EXTRA_BINDER_TOKEN).getBinder(
EXTRA_BINDER_TOKEN);
mConnectivityServiceHandler.obtainMessage(
NetworkAgent.CMD_MONITOR_AUTOMATIC_KEEPALIVE,
slot, 0 , network).sendToTarget();
NetworkAgent.CMD_MONITOR_AUTOMATIC_KEEPALIVE, token).sendToTarget();
}
}
};
@ -183,6 +183,8 @@ public class AutomaticOnOffKeepaliveTracker {
public class AutomaticOnOffKeepalive {
@NonNull
private final KeepaliveTracker.KeepaliveInfo mKi;
@NonNull
private final ISocketKeepaliveCallback mCallback;
@Nullable
private final FileDescriptor mFd;
@Nullable
@ -193,6 +195,7 @@ public class AutomaticOnOffKeepaliveTracker {
AutomaticOnOffKeepalive(@NonNull final KeepaliveTracker.KeepaliveInfo ki,
final boolean autoOnOff, @NonNull Context context) throws InvalidSocketException {
this.mKi = Objects.requireNonNull(ki);
mCallback = ki.mCallback;
if (autoOnOff && mDependencies.isFeatureEnabled(AUTOMATIC_ON_OFF_KEEPALIVE_VERSION)) {
mAutomaticOnOffState = STATE_ENABLED;
if (null == ki.mFd) {
@ -205,8 +208,7 @@ public class AutomaticOnOffKeepaliveTracker {
Log.e(TAG, "Cannot dup fd: ", e);
throw new InvalidSocketException(ERROR_INVALID_SOCKET, e);
}
mTcpPollingAlarm = createTcpPollingAlarmIntent(
context, ki.getNai().network(), ki.getSlot());
mTcpPollingAlarm = createTcpPollingAlarmIntent(context, mCallback.asBinder());
} else {
mAutomaticOnOffState = STATE_ALWAYS_ON;
// A null fd is acceptable in KeepaliveInfo for backward compatibility of
@ -226,12 +228,14 @@ public class AutomaticOnOffKeepaliveTracker {
}
private PendingIntent createTcpPollingAlarmIntent(@NonNull Context context,
@NonNull Network network, int slot) {
@NonNull IBinder token) {
final Intent intent = new Intent(ACTION_TCP_POLLING_ALARM);
intent.putExtra(EXTRA_NETWORK, network);
intent.putExtra(EXTRA_SLOT, slot);
return PendingIntent.getBroadcast(
context, 0 /* requestCode */, intent, PendingIntent.FLAG_IMMUTABLE);
// Intent doesn't expose methods to put extra Binders, but Bundle does.
final Bundle b = new Bundle();
b.putBinder(EXTRA_BINDER_TOKEN, token);
intent.putExtra(EXTRA_BINDER_TOKEN, b);
return BinderUtils.withCleanCallingIdentity(() -> PendingIntent.getBroadcast(
context, 0 /* requestCode */, intent, PendingIntent.FLAG_IMMUTABLE));
}
}
@ -318,13 +322,14 @@ public class AutomaticOnOffKeepaliveTracker {
newKi = autoKi.mKi.withFd(autoKi.mFd);
} catch (InvalidSocketException | IllegalArgumentException | SecurityException e) {
Log.e(TAG, "Fail to construct keepalive", e);
mKeepaliveTracker.notifyErrorCallback(autoKi.mKi.mCallback, ERROR_INVALID_SOCKET);
mKeepaliveTracker.notifyErrorCallback(autoKi.mCallback, ERROR_INVALID_SOCKET);
return;
}
autoKi.mAutomaticOnOffState = STATE_ENABLED;
handleResumeKeepalive(newKi);
}
// TODO : this method should be removed ; the keepalives should always be indexed by callback
private int findAutomaticOnOffKeepaliveIndex(@NonNull Network network, int slot) {
ensureRunningOnHandlerThread();
@ -338,6 +343,7 @@ public class AutomaticOnOffKeepaliveTracker {
return -1;
}
// TODO : this method should be removed ; the keepalives should always be indexed by callback
@Nullable
private AutomaticOnOffKeepalive findAutomaticOnOffKeepalive(@NonNull Network network,
int slot) {
@ -347,6 +353,18 @@ public class AutomaticOnOffKeepaliveTracker {
return (index >= 0) ? mAutomaticOnOffKeepalives.get(index) : null;
}
/**
* Find the AutomaticOnOffKeepalive associated with a given callback.
* @return the keepalive associated with this callback, or null if none
*/
@Nullable
public AutomaticOnOffKeepalive getKeepaliveForBinder(@NonNull final IBinder token) {
ensureRunningOnHandlerThread();
return CollectionUtils.findFirst(mAutomaticOnOffKeepalives,
it -> it.mCallback.asBinder().equals(token));
}
/**
* Handle keepalive events from lower layer.
*

3
service/src/com/android/server/connectivity/KeepaliveTracker.java
View file

@ -125,8 +125,9 @@ public class KeepaliveTracker {
* which is only returned when the hardware has successfully started the keepalive.
*/
class KeepaliveInfo implements IBinder.DeathRecipient {
// Bookkeeping data.
// TODO : remove this member. Only AutoOnOffKeepalive should have a reference to this.
public final ISocketKeepaliveCallback mCallback;
// Bookkeeping data.
private final int mUid;
private final int mPid;
private final boolean mPrivileged;