Commit graph

18 commits

Author SHA1 Message Date
Eric Biggers
1ba8865fec EncryptInplace: fsync cryptofd before reporting success
fsync() the cryptofd when done writing to it.  Without this, any
remaining dirty pages in the crypto_blkdev's page cache (which there
might be a lot of, even as much as all the data that was written) won't
be flushed to disk until the cryptofd is closed, which ignores I/O
errors and is also after we already reported 100% completion.

There wasn't an fsync() in the original version either, so we've been
getting by without it, but it seems it should be there.

Change-Id: Idd1be3ae67ce96ecf3946b9efb9fc57414f5805a
2020-11-04 19:24:19 -08:00
Eric Biggers
f038c5f5e1 Refactor EncryptInplace.cpp
Refactor EncryptInplace.cpp to simplify and improve it a lot.  This is
everything that didn't fit into prior commits, including:

- Share a lot more code between ext4, f2fs, and full encryption.

- Improve the log messages.  Most importantly, don't spam the log with
  huge numbers of messages, and don't log errors in expected cases.
  Note: generate_f2fs_info() is still too noisy, but that's part of
  "system/extras", not vold, so this change doesn't change that.

- When possible, do 32K reads/writes for f2fs and for full encryption,
  not just for ext4.  This might improve performance.

- Take advantage of C++ functionality.

- Be more careful about edge cases.  E.g. if the calculation of the
  number of blocks to encrypt was wrong, don't set vold.encrypt_progress
  to > 99 until we're actually done.

The net change is over 200 lines removed.

Before-after comparison of log when enabling metadata encryption:

ext4 before:
    I vold    : Beginning inplace encryption, nr_sec: 16777216
    D vold    : cryptfs_enable_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, 0)
    D vold    : Opening/dev/block/by-name/userdata
    D vold    : Opening/dev/block/dm-8
    I vold    : Encrypting ext4 filesystem in place...
    [omitted 6387 log messages]
    I vold    : Encrypted to sector 822084608
    D vold    : cryptfs_enable_inplace_ext4 success
    I vold    : Inplace encryption complete

ext4 after:
    D vold    : encrypt_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, false)
    D vold    : ext4 filesystem has 64 block groups
    I vold    : Encrypting ext4 filesystem on /dev/block/by-name/userdata in-place via /dev/block/dm-8
    I vold    : 50327 blocks (206 MB) of 2097152 blocks are in-use
    D vold    : Encrypted 10000 of 50327 blocks
    D vold    : Encrypted 20000 of 50327 blocks
    D vold    : Encrypted 30000 of 50327 blocks
    D vold    : Encrypted 40000 of 50327 blocks
    D vold    : Encrypted 50000 of 50327 blocks
    D vold    : Encrypted 50327 of 50327 blocks
    I vold    : Successfully encrypted ext4 filesystem on /dev/block/by-name/userdata

f2fs before:
    I vold    : Beginning inplace encryption, nr_sec: 16777216
    D vold    : cryptfs_enable_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, 0)
    D vold    : Opening/dev/block/by-name/userdata
    D vold    : Opening/dev/block/dm-8
    E vold    : Reading ext4 extent caused an exception
    D vold    : cryptfs_enable_inplace_ext4()=-1
    [omitted logspam from f2fs_sparseblock]
    I vold    : Encrypting from block 0
    I vold    : Encrypted to block 15872
    I vold    : Encrypting from block 16384
    I vold    : Encrypted to block 16385
    I vold    : Encrypting from block 17408
    I vold    : Encrypted to block 17412
    D vold    : cryptfs_enable_inplace_f2fs success
    I vold    : Inplace encryption complete

f2fs after:
    D vold    : encrypt_inplace(/dev/block/dm-8, /dev/block/by-name/userdata, 16777216, false)
    [omitted logspam from f2fs_sparseblock]
    I vold    : Encrypting f2fs filesystem on /dev/block/by-name/userdata in-place via /dev/block/dm-8
    I vold    : 15880 blocks (65 MB) of 2097152 blocks are in-use
    D vold    : Encrypted 10000 of 15880 blocks
    D vold    : Encrypted 15880 of 15880 blocks
    I vold    : Successfully encrypted f2fs filesystem on /dev/block/by-name/userdata

