From 23e78269a327a374b3ab44c57f8e62adadf469e0 Mon Sep 17 00:00:00 2001 From: Yu-Han Yang Date: Tue, 11 Jun 2024 21:59:02 +0000 Subject: [PATCH 1/4] Fix flaky GNSS VTS In the original code, vector::erase() would invalidate iterators so we should move the thread object into the std::async() for the blocking operation. Bug: 344250967 Test: atest VtsHalGnssTargetTest Change-Id: I4cc82131bb070a37cb6ed9dbe9d7cccc7ab5ee89 --- gnss/aidl/default/GnssMeasurementInterface.cpp | 17 ++++++++++------- .../default/GnssNavigationMessageInterface.cpp | 17 ++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/gnss/aidl/default/GnssMeasurementInterface.cpp b/gnss/aidl/default/GnssMeasurementInterface.cpp index f3242138cb..db1b8c6bc5 100644 --- a/gnss/aidl/default/GnssMeasurementInterface.cpp +++ b/gnss/aidl/default/GnssMeasurementInterface.cpp @@ -146,15 +146,18 @@ void GnssMeasurementInterface::stop() { mIsActive = false; mGnss->setGnssMeasurementEnabled(false); mThreadBlocker.notify(); - for (auto iter = mThreads.begin(); iter != mThreads.end(); ++iter) { + for (auto iter = mThreads.begin(); iter != mThreads.end();) { if (iter->joinable()) { - mFutures.push_back(std::async(std::launch::async, [this, iter] { - iter->join(); - mThreads.erase(iter); - })); - } else { - mThreads.erase(iter); + // Store the thread object by value + std::thread threadToMove = std::move(*iter); + + mFutures.push_back(std::async(std::launch::async, + [threadToMove = std::move(threadToMove)]() mutable { + ALOGD("joining thread"); + threadToMove.join(); + })); } + iter = mThreads.erase(iter); } } diff --git a/gnss/aidl/default/GnssNavigationMessageInterface.cpp b/gnss/aidl/default/GnssNavigationMessageInterface.cpp index c262dc6b88..eb1d6556b6 100644 --- a/gnss/aidl/default/GnssNavigationMessageInterface.cpp +++ b/gnss/aidl/default/GnssNavigationMessageInterface.cpp @@ -90,15 +90,18 @@ void GnssNavigationMessageInterface::stop() { ALOGD("stop"); mIsActive = false; mThreadBlocker.notify(); - for (auto iter = mThreads.begin(); iter != mThreads.end(); ++iter) { + for (auto iter = mThreads.begin(); iter != mThreads.end();) { if (iter->joinable()) { - mFutures.push_back(std::async(std::launch::async, [this, iter] { - iter->join(); - mThreads.erase(iter); - })); - } else { - mThreads.erase(iter); + // Store the thread object by value + std::thread threadToMove = std::move(*iter); + + mFutures.push_back(std::async(std::launch::async, + [threadToMove = std::move(threadToMove)]() mutable { + ALOGD("joining thread"); + threadToMove.join(); + })); } + iter = mThreads.erase(iter); } } From 8659a96281f108259c047c1fd6e9fd181241ee7a Mon Sep 17 00:00:00 2001 From: lichen yu Date: Thu, 6 Jun 2024 03:40:43 +0000 Subject: [PATCH 2/4] Fix VtsHalSensorsV2_0TargetTest During VTS testing, when many sensors are quickly enabled, the load on the Sensorhub side will be too heavy, and then continue to send sensor enable instructions and Sensor Flush instructions. These instructions sent by the Kernel driver to the Sensorhub through inter-core communication cannot be responded to in a timely manner. After the Kernel side judges the timeout, it will determine that the enable fails; after the enable fails, it cannot continue to test the flush or return the expected flush data. ten sensors is tested as a group Bug: 339763843 Test: run vts -m VtsHalSensorsV2_0TargetTest (cherry picked from https://android-review.googlesource.com/q/commit:09952885e441bcf0d8f0bf0834ff46608b07e0d0) Merged-In: I6f6d36d1e3c98b85b412189c3f97163c0945a7ab Change-Id: I6f6d36d1e3c98b85b412189c3f97163c0945a7ab --- .../vts/2_X/VtsHalSensorsV2_XTargetTest.h | 70 +++++++++++-------- 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/sensors/common/vts/2_X/VtsHalSensorsV2_XTargetTest.h b/sensors/common/vts/2_X/VtsHalSensorsV2_XTargetTest.h index aa6e8814a2..b17ecb8df0 100644 --- a/sensors/common/vts/2_X/VtsHalSensorsV2_XTargetTest.h +++ b/sensors/common/vts/2_X/VtsHalSensorsV2_XTargetTest.h @@ -604,42 +604,54 @@ void SensorsHidlTest::runFlushTest(const std::vector& sensors, b EventCallback callback; getEnvironment()->registerCallback(&callback); - for (const SensorInfoType& sensor : sensors) { - // Configure and activate the sensor - batch(sensor.sensorHandle, sensor.maxDelay, 0 /* maxReportLatencyNs */); - activate(sensor.sensorHandle, activateSensor); + // 10 sensors per group + constexpr size_t kSensorsPerGroup = 10; + for (size_t sensorOffset = 0; sensorOffset < sensors.size(); + sensorOffset += kSensorsPerGroup) { + std::vector sensorGroup( + sensors.begin() + sensorOffset, + sensors.begin() + + std::min(sensorOffset + kSensorsPerGroup, sensors.size())); - // Flush the sensor - for (int32_t i = 0; i < flushCalls; i++) { + for (const SensorInfoType& sensor : sensorGroup) { + // Configure and activate the sensor + batch(sensor.sensorHandle, sensor.maxDelay, 0 /* maxReportLatencyNs */); + activate(sensor.sensorHandle, activateSensor); + + // Flush the sensor + for (int32_t i = 0; i < flushCalls; i++) { + SCOPED_TRACE(::testing::Message() + << "Flush " << i << "/" << flushCalls << ": " + << " handle=0x" << std::hex << std::setw(8) << std::setfill('0') + << sensor.sensorHandle << std::dec + << " type=" << static_cast(sensor.type) + << " name=" << sensor.name); + + Result flushResult = flush(sensor.sensorHandle); + EXPECT_EQ(flushResult, expectedResponse); + } + } + + // Wait up to one second for the flush events + callback.waitForFlushEvents(sensorGroup, flushCalls, milliseconds(1000) /* timeout */); + + // Deactivate all sensors after waiting for flush events so pending flush events are not + // abandoned by the HAL. + for (const SensorInfoType& sensor : sensorGroup) { + activate(sensor.sensorHandle, false); + } + + // Check that the correct number of flushes are present for each sensor + for (const SensorInfoType& sensor : sensorGroup) { SCOPED_TRACE(::testing::Message() - << "Flush " << i << "/" << flushCalls << ": " << " handle=0x" << std::hex << std::setw(8) << std::setfill('0') << sensor.sensorHandle << std::dec - << " type=" << static_cast(sensor.type) << " name=" << sensor.name); - - Result flushResult = flush(sensor.sensorHandle); - EXPECT_EQ(flushResult, expectedResponse); + << " type=" << static_cast(sensor.type) + << " name=" << sensor.name); + ASSERT_EQ(callback.getFlushCount(sensor.sensorHandle), expectedFlushCount); } } - - // Wait up to one second for the flush events - callback.waitForFlushEvents(sensors, flushCalls, milliseconds(1000) /* timeout */); - - // Deactivate all sensors after waiting for flush events so pending flush events are not - // abandoned by the HAL. - for (const SensorInfoType& sensor : sensors) { - activate(sensor.sensorHandle, false); - } getEnvironment()->unregisterCallback(); - - // Check that the correct number of flushes are present for each sensor - for (const SensorInfoType& sensor : sensors) { - SCOPED_TRACE(::testing::Message() - << " handle=0x" << std::hex << std::setw(8) << std::setfill('0') - << sensor.sensorHandle << std::dec << " type=" << static_cast(sensor.type) - << " name=" << sensor.name); - ASSERT_EQ(callback.getFlushCount(sensor.sensorHandle), expectedFlushCount); - } } TEST_P(SensorsHidlTest, FlushSensor) { From c2e34fcb91c155b5131aa3e362a46b58c41fc3c6 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Thu, 13 Jun 2024 13:32:08 +0000 Subject: [PATCH 3/4] Update OWNERS files for power and display services. I'm no longer on the team, so make sure any place I'm an explicit owner points to the generic owners file. Bug: 346974431 Change-Id: Icbcb3b2024f8adb9980c6128022965e8f9a87918 --- light/OWNERS | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/light/OWNERS b/light/OWNERS index d1da8a7850..9f312b071f 100644 --- a/light/OWNERS +++ b/light/OWNERS @@ -1,5 +1,3 @@ # Bug Component: 185877106 -michaelwr@google.com -santoscordon@google.com -philipjunker@google.com +file:platform/frameworks/base:/services/core/java/com/android/server/display/OWNERS \ No newline at end of file From d530f7e39bc4d2563080ad3fa4626662853ba36f Mon Sep 17 00:00:00 2001 From: Shikha Panwar Date: Thu, 13 Jun 2024 08:25:02 +0000 Subject: [PATCH 4/4] Vts: New dice_policy_builder api with `TargetEntry`. The policy building library changes in aosp/3125493, accordingly change the function call. This does not change the behaviour or test coverage of VTS. Test: atest VtsSecretkeeperTargetTest Bug: 291245237 Change-Id: I21a7b0abe5bf186893ec9a68bb080b41778d3313 --- .../secretkeeper/aidl/vts/secretkeeper_cli.rs | 26 ++++++++++--------- .../aidl/vts/secretkeeper_test_client.rs | 19 +++++++------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/security/secretkeeper/aidl/vts/secretkeeper_cli.rs b/security/secretkeeper/aidl/vts/secretkeeper_cli.rs index 377ed37b4e..9fbfb45e6e 100644 --- a/security/secretkeeper/aidl/vts/secretkeeper_cli.rs +++ b/security/secretkeeper/aidl/vts/secretkeeper_cli.rs @@ -25,7 +25,7 @@ use authgraph_core::traits::Sha256; use clap::{Args, Parser, Subcommand}; use coset::CborSerializable; use dice_policy_builder::{ - policy_for_dice_chain, CertIndex, ConstraintSpec, ConstraintType, MissingAction, + policy_for_dice_chain, ConstraintSpec, ConstraintType, MissingAction, TargetEntry, WILDCARD_FULL_ARRAY, }; @@ -131,33 +131,35 @@ impl SkClient { } /// Construct a sealing policy on the DICE chain with constraints: - /// 1. `ExactMatch` on `AUTHORITY_HASH` (non-optional). - /// 2. `ExactMatch` on `MODE` (non-optional). - /// 3. `GreaterOrEqual` on `SECURITY_VERSION` (optional). + /// 1. `ExactMatch` on `AUTHORITY_HASH` (non-optional) on all nodes. + /// 2. `ExactMatch` on `MODE` (non-optional) on all nodes. + /// 3. `GreaterOrEqual` on `SECURITY_VERSION` (optional) on all nodes. + /// 4. The DiceChainEntry corresponding to "AVB" contains SubcomponentDescriptor, for each of those: + /// a) GreaterOrEqual on SECURITY_VERSION (Required) + // b) ExactMatch on AUTHORITY_HASH (Required). fn sealing_policy(&self) -> Result> { let dice = self.dice_artifacts.explicit_key_dice_chain().context("extract explicit DICE chain")?; - let constraint_spec = [ + let constraint_spec = vec![ ConstraintSpec::new( ConstraintType::ExactMatch, vec![AUTHORITY_HASH], MissingAction::Fail, - CertIndex::All, + TargetEntry::All, ), ConstraintSpec::new( ConstraintType::ExactMatch, vec![MODE], MissingAction::Fail, - CertIndex::All, + TargetEntry::All, ), ConstraintSpec::new( ConstraintType::GreaterOrEqual, vec![CONFIG_DESC, SECURITY_VERSION], MissingAction::Ignore, - CertIndex::All, + TargetEntry::All, ), - // Constraints on sub components in the second last DiceChainEntry ConstraintSpec::new( ConstraintType::GreaterOrEqual, vec![ @@ -167,7 +169,7 @@ impl SkClient { SUBCOMPONENT_SECURITY_VERSION, ], MissingAction::Fail, - CertIndex::FromEnd(1), + TargetEntry::ByName("AVB".to_string()), ), ConstraintSpec::new( ConstraintType::ExactMatch, @@ -178,10 +180,10 @@ impl SkClient { SUBCOMPONENT_AUTHORITY_HASH, ], MissingAction::Fail, - CertIndex::FromEnd(1), + TargetEntry::ByName("AVB".to_string()), ), ]; - policy_for_dice_chain(dice, &constraint_spec) + policy_for_dice_chain(dice, constraint_spec) .unwrap() .to_vec() .context("serialize DICE policy") diff --git a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs index 595dc7a878..449a99a90c 100644 --- a/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs +++ b/security/secretkeeper/aidl/vts/secretkeeper_test_client.rs @@ -20,7 +20,7 @@ use authgraph_vts_test as ag_vts; use authgraph_boringssl as boring; use authgraph_core::key; use coset::{CborOrdering, CborSerializable, CoseEncrypt0, CoseKey}; -use dice_policy_builder::{CertIndex, ConstraintSpec, ConstraintType, MissingAction, WILDCARD_FULL_ARRAY, policy_for_dice_chain}; +use dice_policy_builder::{TargetEntry, ConstraintSpec, ConstraintType, MissingAction, WILDCARD_FULL_ARRAY, policy_for_dice_chain}; use rdroidtest::{ignore_if, rdroidtest}; use secretkeeper_client::dice::OwnedDiceArtifactsWithExplicitKey; use secretkeeper_client::{SkSession, Error as SkClientError}; @@ -312,30 +312,29 @@ fn assert_result_matches(res: Result, want: SecretkeeperError) { /// 1. ExactMatch on AUTHORITY_HASH (non-optional). /// 2. ExactMatch on MODE (non-optional). /// 3. GreaterOrEqual on SECURITY_VERSION (optional). -/// 4. The second last DiceChainEntry contain SubcomponentDescriptor, for each of those: +/// 4. The DiceChainEntry corresponding to "AVB" contains SubcomponentDescriptor, for each of those: /// a) GreaterOrEqual on SECURITY_VERSION (Required) // b) ExactMatch on AUTHORITY_HASH (Required). fn sealing_policy(dice: &[u8]) -> Vec { - let constraint_spec = [ + let constraint_spec = vec![ ConstraintSpec::new( ConstraintType::ExactMatch, vec![AUTHORITY_HASH], MissingAction::Fail, - CertIndex::All, + TargetEntry::All, ), ConstraintSpec::new( ConstraintType::ExactMatch, vec![MODE], MissingAction::Fail, - CertIndex::All, + TargetEntry::All, ), ConstraintSpec::new( ConstraintType::GreaterOrEqual, vec![CONFIG_DESC, SECURITY_VERSION], MissingAction::Ignore, - CertIndex::All, + TargetEntry::All, ), - // Constraints on sub components in the second last DiceChainEntry ConstraintSpec::new( ConstraintType::GreaterOrEqual, vec![ @@ -345,7 +344,7 @@ fn sealing_policy(dice: &[u8]) -> Vec { SUBCOMPONENT_SECURITY_VERSION, ], MissingAction::Fail, - CertIndex::FromEnd(1), + TargetEntry::ByName("AVB".to_string()), ), ConstraintSpec::new( ConstraintType::ExactMatch, @@ -356,11 +355,11 @@ fn sealing_policy(dice: &[u8]) -> Vec { SUBCOMPONENT_AUTHORITY_HASH, ], MissingAction::Fail, - CertIndex::FromEnd(1), + TargetEntry::ByName("AVB".to_string()), ), ]; - policy_for_dice_chain(dice, &constraint_spec).unwrap().to_vec().unwrap() + policy_for_dice_chain(dice, constraint_spec).unwrap().to_vec().unwrap() } /// Perform AuthGraph key exchange, returning the session keys and session ID.