Add a lock when writing temporary files in BootReceiver

This is needed because there is potentially a race condition between when NativeTombstoneManager tries to delete leftover .tmp files and when those files are added to dropbox.

Direct cherrypick from internal master

Test: atest ErrorsTest
Bug: 284900963
Change-Id: I07936c51646915cb8760f45c4bb920f5fb335025
Merged-In: I07936c51646915cb8760f45c4bb920f5fb335025

2 files changed, 56 insertions, 35 deletions

diff --git a/services/core/java/com/android/server/BootReceiver.java b/services/core/java/com/android/server/BootReceiver.java
index 7d016c82adc5..926d7a4d3ea6 100644
--- a/services/core/java/com/android/server/BootReceiver.java
+++ b/services/core/java/com/android/server/BootReceiver.java
@@ -62,6 +62,7 @@ import java.nio.file.Files;
 import java.nio.file.attribute.PosixFilePermissions;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -328,9 +329,11 @@ public class BootReceiver extends BroadcastReceiver {
      * @param tombstone path to the tombstone
      * @param proto whether the tombstone is stored as proto
      * @param processName the name of the process corresponding to the tombstone
+     * @param tmpFileLock the lock for reading/writing tmp files
      */
     public static void addTombstoneToDropBox(
-                Context ctx, File tombstone, boolean proto, String processName) {
+                Context ctx, File tombstone, boolean proto, String processName,
+                ReentrantLock tmpFileLock) {
         final DropBoxManager db = ctx.getSystemService(DropBoxManager.class);
         if (db == null) {
             Slog.e(TAG, "Can't log tombstone: DropBoxManager not available");
@@ -351,39 +354,11 @@ public class BootReceiver extends BroadcastReceiver {
                     // due to rate limiting. Do this by enclosing the proto tombsstone in a
                     // container proto that has the dropped entry count and the proto tombstone as
                     // bytes (to avoid the complexity of reading and writing nested protos).
-
-                    // Read the proto tombstone file as bytes.
-                    final byte[] tombstoneBytes = Files.readAllBytes(tombstone.toPath());
-
-                    final File tombstoneProtoWithHeaders = File.createTempFile(
-                            tombstone.getName(), ".tmp", TOMBSTONE_TMP_DIR);
-                    Files.setPosixFilePermissions(
-                            tombstoneProtoWithHeaders.toPath(),
-                            PosixFilePermissions.fromString("rw-rw----"));
-
-                    // Write the new proto container proto with headers.
-                    try (ParcelFileDescriptor pfd = ParcelFileDescriptor.open(
-                            tombstoneProtoWithHeaders, MODE_READ_WRITE)) {
-                        ProtoOutputStream protoStream = new ProtoOutputStream(
-                                pfd.getFileDescriptor());
-                        protoStream.write(TombstoneWithHeadersProto.TOMBSTONE, tombstoneBytes);
-                        protoStream.write(
-                                TombstoneWithHeadersProto.DROPPED_COUNT,
-                                rateLimitResult.droppedCountSinceRateLimitActivated());
-                        protoStream.flush();
-
-                        // Add the proto to dropbox.
-                        db.addFile(TAG_TOMBSTONE_PROTO_WITH_HEADERS, tombstoneProtoWithHeaders, 0);
-                    } catch (FileNotFoundException ex) {
-                        Slog.e(TAG, "failed to open for write: " + tombstoneProtoWithHeaders, ex);
-                        throw ex;
-                    } catch (IOException ex) {
-                        Slog.e(TAG, "IO exception during write: " + tombstoneProtoWithHeaders, ex);
+                    tmpFileLock.lock();
+                    try {
+                        addAugmentedProtoToDropbox(tombstone, db, rateLimitResult);
                     } finally {
-                        // Remove the temporary file.
-                        if (tombstoneProtoWithHeaders != null) {
-                            tombstoneProtoWithHeaders.delete();
-                        }
+                        tmpFileLock.unlock();
                     }
                 }
             } else {
@@ -399,6 +374,44 @@ public class BootReceiver extends BroadcastReceiver {
         writeTimestamps(timestamps);
     }
 
+    private static void addAugmentedProtoToDropbox(
+                File tombstone, DropBoxManager db,
+                DropboxRateLimiter.RateLimitResult rateLimitResult) throws IOException {
+        // Read the proto tombstone file as bytes.
+        final byte[] tombstoneBytes = Files.readAllBytes(tombstone.toPath());
+
+        final File tombstoneProtoWithHeaders = File.createTempFile(
+                tombstone.getName(), ".tmp", TOMBSTONE_TMP_DIR);
+        Files.setPosixFilePermissions(
+                tombstoneProtoWithHeaders.toPath(),
+                PosixFilePermissions.fromString("rw-rw----"));
+
+        // Write the new proto container proto with headers.
+        try (ParcelFileDescriptor pfd = ParcelFileDescriptor.open(
+                    tombstoneProtoWithHeaders, MODE_READ_WRITE)) {
+            ProtoOutputStream protoStream =
+                    new ProtoOutputStream(pfd.getFileDescriptor());
+            protoStream.write(TombstoneWithHeadersProto.TOMBSTONE, tombstoneBytes);
+            protoStream.write(
+                    TombstoneWithHeadersProto.DROPPED_COUNT,
+                    rateLimitResult.droppedCountSinceRateLimitActivated());
+            protoStream.flush();
+
+            // Add the proto to dropbox.
+            db.addFile(TAG_TOMBSTONE_PROTO_WITH_HEADERS, tombstoneProtoWithHeaders, 0);
+        } catch (FileNotFoundException ex) {
+            Slog.e(TAG, "failed to open for write: " + tombstoneProtoWithHeaders, ex);
+            throw ex;
+        } catch (IOException ex) {
+            Slog.e(TAG, "IO exception during write: " + tombstoneProtoWithHeaders, ex);
+        } finally {
+            // Remove the temporary file and unlock the lock.
+            if (tombstoneProtoWithHeaders != null) {
+                tombstoneProtoWithHeaders.delete();
+            }
+        }
+    }
+
     private static void addLastkToDropBox(
             DropBoxManager db, HashMap<String, Long> timestamps,
             String headers, String footers, String filename, int maxSize,
diff --git a/services/core/java/com/android/server/os/NativeTombstoneManager.java b/services/core/java/com/android/server/os/NativeTombstoneManager.java
index e5616d04554d..ab0d0d2626db 100644
--- a/services/core/java/com/android/server/os/NativeTombstoneManager.java
+++ b/services/core/java/com/android/server/os/NativeTombstoneManager.java
@@ -61,6 +61,7 @@ import java.util.Collections;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * A class to manage native tombstones.
@@ -74,6 +75,8 @@ public final class NativeTombstoneManager {
     private final Handler mHandler;
     private final TombstoneWatcher mWatcher;
 
+    private final ReentrantLock mTmpFileLock = new ReentrantLock();
+
     private final Object mLock = new Object();
 
     @GuardedBy("mLock")
@@ -112,7 +115,12 @@ public final class NativeTombstoneManager {
 
         // Clean up temporary files if they made it this far (e.g. if system server crashes).
         if (filename.endsWith(".tmp")) {
-            path.delete();
+            mTmpFileLock.lock();
+            try {
+                path.delete();
+            } finally {
+                mTmpFileLock.unlock();
+            }
             return;
         }
 
@@ -128,7 +136,7 @@ public final class NativeTombstoneManager {
         if (parsedTombstone.isPresent()) {
             processName = parsedTombstone.get().getProcessName();
         }
-        BootReceiver.addTombstoneToDropBox(mContext, path, isProtoFile, processName);
+        BootReceiver.addTombstoneToDropBox(mContext, path, isProtoFile, processName, mTmpFileLock);
     }
 
     private Optional<TombstoneFile> handleProtoTombstone(File path, boolean addToList) {