From b20792558e9d9d22fb04cb8994d2a4cc0ce9103e Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Thu, 7 Jul 2022 19:00:52 +0000
Subject: [PATCH] Fix deleteSyntheticPasswordState() to take mFileWriteLock

The cache consistency model in LockSettingsStorage requires that
mFileWriteLock be held during file writes and deletions.
deleteSyntheticPasswordState() forgot to take this lock.  Fix it by
making it just call deleteFile().

Make deleteFile() zeroize the file before unlinking it, so that the
zeroization that deleteSyntheticPasswordState() did isn't lost, and so
that the zeroization is applied to the other locksettings files too.

Found by code review; this bug isn't known to have been causing any
real-world problems.

Test: atest com.android.server.locksettings
Change-Id: I8b4c8468a4ab533164b5750e109e2426f74c9110
---
 .../locksettings/LockSettingsStorage.java     | 24 ++++++++-----------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java
index 10831c7e8f4ca..ec66599ebe828 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java
@@ -344,10 +344,18 @@ class LockSettingsStorage extends WatchableImpl {
     private void deleteFile(File path) {
         synchronized (mFileWriteLock) {
             if (path.exists()) {
+                // Zeroize the file to try to make its contents unrecoverable.  This is *not*
+                // guaranteed to be effective, and in fact it usually isn't, but it doesn't hurt.
+                try (RandomAccessFile raf = new RandomAccessFile(path, "rws")) {
+                    final int fileSize = (int) raf.length();
+                    raf.write(new byte[fileSize]);
+                } catch (Exception e) {
+                    Slog.w(TAG, "Failed to zeroize " + path, e);
+                }
                 path.delete();
+                dispatchChange(this);
                 mCache.putFile(path, null);
             }
-            dispatchChange(this);
         }
     }
 
@@ -387,19 +395,7 @@ class LockSettingsStorage extends WatchableImpl {
     }
 
     public void deleteSyntheticPasswordState(int userId, long handle, String name) {
-        File path = getSyntheticPasswordStateFileForUser(userId, handle, name);
-        if (path.exists()) {
-            try (RandomAccessFile raf = new RandomAccessFile(path, "rws")) {
-                final int fileSize = (int) raf.length();
-                raf.write(new byte[fileSize]);
-            } catch (Exception e) {
-                Slog.w(TAG, "Failed to zeroize " + path, e);
-            } finally {
-                path.delete();
-                dispatchChange(this);
-            }
-            mCache.putFile(path, null);
-        }
+        deleteFile(getSyntheticPasswordStateFileForUser(userId, handle, name));
     }
 
     public Map<Integer, List<Long>> listSyntheticPasswordHandlesForAllUsers(String stateName) {