From 78ec67a264b6b25b3353514c4f9ce5ea1b9fb1f3 Mon Sep 17 00:00:00 2001 From: Michal Karpinski Date: Mon, 18 Sep 2017 13:44:12 +0100 Subject: [PATCH 1/2] [RefactoredBMS] Eliminate a race condition that could lead to calling PBT#finalizeBackup() twice This CL replicates ag/2510127 in RefactoredBMS. Test: runtest -p com.android.server.backup frameworks-services Bug: 37973765 Change-Id: Ic8571c08dd92386d0e4f79d7866aeb0f19f38106 --- .../com/android/server/backup/internal/PerformBackupTask.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java b/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java index ce4f906ecdec5..6bbed8cbfdf9d 100644 --- a/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java +++ b/services/backup/java/com/android/server/backup/internal/PerformBackupTask.java @@ -227,9 +227,8 @@ public class PerformBackupTask implements BackupRestoreTask { if (!mFinished) { finalizeBackup(); } else { - Slog.e(TAG, "Duplicate finish"); + Slog.e(TAG, "Duplicate finish of K/V pass"); } - mFinished = true; break; } } @@ -609,6 +608,7 @@ public class PerformBackupTask implements BackupRestoreTask { break; } } + mFinished = true; Slog.i(TAG, "K/V backup pass finished."); // Only once we're entirely finished do we release the wakelock for k/v backup. backupManagerService.getWakelock().release(); From f9b74cc7ef0e3fc6b95e4a988361b46e90bdfd5d Mon Sep 17 00:00:00 2001 From: Michal Karpinski Date: Mon, 18 Sep 2017 14:15:56 +0100 Subject: [PATCH 2/2] [RefactoredBMS] Ensure backup doesn't reuse ack tokens nearby in time This CL replicates both ag/2551800 and ag/2613950 in RefactoredBMS. Test: runtest -p com.android.server.backup frameworks-services Bug: 37973765 Change-Id: I5e42dee67d587adabbfb81de03f3205d92a46add --- .../backup/RefactoredBackupManagerService.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java index dd99b023c6aab..f158661139319 100644 --- a/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java +++ b/services/backup/java/com/android/server/backup/RefactoredBackupManagerService.java @@ -150,6 +150,7 @@ import java.util.Queue; import java.util.Random; import java.util.Set; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; public class RefactoredBackupManagerService implements BackupManagerServiceInterface { @@ -626,6 +627,7 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter private final SparseArray mCurrentOperations = new SparseArray<>(); private final Object mCurrentOpLock = new Object(); private final Random mTokenGenerator = new Random(); + final AtomicInteger mNextToken = new AtomicInteger(); private final SparseArray mAdbBackupRestoreConfirmations = new SparseArray<>(); @@ -665,15 +667,15 @@ public class RefactoredBackupManagerService implements BackupManagerServiceInter @GuardedBy("mQueueLock") private ArrayList mFullBackupQueue; - // Utility: build a new random integer token + // Utility: build a new random integer token. The low bits are the ordinal of the + // operation for near-time uniqueness, and the upper bits are random for app- + // side unpredictability. @Override public int generateRandomIntegerToken() { - int token; - do { - synchronized (mTokenGenerator) { - token = mTokenGenerator.nextInt(); - } - } while (token < 0); + int token = mTokenGenerator.nextInt(); + if (token < 0) token = -token; + token &= ~0xFF; + token |= (mNextToken.incrementAndGet() & 0xFF); return token; }