Verify arguments passed are non-null.
We should verify they are non-null before
handing them off to other threads to avoid
crashing the system_server process.
Bug: 356731520
Test: atest services/tests/powerstatstests/src/com/android/server/powerstats/PowerStatsServiceTest.java
Flag: com.android.server.powerstats.verify_non_null_arguments
Change-Id: I5b0cbf86976e74fd01e3d70ac01a03398ecdaa56
diff --git a/services/core/java/com/android/server/powerstats/PowerStatsService.java b/services/core/java/com/android/server/powerstats/PowerStatsService.java
index 8f99d28..1ca267e99 100644
--- a/services/core/java/com/android/server/powerstats/PowerStatsService.java
+++ b/services/core/java/com/android/server/powerstats/PowerStatsService.java
@@ -62,6 +62,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
+import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Executor;
@@ -207,19 +208,28 @@
private final IBinder mService = new IPowerStatsService.Stub() {
@Override
- public void getSupportedPowerMonitors(ResultReceiver resultReceiver) {
+ public void getSupportedPowerMonitors(@NonNull ResultReceiver resultReceiver) {
+ if (Flags.verifyNonNullArguments()) {
+ Objects.requireNonNull(resultReceiver);
+ }
getHandler().post(() -> getSupportedPowerMonitorsImpl(resultReceiver));
}
@Override
- public void getPowerMonitorReadings(int[] powerMonitorIds, ResultReceiver resultReceiver) {
+ public void getPowerMonitorReadings(@NonNull int[] powerMonitorIds,
+ @NonNull ResultReceiver resultReceiver) {
+ if (Flags.verifyNonNullArguments()) {
+ Objects.requireNonNull(powerMonitorIds);
+ Objects.requireNonNull(resultReceiver);
+ }
int callingUid = Binder.getCallingUid();
getHandler().post(() ->
getPowerMonitorReadingsImpl(powerMonitorIds, resultReceiver, callingUid));
}
@Override
- protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
+ protected void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter pw,
+ @Nullable String[] args) {
if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return;
if (mPowerStatsLogger == null) {
@@ -263,6 +273,11 @@
}
};
+ @VisibleForTesting
+ IPowerStatsService getIPowerStatsServiceForTest() {
+ return (IPowerStatsService) mService;
+ }
+
private class DeviceConfigListener implements DeviceConfig.OnPropertiesChangedListener {
public Executor mExecutor = new HandlerExecutor(getHandler());
diff --git a/services/core/java/com/android/server/powerstats/flags.aconfig b/services/core/java/com/android/server/powerstats/flags.aconfig
index 0a4a751..29ad7dc 100644
--- a/services/core/java/com/android/server/powerstats/flags.aconfig
+++ b/services/core/java/com/android/server/powerstats/flags.aconfig
@@ -10,4 +10,15 @@
metadata {
purpose: PURPOSE_BUGFIX
}
+}
+
+flag {
+ name: "verify_non_null_arguments"
+ namespace: "backstage_power"
+ description: "Verify arguments passed are non-null"
+ bug: "356731520"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
}
\ No newline at end of file
diff --git a/services/tests/powerstatstests/src/com/android/server/powerstats/PowerStatsServiceTest.java b/services/tests/powerstatstests/src/com/android/server/powerstats/PowerStatsServiceTest.java
index 1838fe8..53e3143 100644
--- a/services/tests/powerstatstests/src/com/android/server/powerstats/PowerStatsServiceTest.java
+++ b/services/tests/powerstatstests/src/com/android/server/powerstats/PowerStatsServiceTest.java
@@ -19,6 +19,7 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -37,6 +38,8 @@
import android.os.Looper;
import android.os.PowerMonitor;
import android.os.ResultReceiver;
+import android.platform.test.annotations.EnableFlags;
+import android.platform.test.flag.junit.SetFlagsRule;
import android.provider.DeviceConfig;
import android.provider.DeviceConfigInterface;
@@ -58,6 +61,7 @@
import com.android.server.testutils.FakeDeviceConfigInterface;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
import java.io.ByteArrayOutputStream;
@@ -101,6 +105,8 @@
private static final int STATE_RESIDENCY_COUNT = 4;
private static final int APP_UID = 10042;
+ @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();
+
private final Context mContext = InstrumentationRegistry.getInstrumentation().getContext();
private PowerStatsService mService;
private TestPowerStatsHALWrapper mPowerStatsHALWrapper = new TestPowerStatsHALWrapper();
@@ -1198,4 +1204,22 @@
assertThat(Arrays.stream(supportedPowerMonitorsResult.powerMonitors)
.map(PowerMonitor::getName).toList()).contains("ENERGYCONSUMER0");
}
+
+ @EnableFlags(Flags.FLAG_VERIFY_NON_NULL_ARGUMENTS)
+ @Test
+ public void testGetSupportedPowerMonitors_withNullArguments() {
+ IPowerStatsService iPowerStatsService = mService.getIPowerStatsServiceForTest();
+ assertThrows(NullPointerException.class,
+ () -> iPowerStatsService.getSupportedPowerMonitors(null));
+ }
+
+ @EnableFlags(Flags.FLAG_VERIFY_NON_NULL_ARGUMENTS)
+ @Test
+ public void testGetPowerMonitorReadings_withNullArguments() {
+ IPowerStatsService iPowerStatsService = mService.getIPowerStatsServiceForTest();
+ assertThrows(NullPointerException.class, () -> iPowerStatsService.getPowerMonitorReadings(
+ null, new GetPowerMonitorsResult()));
+ assertThrows(NullPointerException.class, () -> iPowerStatsService.getPowerMonitorReadings(
+ new int[] {0}, null));
+ }
}