diff options
4 files changed, 178 insertions, 25 deletions
diff --git a/services/core/java/com/android/server/PackageWatchdog.java b/services/core/java/com/android/server/PackageWatchdog.java index 32671144aee3..dee89e5d5c4d 100644 --- a/services/core/java/com/android/server/PackageWatchdog.java +++ b/services/core/java/com/android/server/PackageWatchdog.java @@ -25,6 +25,7 @@ import android.annotation.Nullable; import android.content.Context; import android.content.pm.PackageManager; import android.content.pm.VersionedPackage; +import android.net.NetworkStackClient; import android.os.Environment; import android.os.Handler; import android.os.Looper; @@ -115,6 +116,7 @@ public class PackageWatchdog { // File containing the XML data of monitored packages /data/system/package-watchdog.xml private final AtomicFile mPolicyFile; private final ExplicitHealthCheckController mHealthCheckController; + private final NetworkStackClient mNetworkStackClient; @GuardedBy("mLock") private boolean mIsPackagesReady; // Flag to control whether explicit health checks are supported or not @@ -135,7 +137,8 @@ public class PackageWatchdog { new File(new File(Environment.getDataDirectory(), "system"), "package-watchdog.xml")), new Handler(Looper.myLooper()), BackgroundThread.getHandler(), - new ExplicitHealthCheckController(context)); + new ExplicitHealthCheckController(context), + NetworkStackClient.getInstance()); } /** @@ -143,12 +146,14 @@ public class PackageWatchdog { */ @VisibleForTesting PackageWatchdog(Context context, AtomicFile policyFile, Handler shortTaskHandler, - Handler longTaskHandler, ExplicitHealthCheckController controller) { + Handler longTaskHandler, ExplicitHealthCheckController controller, + NetworkStackClient networkStackClient) { mContext = context; mPolicyFile = policyFile; mShortTaskHandler = shortTaskHandler; mLongTaskHandler = longTaskHandler; mHealthCheckController = controller; + mNetworkStackClient = networkStackClient; loadFromFile(); } @@ -174,6 +179,7 @@ public class PackageWatchdog { () -> syncRequestsAsync()); setPropertyChangedListenerLocked(); updateConfigs(); + registerNetworkStackHealthListener(); } } @@ -630,29 +636,40 @@ public class PackageWatchdog { synchronized (mLock) { PackageHealthObserver registeredObserver = observer.mRegisteredObserver; if (registeredObserver != null) { - PackageManager pm = mContext.getPackageManager(); Iterator<MonitoredPackage> it = failedPackages.iterator(); while (it.hasNext()) { String failedPackage = it.next().getName(); - long versionCode = 0; Slog.i(TAG, "Explicit health check failed for package " + failedPackage); - try { - versionCode = pm.getPackageInfo( - failedPackage, 0 /* flags */).getLongVersionCode(); - } catch (PackageManager.NameNotFoundException e) { + VersionedPackage versionedPkg = getVersionedPackage(failedPackage); + if (versionedPkg == null) { Slog.w(TAG, "Explicit health check failed but could not find package " + failedPackage); // TODO(b/120598832): Skip. We only continue to pass tests for now since // the tests don't install any packages + versionedPkg = new VersionedPackage(failedPackage, 0L); } - registeredObserver.execute( - new VersionedPackage(failedPackage, versionCode)); + registeredObserver.execute(versionedPkg); } } } }); } + @Nullable + private VersionedPackage getVersionedPackage(String packageName) { + final PackageManager pm = mContext.getPackageManager(); + if (pm == null) { + return null; + } + try { + final long versionCode = pm.getPackageInfo( + packageName, 0 /* flags */).getLongVersionCode(); + return new VersionedPackage(packageName, versionCode); + } catch (PackageManager.NameNotFoundException e) { + return null; + } + } + /** * Loads mAllObservers from file. * @@ -726,6 +743,27 @@ public class PackageWatchdog { } } + private void registerNetworkStackHealthListener() { + // TODO: have an internal method to trigger a rollback by reporting high severity errors, + // and rely on ActivityManager to inform the watchdog of severe network stack crashes + // instead of having this listener in parallel. + mNetworkStackClient.registerHealthListener( + packageName -> { + final VersionedPackage pkg = getVersionedPackage(packageName); + if (pkg == null) { + Slog.wtf(TAG, "NetworkStack failed but could not find its package"); + return; + } + // This is a severe failure and recovery should be attempted immediately. + // TODO: have a better way to handle such failures. + final List<VersionedPackage> pkgList = Collections.singletonList(pkg); + final long failureCount = getTriggerFailureCount(); + for (int i = 0; i < failureCount; i++) { + onPackageFailure(pkgList); + } + }); + } + /** * Persists mAllObservers to file. Threshold information is ignored. */ diff --git a/services/net/java/android/net/NetworkStackClient.java b/services/net/java/android/net/NetworkStackClient.java index 6b5842ff9065..09c9b6d360a9 100644 --- a/services/net/java/android/net/NetworkStackClient.java +++ b/services/net/java/android/net/NetworkStackClient.java @@ -26,22 +26,27 @@ import android.content.ComponentName; import android.content.Context; import android.content.Intent; import android.content.ServiceConnection; +import android.content.SharedPreferences; import android.content.pm.PackageManager; import android.net.dhcp.DhcpServingParamsParcel; import android.net.dhcp.IDhcpServerCallbacks; import android.net.ip.IIpClientCallbacks; import android.net.util.SharedLog; import android.os.Binder; -import android.os.Build; +import android.os.Environment; import android.os.IBinder; import android.os.Process; import android.os.RemoteException; import android.os.ServiceManager; +import android.os.SystemClock; import android.os.UserHandle; +import android.provider.DeviceConfig; +import android.util.ArraySet; import android.util.Slog; import com.android.internal.annotations.GuardedBy; +import java.io.File; import java.io.PrintWriter; import java.util.ArrayList; @@ -54,6 +59,15 @@ public class NetworkStackClient { private static final int NETWORKSTACK_TIMEOUT_MS = 10_000; private static final String IN_PROCESS_SUFFIX = ".InProcess"; + private static final String PREFS_FILE = "NetworkStackClientPrefs.xml"; + private static final String PREF_KEY_LAST_CRASH_UPTIME = "lastcrash"; + private static final String CONFIG_MIN_CRASH_INTERVAL_MS = "min_crash_interval"; + + // Even if the network stack is lost, do not crash the system more often than this. + // Connectivity would be broken, but if the user needs the device for something urgent + // (like calling emergency services) we should not bootloop the device. + // This is the default value: the actual value can be adjusted via device config. + private static final long DEFAULT_MIN_CRASH_INTERVAL_MS = 6 * 3_600_000L; private static NetworkStackClient sInstance; @@ -67,12 +81,34 @@ public class NetworkStackClient { @GuardedBy("mLog") private final SharedLog mLog = new SharedLog(TAG); - private volatile boolean mNetworkStackStartRequested = false; + private volatile boolean mWasSystemServerInitialized = false; + + /** + * If non-zero, indicates that the last framework start happened after a crash of the + * NetworkStack which was at the specified uptime. + */ + private volatile long mLastCrashUptime = 0L; + + @GuardedBy("mHealthListeners") + private final ArraySet<NetworkStackHealthListener> mHealthListeners = new ArraySet<>(); private interface NetworkStackCallback { void onNetworkStackConnected(INetworkStackConnector connector); } + /** + * Callback interface for severe failures of the NetworkStack. + * + * <p>Useful for health monitors such as PackageWatchdog. + */ + public interface NetworkStackHealthListener { + /** + * Called when there is a severe failure of the network stack. + * @param packageName Package name of the network stack. + */ + void onNetworkStackFailure(@NonNull String packageName); + } + private NetworkStackClient() { } /** @@ -86,6 +122,15 @@ public class NetworkStackClient { } /** + * Add a {@link NetworkStackHealthListener} to listen to network stack health events. + */ + public void registerHealthListener(@NonNull NetworkStackHealthListener listener) { + synchronized (mHealthListeners) { + mHealthListeners.add(listener); + } + } + + /** * Create a DHCP server according to the specified parameters. * * <p>The server will be returned asynchronously through the provided callbacks. @@ -147,6 +192,16 @@ public class NetworkStackClient { } private class NetworkStackConnection implements ServiceConnection { + @NonNull + private final Context mContext; + @NonNull + private final String mPackageName; + + private NetworkStackConnection(@NonNull Context context, @NonNull String packageName) { + mContext = context; + mPackageName = packageName; + } + @Override public void onServiceConnected(ComponentName name, IBinder service) { logi("Network stack service connected"); @@ -155,14 +210,14 @@ public class NetworkStackClient { @Override public void onServiceDisconnected(ComponentName name) { - // The system has lost its network stack (probably due to a crash in the - // network stack process): better crash rather than stay in a bad state where all - // networking is broken. // onServiceDisconnected is not being called on device shutdown, so this method being // called always indicates a bad state for the system server. - maybeCrashWithTerribleFailure("Lost network stack"); + // This code path is only run by the system server: only the system server binds + // to the NetworkStack as a service. Other processes get the NetworkStack from + // the ServiceManager. + maybeCrashWithTerribleFailure("Lost network stack", mContext, mPackageName); } - }; + } private void registerNetworkStackService(@NonNull IBinder service) { final INetworkStackConnector connector = INetworkStackConnector.Stub.asInterface(service); @@ -189,7 +244,7 @@ public class NetworkStackClient { */ public void init() { log("Network stack init"); - mNetworkStackStartRequested = true; + mWasSystemServerInitialized = true; } /** @@ -202,6 +257,13 @@ public class NetworkStackClient { */ public void start(Context context) { log("Starting network stack"); + + final SharedPreferences prefs = getSharedPreferences(context); + mLastCrashUptime = prefs.getLong(PREF_KEY_LAST_CRASH_UPTIME, 0L); + // Remove the preference after getting the last crash uptime, so mLastCrashUptime always + // indicates this is the first start since the last crash. + prefs.edit().remove(PREF_KEY_LAST_CRASH_UPTIME).commit(); + final PackageManager pm = context.getPackageManager(); // Try to bind in-process if the device was shipped with an in-process version @@ -216,16 +278,19 @@ public class NetworkStackClient { } if (intent == null) { - maybeCrashWithTerribleFailure("Could not resolve the network stack"); + maybeCrashWithTerribleFailure("Could not resolve the network stack", context, null); return; } + final String packageName = intent.getComponent().getPackageName(); + // Start the network stack. The service will be added to the service manager in // NetworkStackConnection.onServiceConnected(). - if (!context.bindServiceAsUser(intent, new NetworkStackConnection(), + if (!context.bindServiceAsUser(intent, new NetworkStackConnection(context, packageName), Context.BIND_AUTO_CREATE | Context.BIND_IMPORTANT, UserHandle.SYSTEM)) { maybeCrashWithTerribleFailure( - "Could not bind to network stack in-process, or in app with " + intent); + "Could not bind to network stack in-process, or in app with " + intent, + context, packageName); return; } @@ -274,11 +339,47 @@ public class NetworkStackClient { } } - private void maybeCrashWithTerribleFailure(@NonNull String message) { + private void maybeCrashWithTerribleFailure(@NonNull String message, + @NonNull Context context, @Nullable String packageName) { logWtf(message, null); - if (Build.IS_DEBUGGABLE) { + // uptime is monotonic even after a framework restart + final long uptime = SystemClock.elapsedRealtime(); + final long minCrashIntervalMs = DeviceConfig.getLong(DeviceConfig.NAMESPACE_CONNECTIVITY, + CONFIG_MIN_CRASH_INTERVAL_MS, DEFAULT_MIN_CRASH_INTERVAL_MS); + + // Either the framework was not restarted after a crash of the NetworkStack, or the min + // crash interval has passed since then. + if (mLastCrashUptime == 0L || uptime - mLastCrashUptime > minCrashIntervalMs) { + // The system is not bound to its network stack (for example due to a crash in the + // network stack process): better crash rather than stay in a bad state where all + // networking is broken. + // Using device-encrypted SharedPreferences as DeviceConfig does not have a synchronous + // API to persist settings before a crash. + final SharedPreferences prefs = getSharedPreferences(context); + if (!prefs.edit().putLong(PREF_KEY_LAST_CRASH_UPTIME, uptime).commit()) { + logWtf("Could not persist last crash uptime", null); + } throw new IllegalStateException(message); } + + // Here the system crashed recently already. Inform listeners that something is + // definitely wrong. + if (packageName != null) { + final ArraySet<NetworkStackHealthListener> listeners; + synchronized (mHealthListeners) { + listeners = new ArraySet<>(mHealthListeners); + } + for (NetworkStackHealthListener listener : listeners) { + listener.onNetworkStackFailure(packageName); + } + } + } + + private SharedPreferences getSharedPreferences(Context context) { + final File prefsFile = new File( + Environment.getDataSystemDeDirectory(UserHandle.USER_SYSTEM), PREFS_FILE); + return context.createDeviceProtectedStorageContext() + .getSharedPreferences(prefsFile, Context.MODE_PRIVATE); } /** @@ -350,7 +451,7 @@ public class NetworkStackClient { "Only the system server should try to bind to the network stack."); } - if (!mNetworkStackStartRequested) { + if (!mWasSystemServerInitialized) { // The network stack is not being started in this process, e.g. this process is not // the system server. Get a remote connector registered by the system server. final INetworkStackConnector connector = getRemoteConnector(); diff --git a/tests/PackageWatchdog/Android.bp b/tests/PackageWatchdog/Android.bp index b07996568839..88d92c4d94fe 100644 --- a/tests/PackageWatchdog/Android.bp +++ b/tests/PackageWatchdog/Android.bp @@ -18,11 +18,18 @@ android_test { srcs: ["src/**/*.java"], static_libs: [ "junit", + "mockito-target-extended-minus-junit4", "frameworks-base-testutils", "androidx.test.rules", "services.core", + "services.net", ], libs: ["android.test.runner"], + jni_libs: [ + // mockito-target-extended dependencies + "libdexmakerjvmtiagent", + "libstaticjvmtiagent", + ], platform_apis: true, test_suites: ["device-tests"], } diff --git a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java index eb19361d86a3..97716377ea2e 100644 --- a/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java +++ b/tests/PackageWatchdog/src/com/android/server/PackageWatchdogTest.java @@ -27,6 +27,7 @@ import static org.junit.Assert.fail; import android.Manifest; import android.content.Context; import android.content.pm.VersionedPackage; +import android.net.NetworkStackClient; import android.os.Handler; import android.os.test.TestLooper; import android.provider.DeviceConfig; @@ -41,6 +42,8 @@ import com.android.server.PackageWatchdog.PackageHealthObserverImpact; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import java.io.File; import java.util.ArrayList; @@ -70,9 +73,12 @@ public class PackageWatchdogTest { private static final long SHORT_DURATION = TimeUnit.SECONDS.toMillis(1); private static final long LONG_DURATION = TimeUnit.SECONDS.toMillis(5); private TestLooper mTestLooper; + @Mock + private NetworkStackClient mNetworkStackClient; @Before public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); new File(InstrumentationRegistry.getContext().getFilesDir(), "package-watchdog.xml").delete(); adoptShellPermissions(Manifest.permission.READ_DEVICE_CONFIG); @@ -732,7 +738,8 @@ public class PackageWatchdogTest { new AtomicFile(new File(context.getFilesDir(), "package-watchdog.xml")); Handler handler = new Handler(mTestLooper.getLooper()); PackageWatchdog watchdog = - new PackageWatchdog(context, policyFile, handler, handler, controller); + new PackageWatchdog(context, policyFile, handler, handler, controller, + mNetworkStackClient); // Verify controller is not automatically started assertFalse(controller.mIsEnabled); if (withPackagesReady) { |