Test: Booted Cuttlefish with metadata encryption enabled and with the
      userdata filesystem using (1) ext4, (2) f2fs, and (3) f2fs but
      with EncryptInplace.cpp patched to not recognize the filesystem
      and fall back to the "full" encryption case.  Checked that the log
      messages were as expected and that /data was mounted.

      I've had no luck testing FDE yet; it doesn't work even without
      these changes.  Suggestions appreciated...

Change-Id: I08fc8465f7962abd698904b5466f3ed080d53953
2020-11-03 14:16:32 -08:00
Eric Biggers
7e70d6939d Correctly calculate tot_used_blocks on ext4 with uninit_bg
The calculated number of blocks to encrypt is too high on ext4
filesystems that have the uninit_bg feature.  This is because the
calculation assumes that all blocks not counted in bg_free_blocks_count
need to encrypted.  But actually, uninitialized block groups have inode
blocks which vold doesn't encrypt since they are uninitialized, but they
are "allocated" and thus reduce bg_free_blocks_count.

Therefore, add a helper function num_base_meta_blocks_in_group() which
returns the number of blocks to encrypt in an uninitialized block group.
Use it both for the encryption and for calculating 'tot_used_blocks'.

Also compute 'tot_used_blocks' additively rather than subtractively, as
this is easier to understand.

Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: I4d2cb40291da67dd1bafd61289ccb9e6343bfda3
2020-11-03 14:11:01 -08:00
Eric Biggers
b3ba087d9c Fix memory leak of f2fs_info
'struct f2fs_info' from system/extras/f2fs_utils is supposed to be
freed using free_f2fs_info(), not just free().

Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: If6e75e5c604b40be24538b156a37cc76f4f0d4f7
2020-11-03 14:11:01 -08:00
Eric Biggers
69520d2d39 Remove special handling for missing crypto_blkdev
This logic is no longer necessary, since the code that creates the
crypto_blkdev (create_crypto_blk_dev() in MetadataCrypt.cpp or in
cryptfs.cpp) now waits for the block device to appear before continuing.

