New issue
Advanced search Search tips

Issue 684269 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Mar 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

attestationd: stopping leads to crash

Project Member Reported by apronin@chromium.org, Jan 24 2017

Issue description

Stopping attestationd leads to:

CRIT attestationd[1953]: Check failed: origin_thread_id_ == base::PlatformThread::CurrentId() (1993 vs. 1953)#012/usr/lib64/libbase-core-395517.so(base::debug::StackTrace::StackTrace()+0x13) [0x795b96a29d63]#012
 
Origin thread is the worker thread created by AttestationService::InitializeTask. The current thread is the main dbus server thread.
The reason is default_tpm_utility_->Initialize() is called from AttestationService::InitializeTask() on worker_thread. Inside it creates tpm_manager_thread_, trunks_utility_, etc.
These objects are destroyed in AttestationService::~AttestationService(), which is called on different thread - main dbus thread.
Bottomline: need to move initialization from worker thread to the thread that calls Initialize().
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/aosp/platform/system/tpm/+/6bc0da49ff76fbf33d8aad11b617667f33871801

commit 6bc0da49ff76fbf33d8aad11b617667f33871801
Author: Andrey Pronin <apronin@chromium.org>
Date: Sat Feb 18 11:11:01 2017

attestation: fix shutdown for dbus threads

Objects that communicate with trunksd and tpm_managerd over D-Bus
must be destroyed on the same thread that initialized them. Their
destructors call dbus::Bus::ShutdownAndBlock(), which require that
they are called on the same thread that created dbus::Bus objects.

That applies to:
 - tpm_manager::TpmOwnershipDBusProxy;
 - tpm_manager::TpmNvramDBusProxy;
 - trunks::TrunksDBusProxy
   (destructed in trunks::~TrunksFactoryImpl, in turn called from
    attestation::~TpmUtilityV2).

Destruct such objects in CleanUp() hooks of the threads that
created them. Doing it in these hooks, rather than posting a
special shutdown task to that thread from the owning object
destructor, ensures that these objects are not destructed while
there are tasks left in the thread's message queue that might
access the destructed objects.

BUG= chromium:684269 
TEST=Login, corp enrollment still succeeds. Unit tests pass.
     "stop attestationd" doesn't lead to a crash.

Change-Id: I2576eb4cf8260092adb4d54cb3aeb5f662e85010
Reviewed-on: https://chromium-review.googlesource.com/444213
Commit-Ready: Andrey Pronin <apronin@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[modify] https://crrev.com/6bc0da49ff76fbf33d8aad11b617667f33871801/attestation/server/attestation_service.cc
[modify] https://crrev.com/6bc0da49ff76fbf33d8aad11b617667f33871801/attestation/common/tpm_utility_v2.h
[modify] https://crrev.com/6bc0da49ff76fbf33d8aad11b617667f33871801/attestation/server/attestation_service.h
[modify] https://crrev.com/6bc0da49ff76fbf33d8aad11b617667f33871801/attestation/common/tpm_utility_v2.cc

Status: Fixed (was: Assigned)

Comment 6 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 7 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 8 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 9 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment