update_engine: AddressSanitizer PostinstallerRunnerAction RunAsRoot leak fix
The reason why this change is required is because of
|FileDescriptorWatcher::Controller| within |PostinstallRunnerAction|. If
the delay is too short (previously 10ms) it was possible for the posted
task within the |FileDescriptorWatcher::Controller| to be
present after that of the task which stops the processor. In order to
mitigate this issue, the process of stopping the processor should be a
|PostDelayedTask()| instead of a direct call in stopping the processor
to ensure the processor stops after |Watcher::StartWatching()| happens.
Within |FileDescriptorWatcher::Controller| it states:
"If the MessageLoopForIO is deleted before Watcher::StartWatching()
runs, |watcher_| is leaked."
BUG=chromium:989749
TEST=FEATURES="nostrip" ./build_packages --board amd64-generic --withdebugsymbols
TEST=FEATURES="test nostrip -splitdebug" USE="asan debug" CFLAGS="-g -O2" CXXFLAGS="-g -O2" emerge-amd64-generic update_engine
TEST=sudo cat /build/amd64-generic/tmp/portage/chromeos-base/update_engine-9999/temp/asan_logs/asan.10 | asan_symbolize.py -s /build/amd64-generic -d > /tmp/asan.log
Change-Id: I88f0a86686830553fea150d0188b2851753c2f94
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/1796613
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/payload_consumer/postinstall_runner_action_unittest.cc b/payload_consumer/postinstall_runner_action_unittest.cc
index 04c81fa..84f2c2c 100644
--- a/payload_consumer/postinstall_runner_action_unittest.cc
+++ b/payload_consumer/postinstall_runner_action_unittest.cc
@@ -142,7 +142,14 @@
base::TimeDelta::FromMilliseconds(10));
} else {
CHECK(processor_);
- processor_->StopProcessing();
+ // Must |PostDelayedTask()| here to be safe that |FileDescriptorWatcher|
+ // doesn't leak memory, do not directly call |StopProcessing()|.
+ loop_.PostDelayedTask(
+ FROM_HERE,
+ base::Bind(
+ [](ActionProcessor* processor) { processor->StopProcessing(); },
+ base::Unretained(processor_)),
+ base::TimeDelta::FromMilliseconds(100));
}
}