It's also worth noting that the retry loop was only present for ext4,
not for f2fs, yet most Android devices are using f2fs these days.

Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: I173ca6cc187a810e008990dfa22aede58632db25
2020-11-03 14:11:01 -08:00
Eric Biggers
c01995ea3b Remove unused support for partial encryption
Commit 87999173dd ("Don't corrupt ssd when encrypting and power
fails") added a lot of code to handle pausing in-place conversion from
unencrypted => FDE when the battery was low, and resuming it later.

It was eventually decided that this wasn't needed, and commit
7e17e2d226 ("Don't worry about battery levels when encrypting")
removed the checks for low battery.

This made the partial encryption code unused.  So remove it.

Note that this was cluttering up the metadata encryption code too, since
EncryptInplace.cpp is now shared by both FDE and metadata encryption.

Bug: 16868177
Test: see I08fc8465f7962abd698904b5466f3ed080d53953
Change-Id: Ibd2eb08a2aa15938097abcb8a67b5a813c4d76c7
2020-11-03 14:11:00 -08:00
Eric Biggers
b67708361f vold: remove unused code for CONFIG_HW_DISK_ENCRYPTION
Nothing defines CONFIG_HW_DISK_ENCRYPTION, so remove the unused code
that's conditional on it being defined.

Change-Id: Ie435e138686eb4eac47d9aa762ae06f1645a117f
2020-09-15 11:57:30 -07:00
Will Shiu
4ac43f0e16 EncryptInPlace: ensure that backup superblocks get encrypted
Block groups with EXT4_BG_BLOCK_UNINIT still have backup superblocks
(and backup block group descriptors).  Fix EncryptInPlace to encrypt
these backup superblocks rather than leave them unencrypted.

Previously leaving the backup superblocks unencrypted didn't cause any
problems, but due to system/core commit 72abd7b246f7 ("Try to recover
corrupted ext4 /data with backup superblock") it is causing problems.

Bug: 162479411
Bug: 161871210
Change-Id: Ic090bf4e88193b289b04c5254ddf661ef40b037e
2020-08-06 11:05:45 -07:00
Denis Hsu
1740effeaa Consistently use CLOCK_MONOTONIC for timing encryption
time_started in encryptGroupsData is set from and compared to
clock_gettime(CLOCK_MONOTONIC, ...) nearly everywhere: "Clock that
cannot be set and represents monotonic time since some unspecified
starting point". However in cryptfs_enable_inplace_f2fs() it is set
from a different clock, time(NULL), with the result that the setprop
calls that indicate progress are wrong and can be called much too
often. The fix is to make this function consistent with
cryptfs_enable_inplace_ext4.

Bug: 146877356
Change-Id: I2707180e5c5bf723a5a880f6a3aac47f2bb34ccd
2019-12-26 18:53:53 +00:00
Paul Lawrence
236e5e800e Make ext4 userdata checkpoints work with metadata encryption
When both ext4 user data checkpoints and metadata encryption are
enabled, we are creating two stacked dm devices. This had not been
properly thought through or debugged.

Test: Enable metadata encryption on taimen (add
keydirectory=/metadata/vold/metadata_encryption to flags for userdata in
fstab.hardware)
    Unfortunately metadata is not wiped by fastboot -w, so it is
    necessary to rm metadata/vold -rf whenever you wipe data.
    fastboot flashall -w works
    fastboot reboot -w works
    A normal boot works
    Disable checkpoint commits with
    setprop persist.vold.dont_commit_checkpoint 1
    vdc checkpoint startCheckpoint 10
    adb reboot
    wait for device to fully boot then
    adb reboot
    Wait for device to fully boot then
    adb logcat -d | grep Checkpoint shows the rollback in the logs

    This tests encryption on top of checkpoints with commit, encryption
    without checkpoints, and rollback, which seems to be the key cases.

Bug: 135905679
Change-Id: I8365a40298b752af4bb10d00d9ff58ce04beab1f
2019-06-26 15:19:24 -07:00
Greg Kaiser
e0691cc674 EncryptInplace: Rename variable
We rename our 'buf' in the inner scope to avoid confusion with
the 'buf' in the outer scope which is used immediately after
exiting the inner scope.

Test: TreeHugger
Change-Id: I1c50546e86c680e963eedcbda26138f8b43e55e9
2018-12-20 10:38:31 -08:00
Sudheer Shanka
4b6ca4ea65 Update vold to log only debug or higher level messages.
This will allow adding lots of verbose logs which can be enabled
only during local testing/debugging. Update the existing verbose
level logs to debug level since we want those to be logged by
default.

Test: manual
Change-Id: Ib05e2b6efa71308458d49affb6ed81d3975b28ab
2018-09-21 11:16:51 -07:00
Paul Crowley
14c8c0765a clang-format many files.
Test: Format-only changes; treehugger suffices.
Change-Id: I23cde3f0bbcac13bef555d13514e922c79d5ad48
2018-09-18 15:41:22 -07:00
Paul Crowley
0fd2626fc3 Add a mount with metadata encryption service
Don't use the FDE flow to support metadata encryption; just provide a
vold service which directly mounts the volume and use that.

Bug: 63927601
Test: Boot Taimen to SUW with and without metadata encryption.
Change-Id: Ifc6a012c02c0ea66893020ed1d0da4cba6914aed
2018-02-01 10:08:17 -08:00
Paul Crowley
772cc85d71 Refactor logging in EncryptInplace.cpp
Done as part of work towards metadata encryption.

Bug: 63927601
Test: Boot Taimen to SUW

Change-Id: I0f5fda0e002944ab658756c7cfcb386c3658a446
2018-02-01 09:53:27 -08:00
Paul Crowley
5385417922 Remove CheckBattery altogether
Test: changed Angler fstab to encryptable and encrypted.
Bug: 16868177
Change-Id: I17d36ea838d6d96f0752b2d6d03b1f9a781ed018
2017-10-03 11:53:36 -07:00
Elliott Hughes
f654c04d01 <stdbool.h> unnecessary in C++.
Bug: N/A
Test: builds
Change-Id: Iddbd364e581477b8304dc6f0764f153dbcf122a7
2017-09-08 14:58:08 -07:00
Paul Crowley
f71ace310e Refactor to lay the groundwork for metadata encryption
Bug: 26778031
Test: Angler, Marlin build and boot
Change-Id: Ic136dfe6195a650f7db76d3489f36da6a1929dc5
2017-04-21 10:47:17 -07:00