init: Wait for snapuserd before starting second stage

This is a race between init process and bionic libc initialization of
snapuserd.

init->fork() ----------------> SecondStageMain() -> PropertyInit()
       |
       |
       v
      execveat ---> __libc_init_common() -> __system_properties_init()
     (snapuserd)

When init process calls PropertyInit(), /dev/__properties__ directory
is created. When bionic libc of snapuserd daemon invokes __system_properties_init
_after_ init process PropertyInit() function is invoked, libc will
try to initialize the property by reading
/system/etc/selinux/plat_property_contexts. Since any reads on /system
has to be served by snapuserd, this specific read from libc cannot be
serviced leading to deadlock.

Reproduce the race by inducing a sleep of 1500ms just before execveat()
so that init process calls PropertyInit() before bionic libc
initialization. This leads to deadlock
immediately and with additional kernel instrumentation with debug
logs confirms the failure:

======================================================
init: Relaunched snapuserd with pid: 428
ext4_file_open: SNAPUSERD: path /system/etc/selinux/plat_property_contexts - Pid: 428 comm 8
ext4_file_read_iter: SNAPUSERD for path: /system/etc/selinux/plat_property_contexts pid: 428 comm 8

[   25.418043][  T428]  ext4_file_read_iter+0x3dc/0x3e0
[   25.423000][  T428]  vfs_read+0x2e0/0x354
[   25.426986][  T428]  ksys_read+0x7c/0xec
[   25.430894][  T428]  __arm64_sys_read+0x20/0x30
[   25.435419][  T428]  el0_svc_common.llvm.17612735770287389485+0xd0/0x1e0
[   25.442095][  T428]  do_el0_svc+0x28/0xa0
[   25.446100][  T428]  el0_svc+0x14/0x24
[   25.449825][  T428]  el0_sync_handler+0x88/0xec
[   25.454343][  T428]  el0_sync+0x1c0/0x200

=====================================================

Fix:

Before starting init second stage, we will wait
for snapuserd daemon to be up and running. We do a simple probe by
reading system partition. This read will eventually be serviced by
daemon confirming that daemon is up and running. Furthermore,
we are still in the kernel domain and sepolicy has not been enforced yet.
Thus, access to these device mapper block devices are ok even though
we may see audit logs.

Note that daemon will re-initialize the __system_property_init()
as part of WaitForSocket() call. This is subtle but important; since
bionic libc initialized had failed silently, it is important
that this re-initialization is done.

Bug: 207298357
Test: Induce the failure by explicitly delaying the call of execveat().
      With fix, no issues observed.
      Tested incremental OTA on pixel ~15 times.

Ignore-AOSP-First: cherry-pick from AOSP
Signed-off-by: Akilesh Kailash <akailash@google.com>
Change-Id: I86c2de977de052bfe9dcdc002dcbd9026601d0f3
(cherry picked from commit 65f3d09445699910947fd73b35979121b2bf07fb)
Merged-In:I86c2de977de052bfe9dcdc002dcbd9026601d0f3
This commit is contained in:
Akilesh Kailash 2022-01-25 07:05:31 +00:00 committed by Android Build Coastguard Worker
parent 73d1c56fba
commit f53f371806
2 changed files with 59 additions and 0 deletions

View File

@ -32,6 +32,7 @@
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <cutils/sockets.h>
#include <fs_avb/fs_avb.h>
#include <libsnapshot/snapshot.h>
#include <libsnapshot/snapuserd_client.h>
#include <private/android_filesystem_config.h>
@ -227,6 +228,56 @@ void SnapuserdSelinuxHelper::FinishTransition() {
}
}
/*
* Before starting init second stage, we will wait
* for snapuserd daemon to be up and running; bionic libc
* may read /system/etc/selinux/plat_property_contexts file
* before invoking main() function. This will happen if
* init initializes property during second stage. Any access
* to /system without snapuserd daemon will lead to a deadlock.
*
* Thus, we do a simple probe by reading system partition. This
* read will eventually be serviced by daemon confirming that
* daemon is up and running. Furthermore, we are still in the kernel
* domain and sepolicy has not been enforced yet. Thus, access
* to these device mapper block devices are ok even though
* we may see audit logs.
*/
bool SnapuserdSelinuxHelper::TestSnapuserdIsReady() {
std::string dev = "/dev/block/mapper/system"s + fs_mgr_get_slot_suffix();
android::base::unique_fd fd(open(dev.c_str(), O_RDONLY | O_DIRECT));
if (fd < 0) {
PLOG(ERROR) << "open " << dev << " failed";
return false;
}
void* addr;
ssize_t page_size = getpagesize();
if (posix_memalign(&addr, page_size, page_size) < 0) {
PLOG(ERROR) << "posix_memalign with page size " << page_size;
return false;
}
std::unique_ptr<void, decltype(&::free)> buffer(addr, ::free);
int iter = 0;
while (iter < 10) {
ssize_t n = TEMP_FAILURE_RETRY(pread(fd.get(), buffer.get(), page_size, 0));
if (n < 0) {
// Wait for sometime before retry
std::this_thread::sleep_for(100ms);
} else if (n == page_size) {
return true;
} else {
LOG(ERROR) << "pread returned: " << n << " from: " << dev << " expected: " << page_size;
}
iter += 1;
}
return false;
}
void SnapuserdSelinuxHelper::RelaunchFirstStageSnapuserd() {
auto fd = GetRamdiskSnapuserdFd();
if (!fd) {
@ -248,6 +299,13 @@ void SnapuserdSelinuxHelper::RelaunchFirstStageSnapuserd() {
setenv(kSnapuserdFirstStagePidVar, std::to_string(pid).c_str(), 1);
LOG(INFO) << "Relaunched snapuserd with pid: " << pid;
if (!TestSnapuserdIsReady()) {
PLOG(FATAL) << "snapuserd daemon failed to launch";
} else {
LOG(INFO) << "snapuserd daemon is up and running";
}
return;
}

View File

@ -51,6 +51,7 @@ class SnapuserdSelinuxHelper final {
private:
void RelaunchFirstStageSnapuserd();
void ExecSnapuserd();
bool TestSnapuserdIsReady();
std::unique_ptr<SnapshotManager> sm_;
BlockDevInitializer block_dev_init_;