From 2a245e16f70635c0489efc204d8a76b935f4a0df Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Tue, 7 Apr 2020 16:54:14 -0700 Subject: [PATCH] Camera: Fix possible invalid metadata access Camera metadata entries can become invalid after updates. Avoid using the possibly released entry pointer when extending camera characteristics with dynamic depth tags. Additionally simplify rotate and crop region checks. Bug: 152240541 Test: cameraservice_test Change-Id: I416739ed7e128f4ec94353ec2938b9bf226be182 --- .../common/CameraProviderManager.cpp | 13 ++-- .../camera/libcameraservice/tests/Android.mk | 2 + .../tests/CameraProviderManagerTest.cpp | 63 ++++++++++++++++++- .../tests/RotateAndCropMapperTest.cpp | 7 +-- 4 files changed, 72 insertions(+), 13 deletions(-) diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp index cc369fac3e..10b653ecf9 100644 --- a/services/camera/libcameraservice/common/CameraProviderManager.cpp +++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp @@ -841,13 +841,6 @@ status_t CameraProviderManager::ProviderInfo::DeviceInfo3::addDynamicDepthTags() itDuration++; itSize++; } - c.update(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_STREAM_CONFIGURATIONS, - dynamicDepthEntries.data(), dynamicDepthEntries.size()); - c.update(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_MIN_FRAME_DURATIONS, - dynamicDepthMinDurationEntries.data(), dynamicDepthMinDurationEntries.size()); - c.update(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_STALL_DURATIONS, - dynamicDepthStallDurationEntries.data(), dynamicDepthStallDurationEntries.size()); - std::vector supportedChTags; supportedChTags.reserve(chTags.count + 3); supportedChTags.insert(supportedChTags.end(), chTags.data.i32, @@ -855,6 +848,12 @@ status_t CameraProviderManager::ProviderInfo::DeviceInfo3::addDynamicDepthTags() supportedChTags.push_back(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_STREAM_CONFIGURATIONS); supportedChTags.push_back(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_MIN_FRAME_DURATIONS); supportedChTags.push_back(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_STALL_DURATIONS); + c.update(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_STREAM_CONFIGURATIONS, + dynamicDepthEntries.data(), dynamicDepthEntries.size()); + c.update(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_MIN_FRAME_DURATIONS, + dynamicDepthMinDurationEntries.data(), dynamicDepthMinDurationEntries.size()); + c.update(ANDROID_DEPTH_AVAILABLE_DYNAMIC_DEPTH_STALL_DURATIONS, + dynamicDepthStallDurationEntries.data(), dynamicDepthStallDurationEntries.size()); c.update(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS, supportedChTags.data(), supportedChTags.size()); diff --git a/services/camera/libcameraservice/tests/Android.mk b/services/camera/libcameraservice/tests/Android.mk index fa5d69e2c4..3ead71557f 100644 --- a/services/camera/libcameraservice/tests/Android.mk +++ b/services/camera/libcameraservice/tests/Android.mk @@ -48,6 +48,8 @@ LOCAL_C_INCLUDES += \ LOCAL_CFLAGS += -Wall -Wextra -Werror +LOCAL_SANITIZE := address + LOCAL_MODULE:= cameraservice_test LOCAL_COMPATIBILITY_SUITE := device-tests LOCAL_MODULE_TAGS := tests diff --git a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp index a8f6889a1d..855b5abb4f 100644 --- a/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp +++ b/services/camera/libcameraservice/tests/CameraProviderManagerTest.cpp @@ -40,8 +40,15 @@ using android::hardware::camera::provider::V2_5::DeviceState; */ struct TestDeviceInterface : public device::V3_2::ICameraDevice { std::vector mDeviceNames; + android::hardware::hidl_vec mCharacteristics; + + TestDeviceInterface(std::vector deviceNames, + android::hardware::hidl_vec chars) : + mDeviceNames(deviceNames), mCharacteristics(chars) {} + TestDeviceInterface(std::vector deviceNames) : mDeviceNames(deviceNames) {} + using getResourceCost_cb = std::function; @@ -58,8 +65,7 @@ struct TestDeviceInterface : public device::V3_2::ICameraDevice { const hardware::hidl_vec& cameraCharacteristics)>; hardware::Return getCameraCharacteristics( getCameraCharacteristics_cb _hidl_cb) override { - hardware::hidl_vec cameraCharacteristics; - _hidl_cb(Status::OK, cameraCharacteristics); + _hidl_cb(Status::OK, mCharacteristics); return hardware::Void(); } @@ -100,6 +106,13 @@ struct TestICameraProvider : virtual public provider::V2_5::ICameraProvider { mDeviceInterface(new TestDeviceInterface(devices)), mVendorTagSections (vendorSection) {} + TestICameraProvider(const std::vector &devices, + const hardware::hidl_vec &vendorSection, + android::hardware::hidl_vec chars) : + mDeviceNames(devices), + mDeviceInterface(new TestDeviceInterface(devices, chars)), + mVendorTagSections (vendorSection) {} + virtual hardware::Return setCallback( const sp& callbacks) override { mCalledCounter[SET_CALLBACK]++; @@ -243,6 +256,52 @@ struct TestStatusListener : public CameraProviderManager::StatusListener { void onNewProviderRegistered() override {} }; +TEST(CameraProviderManagerTest, InitializeDynamicDepthTest) { + std::vector deviceNames; + deviceNames.push_back("device@3.2/test/0"); + hardware::hidl_vec vendorSection; + status_t res; + sp providerManager = new CameraProviderManager(); + sp statusListener = new TestStatusListener(); + TestInteractionProxy serviceProxy; + + android::hardware::hidl_vec chars; + CameraMetadata meta; + int32_t charKeys[] = { ANDROID_DEPTH_DEPTH_IS_EXCLUSIVE, + ANDROID_DEPTH_AVAILABLE_DEPTH_STREAM_CONFIGURATIONS }; + meta.update(ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS, charKeys, + sizeof(charKeys) / sizeof(charKeys[0])); + uint8_t depthIsExclusive = ANDROID_DEPTH_DEPTH_IS_EXCLUSIVE_FALSE; + meta.update(ANDROID_DEPTH_DEPTH_IS_EXCLUSIVE, &depthIsExclusive, 1); + int32_t sizes[] = { HAL_PIXEL_FORMAT_BLOB, + 640, 480, ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT }; + meta.update(ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, sizes, + sizeof(sizes) / sizeof(sizes[0])); + sizes[0] = HAL_PIXEL_FORMAT_Y16; + meta.update(ANDROID_DEPTH_AVAILABLE_DEPTH_STREAM_CONFIGURATIONS, sizes, + sizeof(sizes) / sizeof(sizes[0])); + int64_t durations[] = { HAL_PIXEL_FORMAT_BLOB, 640, 480, 0 }; + meta.update(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS, durations, + sizeof(durations) / sizeof(durations[0])); + meta.update(ANDROID_SCALER_AVAILABLE_STALL_DURATIONS, durations, + sizeof(durations) / sizeof(durations[0])); + durations[0]= HAL_PIXEL_FORMAT_Y16; + meta.update(ANDROID_DEPTH_AVAILABLE_DEPTH_MIN_FRAME_DURATIONS, durations, + sizeof(durations) / sizeof(durations[0])); + meta.update(ANDROID_DEPTH_AVAILABLE_DEPTH_STALL_DURATIONS, durations, + sizeof(durations) / sizeof(durations[0])); + camera_metadata_t* metaBuffer = const_cast(meta.getAndLock()); + chars.setToExternal(reinterpret_cast(metaBuffer), + get_camera_metadata_size(metaBuffer)); + + sp provider = new TestICameraProvider(deviceNames, + vendorSection, chars); + serviceProxy.setProvider(provider); + + res = providerManager->initialize(statusListener, &serviceProxy); + ASSERT_EQ(res, OK) << "Unable to initialize provider manager"; +} + TEST(CameraProviderManagerTest, InitializeTest) { std::vector deviceNames; deviceNames.push_back("device@3.2/test/0"); diff --git a/services/camera/libcameraservice/tests/RotateAndCropMapperTest.cpp b/services/camera/libcameraservice/tests/RotateAndCropMapperTest.cpp index c638d40bc8..3c187cd3a7 100644 --- a/services/camera/libcameraservice/tests/RotateAndCropMapperTest.cpp +++ b/services/camera/libcameraservice/tests/RotateAndCropMapperTest.cpp @@ -38,10 +38,9 @@ using ::testing::Le; #define EXPECT_EQUAL_WITHIN_N(vec, array, N, msg) \ { \ - std::vector vec_diff; \ - std::transform(vec.begin(), vec.end(), array, \ - std::back_inserter(vec_diff), std::minus()); \ - EXPECT_THAT(vec_diff, Each(AllOf(Ge(-N), Le(N)))) << msg; \ + for (size_t i = 0; i < vec.size(); i++) { \ + EXPECT_THAT(vec[i] - array[i], AllOf(Ge(-N), Le(N))) << msg " failed at index:" << i; \ + } \ } int32_t testActiveArray[] = {100, 100, 4000, 3000};