Feature im sm

sonar-project.properties

@@ -38,11 +38,12 @@ sonar.coverage.exclusions=**/tests/**
 # overridden by the build system before this header is included, which constexpr/enum can't do.
 #
 # Ignore cpp:M23_163 (MISRA C++:2023 13.1.1, classes should not be inherited virtually) in the CM
-# database interface only: DatabaseItf lists all storage interfaces the CM database implements,
+# and SM database interfaces and the SM image manager storage interface only: DatabaseItf lists all
+# storage interfaces the CM or SM database implements,
 # and the application database implements the image manager storage in a common base class, so
 # StorageItf is inherited virtually to keep a single instance of it in the diamond. StorageItf is
 # a pure interface without data.
-sonar.issue.ignore.multicriteria=e1,e2,e3,e4,e5,e6,e7,e8,e9,e10
+sonar.issue.ignore.multicriteria=e1,e2,e3,e4,e5,e6,e7,e8,e9,e10,e11,e12
 sonar.issue.ignore.multicriteria.e1.ruleKey=cpp:M23_356
 sonar.issue.ignore.multicriteria.e1.resourceKey=**/*
 sonar.issue.ignore.multicriteria.e2.ruleKey=cpp:S6022
@@ -63,5 +64,9 @@ sonar.issue.ignore.multicriteria.e9.ruleKey=cpp:S7032
 sonar.issue.ignore.multicriteria.e9.resourceKey=**/*
 sonar.issue.ignore.multicriteria.e10.ruleKey=cpp:M23_163
 sonar.issue.ignore.multicriteria.e10.resourceKey=**/cm/database/itf/database.hpp
+sonar.issue.ignore.multicriteria.e11.ruleKey=cpp:M23_163
+sonar.issue.ignore.multicriteria.e11.resourceKey=**/sm/database/itf/database.hpp
+sonar.issue.ignore.multicriteria.e12.ruleKey=cpp:M23_163
+sonar.issue.ignore.multicriteria.e12.resourceKey=**/sm/imagemanager/itf/storage.hpp
 
 sonar.ws.timeout=600

src/core/common/imagemanager/imagemanager.cpp

@@ -296,7 +296,7 @@
     return ErrorEnum::eNone;
 }
 
 Error ImageManager::Init(const Config& config, BlobInfoProviderItf& blobInfoProvider, StorageItf& storage,
     spaceallocator::SpaceAllocatorItf& spaceAllocator, fs::FileInfoProviderItf& fileInfoProvider,
     oci::OCISpecItf& ociSpec, fs::FSPlatformItf& fsPlatform, AllocatorItf& allocator)
 {
@@ -984,7 +984,7 @@
     StaticString<oci::cDigestLen> hash;
 
     // The file which doesn't match a valid digest is not a blob and is removed.
     if (digest.IsEmpty() || !ParseDigest(digest, algorithm, hash).IsNone()) {
         LOG_WRN() << "Remove invalid blob file" << Log::Field("path", path);
 
         return RemoveBlob(path, digest);
@@ -1422,7 +1422,7 @@
     return ErrorEnum::eNone;
 }
 
 Error ImageManager::InstallManifestBlobs(const InstallItemInfo& itemInfo, const oci::ContentDescriptor& descriptor)
 {
     LOG_DBG() << "Install manifest" << Log::Field("digest", descriptor.mDigest);
 
@@ -1465,7 +1465,10 @@
                 return AOS_ERROR_WRAP(err);
             }
 
-            if (imageConfig->mRootfs.mDiffIDs.Size() < manifest->mLayers.Size()) {
+            // A component config may have no rootfs: its layers are kept without diff IDs as for the empty config.
+            if (imageConfig->mRootfs.mDiffIDs.IsEmpty()) {
+                imageConfig.Reset();
+            } else if (imageConfig->mRootfs.mDiffIDs.Size() < manifest->mLayers.Size()) {
                 return AOS_ERROR_WRAP(Error(ErrorEnum::eInvalidArgument, "diff ID's size mismatch"));
             }
         }

src/core/common/imagemanager/tests/imagemanager.cpp

@@ -1319,6 +1319,25 @@ TEST_F(CommonImageManagerInstallTest, InstallEmptyConfig)
     EXPECT_EQ(item->mState, ItemStateEnum::eInstalled);
 }
 
