Maybe fix issue #2457218: Corrupt batterystats.bin file preventing phone boot - LIBtt68127
No steps to repro, but makes the code more robust by using the standard
JournaledFile class and doing sanity checks on the input it reads.
This required moving the JournaledFile class in to the framework (and
we really should get rid of either it or AtomicFile, but they have
different recovery semantics so that is tough). Also went through and
cleaned up the file management in various places.
Change-Id: Ieb7268d8435e77dff66b6e67bb63b62e5dea572e
diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java
index 4464ab9..f296f43 100644
--- a/core/java/android/app/ContextImpl.java
+++ b/core/java/android/app/ContextImpl.java
@@ -2935,9 +2935,14 @@
private boolean writeFileLocked() {
// Rename the current file so it may be used as a backup during the next read
if (mFile.exists()) {
- if (!mFile.renameTo(mBackupFile)) {
- Log.e(TAG, "Couldn't rename file " + mFile + " to backup file " + mBackupFile);
- return false;
+ if (!mBackupFile.exists()) {
+ if (!mFile.renameTo(mBackupFile)) {
+ Log.e(TAG, "Couldn't rename file " + mFile
+ + " to backup file " + mBackupFile);
+ return false;
+ }
+ } else {
+ mFile.delete();
}
}
diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java
index 4780cf3..a17b7fe 100644
--- a/core/java/android/os/FileUtils.java
+++ b/core/java/android/os/FileUtils.java
@@ -115,6 +115,9 @@
*/
public static boolean copyToFile(InputStream inputStream, File destFile) {
try {
+ if (destFile.exists()) {
+ destFile.delete();
+ }
OutputStream out = new FileOutputStream(destFile);
try {
byte[] buffer = new byte[4096];
diff --git a/core/java/android/view/ViewDebug.java b/core/java/android/view/ViewDebug.java
index 8311bdc..2b5489c 100644
--- a/core/java/android/view/ViewDebug.java
+++ b/core/java/android/view/ViewDebug.java
@@ -546,6 +546,9 @@
recyclerDump = new File(Environment.getExternalStorageDirectory(), "view-recycler/");
recyclerDump = new File(recyclerDump, sRecyclerTracePrefix + ".traces");
try {
+ if (recyclerDump.exists()) {
+ recyclerDump.delete();
+ }
final FileOutputStream file = new FileOutputStream(recyclerDump);
final DataOutputStream out = new DataOutputStream(file);
diff --git a/core/java/com/android/internal/backup/LocalTransport.java b/core/java/com/android/internal/backup/LocalTransport.java
index 9e4b606..b436363 100644
--- a/core/java/com/android/internal/backup/LocalTransport.java
+++ b/core/java/com/android/internal/backup/LocalTransport.java
@@ -105,6 +105,9 @@
+ " key64=" + base64Key);
if (dataSize >= 0) {
+ if (entityFile.exists()) {
+ entityFile.delete();
+ }
FileOutputStream entity = new FileOutputStream(entityFile);
if (dataSize > bufSize) {
diff --git a/core/java/com/android/internal/os/AtomicFile.java b/core/java/com/android/internal/os/AtomicFile.java
index ca0345f..e675ef0 100644
--- a/core/java/com/android/internal/os/AtomicFile.java
+++ b/core/java/com/android/internal/os/AtomicFile.java
@@ -45,12 +45,13 @@
public FileOutputStream startWrite() throws IOException {
// Rename the current file so it may be used as a backup during the next read
if (mBaseName.exists()) {
- if (!mBaseName.renameTo(mBackupName)) {
- mBackupName.delete();
+ if (!mBackupName.exists()) {
if (!mBaseName.renameTo(mBackupName)) {
Log.w("AtomicFile", "Couldn't rename file " + mBaseName
+ " to backup file " + mBackupName);
}
+ } else {
+ mBaseName.delete();
}
}
FileOutputStream str = null;
diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java
index 71ccb3b..46769ce 100644
--- a/core/java/com/android/internal/os/BatteryStatsImpl.java
+++ b/core/java/com/android/internal/os/BatteryStatsImpl.java
@@ -16,6 +16,8 @@
package com.android.internal.os;
+import com.android.internal.util.JournaledFile;
+
import android.bluetooth.BluetoothHeadset;
import android.net.TrafficStats;
import android.os.BatteryStats;
@@ -30,6 +32,7 @@
import android.util.Log;
import android.util.PrintWriterPrinter;
import android.util.Printer;
+import android.util.Slog;
import android.util.SparseArray;
import java.io.BufferedReader;
@@ -57,7 +60,7 @@
private static final int MAGIC = 0xBA757475; // 'BATSTATS'
// Current on-disk Parcel version
- private static final int VERSION = 42;
+ private static final int VERSION = 43;
// The maximum number of names wakelocks we will keep track of
// per uid; once the limit is reached, we batch the remaining wakelocks
@@ -68,8 +71,7 @@
private static int sNumSpeedSteps;
- private final File mFile;
- private final File mBackupFile;
+ private final JournaledFile mFile;
/**
* The statistics we have collected organized by uids.
@@ -216,7 +218,7 @@
// For debugging
public BatteryStatsImpl() {
- mFile = mBackupFile = null;
+ mFile = null;
}
public static interface Unpluggable {
@@ -2704,8 +2706,7 @@
}
public BatteryStatsImpl(String filename) {
- mFile = new File(filename);
- mBackupFile = new File(filename + ".bak");
+ mFile = new JournaledFile(new File(filename), new File(filename + ".tmp"));
mStartCount++;
mScreenOnTimer = new StopwatchTimer(-1, null, mUnpluggables);
for (int i=0; i<NUM_SCREEN_BRIGHTNESS_BINS; i++) {
@@ -2736,7 +2737,7 @@
}
public BatteryStatsImpl(Parcel p) {
- mFile = mBackupFile = null;
+ mFile = null;
readFromParcel(p);
}
@@ -2799,7 +2800,7 @@
if (m == null) {
// Not crashing might make board bringup easier.
- Log.w(TAG, "Couldn't get kernel wake lock stats");
+ Slog.w(TAG, "Couldn't get kernel wake lock stats");
return;
}
@@ -3047,26 +3048,19 @@
return u.getServiceStatsLocked(pkg, name);
}
+ private static JournaledFile makeJournaledFile() {
+ final String base = "/data/system/device_policies.xml";
+ return new JournaledFile(new File(base), new File(base + ".tmp"));
+ }
+
public void writeLocked() {
- if ((mFile == null) || (mBackupFile == null)) {
- Log.w("BatteryStats", "writeLocked: no file associated with this instance");
+ if (mFile == null) {
+ Slog.w("BatteryStats", "writeLocked: no file associated with this instance");
return;
}
- // Keep the old file around until we know the new one has
- // been successfully written.
- if (mFile.exists()) {
- if (mBackupFile.exists()) {
- mBackupFile.delete();
- }
- if (!mFile.renameTo(mBackupFile)) {
- Log.w("BatteryStats", "Failed to back up file before writing new stats");
- return;
- }
- }
-
try {
- FileOutputStream stream = new FileOutputStream(mFile);
+ FileOutputStream stream = new FileOutputStream(mFile.chooseForWrite());
Parcel out = Parcel.obtain();
writeSummaryToParcel(out);
stream.write(out.marshall());
@@ -3074,18 +3068,14 @@
stream.flush();
stream.close();
- mBackupFile.delete();
+ mFile.commit();
mLastWriteTime = SystemClock.elapsedRealtime();
return;
} catch (IOException e) {
- Log.w("BatteryStats", "Error writing battery statistics", e);
+ Slog.w("BatteryStats", "Error writing battery statistics", e);
}
- if (mFile.exists()) {
- if (!mFile.delete()) {
- Log.w(TAG, "Failed to delete mangled file " + mFile);
- }
- }
+ mFile.rollback();
}
static byte[] readFully(FileInputStream stream) throws java.io.IOException {
@@ -3112,29 +3102,19 @@
}
public void readLocked() {
- if ((mFile == null) || (mBackupFile == null)) {
- Log.w("BatteryStats", "readLocked: no file associated with this instance");
+ if (mFile == null) {
+ Slog.w("BatteryStats", "readLocked: no file associated with this instance");
return;
}
mUidStats.clear();
- FileInputStream stream = null;
- if (mBackupFile.exists()) {
- try {
- stream = new FileInputStream(mBackupFile);
- } catch (java.io.IOException e) {
- // We'll try for the normal settings file.
- }
- }
-
try {
- if (stream == null) {
- if (!mFile.exists()) {
- return;
- }
- stream = new FileInputStream(mFile);
+ File file = mFile.chooseForRead();
+ if (!file.exists()) {
+ return;
}
+ FileInputStream stream = new FileInputStream(file);
byte[] raw = readFully(stream);
Parcel in = Parcel.obtain();
@@ -3144,7 +3124,7 @@
readSummaryFromParcel(in);
} catch(java.io.IOException e) {
- Log.e("BatteryStats", "Error reading battery statistics", e);
+ Slog.e("BatteryStats", "Error reading battery statistics", e);
}
}
@@ -3155,7 +3135,7 @@
private void readSummaryFromParcel(Parcel in) {
final int version = in.readInt();
if (version != VERSION) {
- Log.w("BatteryStats", "readFromParcel: version got " + version
+ Slog.w("BatteryStats", "readFromParcel: version got " + version
+ ", expected " + VERSION + "; erasing old stats");
return;
}
@@ -3197,6 +3177,10 @@
mBluetoothOnTimer.readSummaryFromParcelLocked(in);
int NKW = in.readInt();
+ if (NKW > 10000) {
+ Slog.w(TAG, "File corrupt: too many kernel wake locks " + NKW);
+ return;
+ }
for (int ikw = 0; ikw < NKW; ikw++) {
if (in.readInt() != 0) {
String kwltName = in.readString();
@@ -3207,6 +3191,10 @@
sNumSpeedSteps = in.readInt();
final int NU = in.readInt();
+ if (NU > 10000) {
+ Slog.w(TAG, "File corrupt: too many uids " + NU);
+ return;
+ }
for (int iu = 0; iu < NU; iu++) {
int uid = in.readInt();
Uid u = new Uid(uid);
@@ -3235,6 +3223,10 @@
}
int NW = in.readInt();
+ if (NW > 10000) {
+ Slog.w(TAG, "File corrupt: too many wake locks " + NW);
+ return;
+ }
for (int iw = 0; iw < NW; iw++) {
String wlName = in.readString();
if (in.readInt() != 0) {
@@ -3249,6 +3241,10 @@
}
int NP = in.readInt();
+ if (NP > 10000) {
+ Slog.w(TAG, "File corrupt: too many sensors " + NP);
+ return;
+ }
for (int is = 0; is < NP; is++) {
int seNumber = in.readInt();
if (in.readInt() != 0) {
@@ -3258,6 +3254,10 @@
}
NP = in.readInt();
+ if (NP > 10000) {
+ Slog.w(TAG, "File corrupt: too many processes " + NP);
+ return;
+ }
for (int ip = 0; ip < NP; ip++) {
String procName = in.readString();
Uid.Proc p = u.getProcessStatsLocked(procName);
@@ -3270,6 +3270,10 @@
}
NP = in.readInt();
+ if (NP > 10000) {
+ Slog.w(TAG, "File corrupt: too many packages " + NP);
+ return;
+ }
for (int ip = 0; ip < NP; ip++) {
String pkgName = in.readString();
Uid.Pkg p = u.getPackageStatsLocked(pkgName);
diff --git a/services/java/com/android/server/JournaledFile.java b/core/java/com/android/internal/util/JournaledFile.java
similarity index 98%
rename from services/java/com/android/server/JournaledFile.java
rename to core/java/com/android/internal/util/JournaledFile.java
index 3d1f52d..af0c6c6 100644
--- a/services/java/com/android/server/JournaledFile.java
+++ b/core/java/com/android/internal/util/JournaledFile.java
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-package com.android.server;
+package com.android.internal.util;
import java.io.File;
import java.io.IOException;
diff --git a/services/java/com/android/server/DevicePolicyManagerService.java b/services/java/com/android/server/DevicePolicyManagerService.java
index a555244..1d71577 100644
--- a/services/java/com/android/server/DevicePolicyManagerService.java
+++ b/services/java/com/android/server/DevicePolicyManagerService.java
@@ -18,6 +18,7 @@
import com.android.internal.content.PackageMonitor;
import com.android.internal.util.FastXmlSerializer;
+import com.android.internal.util.JournaledFile;
import com.android.internal.util.XmlUtils;
import com.android.internal.widget.LockPatternUtils;
diff --git a/services/java/com/android/server/PackageManagerService.java b/services/java/com/android/server/PackageManagerService.java
index 677ff4a..48b3fbb 100644
--- a/services/java/com/android/server/PackageManagerService.java
+++ b/services/java/com/android/server/PackageManagerService.java
@@ -20,8 +20,8 @@
import com.android.internal.app.ResolverActivity;
import com.android.internal.content.PackageHelper;
import com.android.internal.util.FastXmlSerializer;
+import com.android.internal.util.JournaledFile;
import com.android.internal.util.XmlUtils;
-import com.android.server.JournaledFile;
import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlPullParserException;
@@ -8280,6 +8280,7 @@
return;
}
} else {
+ mSettingsFilename.delete();
Slog.w(TAG, "Preserving older settings backup");
}
}
diff --git a/services/java/com/android/server/WallpaperManagerService.java b/services/java/com/android/server/WallpaperManagerService.java
index f4bdd1f..124da4e 100644
--- a/services/java/com/android/server/WallpaperManagerService.java
+++ b/services/java/com/android/server/WallpaperManagerService.java
@@ -68,6 +68,7 @@
import com.android.internal.content.PackageMonitor;
import com.android.internal.service.wallpaper.ImageWallpaper;
import com.android.internal.util.FastXmlSerializer;
+import com.android.internal.util.JournaledFile;
import com.android.server.DevicePolicyManagerService.ActiveAdmin;
import com.android.server.DevicePolicyManagerService.MyPackageMonitor;
@@ -804,6 +805,9 @@
}
res = r.openRawResource(resId);
+ if (WALLPAPER_FILE.exists()) {
+ WALLPAPER_FILE.delete();
+ }
fos = new FileOutputStream(WALLPAPER_FILE);
byte[] buffer = new byte[32768];
diff --git a/services/java/com/android/server/am/UsageStatsService.java b/services/java/com/android/server/am/UsageStatsService.java
index d170b02..1b9e1c7 100644
--- a/services/java/com/android/server/am/UsageStatsService.java
+++ b/services/java/com/android/server/am/UsageStatsService.java
@@ -383,9 +383,13 @@
File backupFile = null;
if (mFile != null && mFile.exists()) {
backupFile = new File(mFile.getPath() + ".bak");
- if (!mFile.renameTo(backupFile)) {
- Slog.w(TAG, "Failed to persist new stats");
- return;
+ if (!backupFile.exists()) {
+ if (!mFile.renameTo(backupFile)) {
+ Slog.w(TAG, "Failed to persist new stats");
+ return;
+ }
+ } else {
+ mFile.delete();
}
}