+TEST_F(CommonImageManagerInstallTest, InstallConfigWithoutDiffIDs)
+{
+    // A component may have an image config without rootfs, so its layers have no diff IDs.
+    const auto manifest = AddManifest(1, 1, 0);
+
+    CreateValidBlobs(manifest);
+
+    auto err = mImageManager.Install(MakeInstallInfo(manifest.mManifest));
+    ASSERT_TRUE(err.IsNone()) << tests::utils::ErrorToStr(err);
+
+    const auto calls = mImageManager.GetValidateLayerCalls();
+    ASSERT_EQ(calls.size(), 1);
+    EXPECT_TRUE(calls[0].mDiffDigest.empty());
+
+    auto item = GetStoredItem(cTestItemID, cTestVersion);
+    ASSERT_TRUE(item.has_value());
+    EXPECT_EQ(item->mState, ItemStateEnum::eInstalled);
+}
+
 TEST_F(CommonImageManagerInstallTest, InstallMissingDiffID)
 {
     const auto manifest = AddManifest(1, 2, 1);

src/core/common/spaceallocator/spaceallocator.hpp

@@ -13,18 +13,24 @@
 #include <core/common/tools/map.hpp>
 #include <core/common/tools/memory.hpp>
 #include <core/common/tools/thread.hpp>
+#include <core/common/tools/utils.hpp>
 
 #include "itf/spaceallocator.hpp"
 
 namespace aos::spaceallocator {
 
 struct Partition;
 
+/**
+ * Outdated item ID len: item ID or blob digest.
+ */
+constexpr auto cOutdatedItemIDLen = Max(cIDLen, oci::cDigestLen);
+
 /**
  * Outdated item.
  */
 struct OutdatedItem {
-    StaticString<cIDLen>                    mID;
+    StaticString<cOutdatedItemIDLen>        mID;
     StaticString<cVersionLen>               mVersion;
     SpaceAllocatorItf*                      mSpaceAllocator {};
     StaticFunction<cDefaultFunctionMaxSize> mFreeCallback;
@@ -565,8 +571,14 @@ class SpaceAllocator : public SpaceAllocatorItf, public SpaceAllocatorStorage {
 
         OutdatedItem item;
 
-        item.mID             = id;
-        item.mVersion        = version;
+        if (auto err = item.mID.Assign(id); !err.IsNone()) {
+            return AOS_ERROR_WRAP(Error(err, "outdated item id too long"));
+        }
+
+        if (auto err = item.mVersion.Assign(version); !err.IsNone()) {
+            return AOS_ERROR_WRAP(Error(err, "outdated item version too long"));
+        }
+
         item.mPartition      = mPartition;
         item.mRemover        = mRemover;
         item.mTimestamp      = timestamp;

src/core/common/spaceallocator/tests/spaceallocator.cpp

@@ -789,6 +789,52 @@ TEST_F(SpaceallocatorTest, ReAddOutdatedItemUpdatesExisting)
     ASSERT_TRUE(mSpaceAllocator.Close().IsNone());
 }
 
+TEST_F(SpaceallocatorTest, OutdatedItemDigestID)
+{
+    SpaceAllocator<5> mSpaceAllocator;
+
+    EXPECT_CALL(mPlatformFS, GetMountPoint(mPath))
+        .WillOnce(Return(RetWithError<StaticString<cFilePathLen>>(mMountPoint, ErrorEnum::eNone)));
+
+    ASSERT_TRUE(mSpaceAllocator.Init(mAllocator, mPath, mPlatformFS, mLimit, &mRemover).IsNone());
+
+    const auto digest = "sha256:" + std::string(64, 'a');
+
+    ASSERT_GT(digest.size(), cIDLen);
+    ASSERT_TRUE(mSpaceAllocator.AddOutdatedItem(digest.c_str(), "", Time::Now()).IsNone());
+
+    EXPECT_CALL(mRemover, RemoveItem(String(digest.c_str()), String("")))
+        .WillOnce(Return(RetWithError<size_t>(32 * cKilobyte, ErrorEnum::eNone)));
+
+    auto [size, err] = mSpaceAllocator.RemoveOutdatedItem();
+    ASSERT_TRUE(err.IsNone());
+    EXPECT_EQ(size, 32 * cKilobyte);
+
+    ASSERT_TRUE(mSpaceAllocator.Close().IsNone());
+}
+
+TEST_F(SpaceallocatorTest, OutdatedItemTooLongID)
+{
+    SpaceAllocator<5> mSpaceAllocator;
+
+    EXPECT_CALL(mPlatformFS, GetMountPoint(mPath))
+        .WillOnce(Return(RetWithError<StaticString<cFilePathLen>>(mMountPoint, ErrorEnum::eNone)));
+
+    ASSERT_TRUE(mSpaceAllocator.Init(mAllocator, mPath, mPlatformFS, mLimit, &mRemover).IsNone());
+
+    const auto id = std::string(cOutdatedItemIDLen + 1, 'a');
+
+    EXPECT_FALSE(mSpaceAllocator.AddOutdatedItem(id.c_str(), "", Time::Now()).IsNone());
+
+    EXPECT_CALL(mRemover, RemoveItem(_, _)).Times(0);
+
+    auto [size, err] = mSpaceAllocator.RemoveOutdatedItem();
+    EXPECT_EQ(err, ErrorEnum::eNotFound);
+    EXPECT_EQ(size, 0U);
+
+    ASSERT_TRUE(mSpaceAllocator.Close().IsNone());
+}
+
 TEST_F(SpaceallocatorTest, RemoveOutdatedItemNoItems)
 {
     SpaceAllocator<5> mSpaceAllocator;

src/core/common/tools/fs.cpp

@@ -37,6 +37,16 @@
     }
 }
 
+int StatNoFollow(const char* path, struct stat* st)
+{
+#if defined(__ZEPHYR__)
+    // Zephyr doesn't provide lstat, its file systems don't support symlinks.
+    return stat(path, st);
+#else
+    return lstat(path, st);
+#endif
+}
+
 } // namespace
 
 /***********************************************************************************************************************
@@ -93,7 +103,7 @@
         auto        path = JoinPath(mRoot, entry->d_name);
         struct stat entryStat;
 
-        if (stat(path.CStr(), &entryStat) == -1) {
+        if (StatNoFollow(path.CStr(), &entryStat) == -1) {
             return false;
         }
 
@@ -591,11 +601,11 @@
 {
     struct stat st;
 
-    if (auto ret = stat(path.CStr(), &st); ret != 0) {
+    if (auto ret = StatNoFollow(path.CStr(), &st); ret != 0) {
         return {0, AOS_ERROR_WRAP(Error(errno))};
     }
 
-    if (S_ISREG(st.st_mode)) {
+    if (!S_ISDIR(st.st_mode)) {
         return {static_cast<size_t>(st.st_size)};
     }
 
@@ -617,7 +627,12 @@
         while (dirIt.Next()) {
             const auto fullPath = JoinPath(dirIt.GetRootPath(), dirIt->mPath);
 
-            if (dirIt->mIsDir) {
+            // Symlinks are not followed: they may point outside the path or to the content counted elsewhere.
+            if (auto ret = StatNoFollow(fullPath.CStr(), &st); ret != 0) {
+                return {0, AOS_ERROR_WRAP(Error(errno))};
+            }
+
+            if (S_ISDIR(st.st_mode)) {
                 if (auto err = dirIterators->EmplaceBack(fullPath); !err.IsNone()) {
                     return {0, AOS_ERROR_WRAP(err)};
                 }
@@ -627,10 +642,6 @@
                 break;
             }
 
-            if (auto ret = stat(fullPath.CStr(), &st); ret != 0) {
-                return {0, AOS_ERROR_WRAP(Error(ret))};
-            }
-
             size += st.st_size;
         }
 

src/core/common/tools/tests/fs.cpp

@@ -711,6 +711,34 @@ TEST_F(FSTest, CalculateSize)
     EXPECT_EQ(size, 0);
 }
 
+TEST_F(FSTest, CalculateSizeDoesNotFollowSymlinks)
+{
+    const auto walkDirRoot = cBaseTestDir / "calculate-symlinks-test";
+    const auto outsideDir  = cBaseTestDir / "calculate-symlinks-outside";
+
+    ASSERT_TRUE(fs::MakeDirAll(walkDirRoot.c_str()).IsNone());
+    ASSERT_TRUE(fs::MakeDirAll(outsideDir.c_str()).IsNone());
+
+    CreateFile((walkDirRoot / "f.txt").c_str(), std::string(1024, 'a').c_str(), 0444);
+    CreateFile((outsideDir / "big.txt").c_str(), std::string(4096, 'b').c_str(), 0444);
+
+    const std::vector<std::pair<std::string, std::filesystem::path>> links = {
+        {outsideDir.string(), walkDirRoot / "dir-link"},
+        {(outsideDir / "big.txt").string(), walkDirRoot / "file-link"},
+        {"/not-existing-target", walkDirRoot / "dangling-link"},
+    };
+
+    size_t expectedSize = 1024;
+
+    for (const auto& [target, link] : links) {
+        std::filesystem::create_symlink(target, link);
+
+        expectedSize += target.size();
+    }
+
+    EXPECT_EQ(fs::CalculateSize(mAllocator, walkDirRoot.c_str()), RetWithError<size_t>(expectedSize));
+}
+
 TEST_F(FSTest, GetFileMTime)
 {
     const auto testFile = cBaseTestDir / "modify-time-test.txt";

src/core/sm/database/database.md

@@ -3,7 +3,11 @@
 The database module provides persistent storage for various SM modules. It implements storage interfaces for the
 following modules:
 
-- [aos::sm::imagemanager::StorageItf](../imagemanager/itf/storage.hpp) - image manager storage interface;
+- [aos::common::imagemanager::StorageItf](../../common/imagemanager/itf/storage.hpp) - common image manager storage
+  interface, inherited virtually so that the implementation may share it with a common image manager storage base
+  class;
+- [aos::sm::imagemanager::StorageItf](../imagemanager/itf/storage.hpp) - SM image manager storage interface, which
+  extends the common one;
 - [aos::sm::launcher::StorageItf](../launcher/itf/storage.hpp) - launcher storage interface.
 - [aos::sm::networkmanager::StorageItf](../networkmanager/itf/storage.hpp) - network manager storage interface;
 
@@ -13,6 +17,10 @@ classDiagram
         <<interface>>
     }
 
+    class CommonImageManagerStorageItf ["aos::common::imagemanager::StorageItf"] {
+        <<interface>>
+    }
+
     class ImageManagerStorageItf ["aos::sm::imagemanager::StorageItf"] {
         <<interface>>
     }
@@ -25,7 +33,9 @@ classDiagram
         <<interface>>
     }
 
+    DatabaseItf ..|> CommonImageManagerStorageItf
     DatabaseItf ..|> ImageManagerStorageItf
+    ImageManagerStorageItf ..|> CommonImageManagerStorageItf
     DatabaseItf ..|> LauncherStorageItf
     DatabaseItf ..|> NetworkManagerStorageItf
 ```

src/core/sm/database/itf/database.hpp

@@ -7,6 +7,7 @@
 #ifndef AOS_CORE_SM_DATABASE_ITF_DATABASE_HPP_
 #define AOS_CORE_SM_DATABASE_ITF_DATABASE_HPP_
 
+#include <core/common/imagemanager/itf/storage.hpp>
 #include <core/sm/imagemanager/itf/storage.hpp>
 #include <core/sm/launcher/itf/storage.hpp>
 #include <core/sm/networkmanager/itf/storage.hpp>
@@ -20,7 +21,10 @@ namespace aos::sm::database {
 /**
  * Database interface.
  */
-class DatabaseItf : public imagemanager::StorageItf, public launcher::StorageItf, public networkmanager::StorageItf { };
+class DatabaseItf : public virtual common::imagemanager::StorageItf,
+                    public imagemanager::StorageItf,
+                    public launcher::StorageItf,
+                    public networkmanager::StorageItf { };
 
 /** @}*/
 

src/core/sm/imagemanager/CMakeLists.txt

@@ -20,13 +20,13 @@ set(SOURCES imagemanager.cpp)
 # Headers
 # ######################################################################################################################
 
-set(HEADERS itf/blobinfoprovider.hpp itf/imagemanager.hpp itf/iteminfoprovider.hpp config.hpp imagemanager.hpp)
+set(HEADERS itf/imagehandler.hpp itf/imagemanager.hpp itf/storage.hpp imagemanager.hpp)
 
 # ######################################################################################################################
 # Libraries
 # ######################################################################################################################
 
-set(LIBRARIES aos::core::common::tools)
+set(LIBRARIES aos::core::common::imagemanager aos::core::common::tools)
 
 # ######################################################################################################################
 # Target