Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 12d34177 authored by Dennis Shen's avatar Dennis Shen Committed by Gerrit Code Review
Browse files

Merge "aconfig: update aconfig_storage_read_api" into main

parents d14e178b bb13bbb0

tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp

+0 −5
Original line number Diff line number Diff line
@@ -74,11 +74,6 @@ static Result<MappedStorageFile> map_storage_file(std::string const& file) { 
  if (fstat(fd, &fd_stat) < 0) {

    return Error() << "fstat failed";

  }



  if ((fd_stat.st_mode & (S_IWUSR | S_IWGRP | S_IWOTH)) != 0) {

    return Error() << "cannot map writeable file";

  }



  size_t file_size = fd_stat.st_size;



  void* const map_result = mmap(nullptr, file_size, PROT_READ, MAP_SHARED, fd, 0);

tools/aconfig/aconfig_storage_read_api/src/lib.rs

+20 −14
Original line number Diff line number Diff line
@@ -64,11 +64,17 @@ pub const STORAGE_LOCATION_FILE: &str = "/metadata/aconfig/boot/available_storag 
/// \input container: the flag package container

/// \input file_type: stoarge file type enum

/// \return a result of read only mapped file

pub fn get_mapped_storage_file(

///

/// # Safety

///

/// The memory mapped file may have undefined behavior if there are writes to this

/// file after being mapped. Ensure no writes can happen to this file while this

/// mapping stays alive.

pub unsafe fn get_mapped_storage_file(

    container: &str,

    file_type: StorageFileType,

) -> Result<Mmap, AconfigStorageError> {

    crate::mapped_file::get_mapped_file(STORAGE_LOCATION_FILE, container, file_type)

    unsafe { crate::mapped_file::get_mapped_file(STORAGE_LOCATION_FILE, container, file_type) }

}



/// Get package start offset for flags.
@@ -311,10 +317,10 @@ mod tests { 
    use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;

    use tempfile::NamedTempFile;



    fn create_test_storage_files(read_only: bool) -> [NamedTempFile; 4] {

        let package_map = copy_to_temp_file("./tests/package.map", read_only).unwrap();

        let flag_map = copy_to_temp_file("./tests/flag.map", read_only).unwrap();

        let flag_val = copy_to_temp_file("./tests/flag.val", read_only).unwrap();

    fn create_test_storage_files() -> [NamedTempFile; 4] {

        let package_map = copy_to_temp_file("./tests/package.map").unwrap();

        let flag_map = copy_to_temp_file("./tests/flag.map").unwrap();

        let flag_val = copy_to_temp_file("./tests/flag.val").unwrap();



        let text_proto = format!(

            r#"
@@ -338,10 +344,11 @@ files {{ 
    #[test]

    // this test point locks down flag package offset query

    fn test_package_offset_query() {

        let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);

        let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();

        let pb_file_path = pb_file.path().display().to_string();

        let package_mapped_file =

            get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap();

        let package_mapped_file = unsafe {

            get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap()

        };



        let package_offset =

            get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_1")
@@ -368,10 +375,10 @@ files {{ 
    #[test]

    // this test point locks down flag offset query

    fn test_flag_offset_query() {

        let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);

        let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();

        let pb_file_path = pb_file.path().display().to_string();

        let flag_mapped_file =

            get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap();

            unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap() };



        let baseline = vec![

            (0, "enabled_ro", 1u16),
@@ -393,10 +400,10 @@ files {{ 
    #[test]

    // this test point locks down flag offset query

    fn test_flag_value_query() {

        let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);

        let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();

        let pb_file_path = pb_file.path().display().to_string();

        let flag_value_file =

            get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap();

            unsafe { get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap() };

        let baseline: Vec<bool> = vec![false; 8];

        for (offset, expected_value) in baseline.into_iter().enumerate() {

            let flag_value = get_boolean_flag_value(&flag_value_file, offset as u32).unwrap();
@@ -407,7 +414,6 @@ files {{ 
    #[test]

    // this test point locks down flag storage file version number query api

    fn test_storage_version_query() {

        let _ro_files = create_test_storage_files(true);

        assert_eq!(get_storage_file_version("./tests/package.map").unwrap(), 1);

        assert_eq!(get_storage_file_version("./tests/flag.map").unwrap(), 1);

        assert_eq!(get_storage_file_version("./tests/flag.val").unwrap(), 1);

tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs

+26 −60
Original line number Diff line number Diff line
@@ -14,7 +14,7 @@ 
 * limitations under the License.

 */



use std::fs::{self, File};

use std::fs::File;

use std::io::{BufReader, Read};



use anyhow::anyhow;
@@ -56,26 +56,16 @@ fn find_container_storage_location( 
    Err(StorageFileNotFound(anyhow!("Storage file does not exist for {}", container)))

}



/// Verify the file is read only and then map it

fn verify_read_only_and_map(file_path: &str) -> Result<Mmap, AconfigStorageError> {

    // ensure file has read only permission

    let perms = fs::metadata(file_path).unwrap().permissions();

    if !perms.readonly() {

        return Err(MapFileFail(anyhow!("fail to map non read only storage file {}", file_path)));

    }



/// Get the read only memory mapping of a storage file

///

/// # Safety

///

/// The memory mapped file may have undefined behavior if there are writes to this

/// file after being mapped. Ensure no writes can happen to this file while this

/// mapping stays alive.

unsafe fn map_file(file_path: &str) -> Result<Mmap, AconfigStorageError> {

    let file = File::open(file_path)

        .map_err(|errmsg| FileReadFail(anyhow!("Failed to open file {}: {}", file_path, errmsg)))?;



    // SAFETY:

    //

    // Mmap constructors are unsafe as it would have undefined behaviors if the file

    // is modified after mapped (https://docs.rs/memmap2/latest/memmap2/struct.Mmap.html).

    //

    // We either have to make this api unsafe or ensure that the file will not be modified

    // which means it is read only. Here in the code, we check explicitly that the file

    // being mapped must only have read permission, otherwise, error out, thus making sure

    // it is safe.

    unsafe {

        let mapped_file = Mmap::map(&file).map_err(|errmsg| {

            MapFileFail(anyhow!("fail to map storage file {}: {}", file_path, errmsg))
@@ -85,16 +75,22 @@ fn verify_read_only_and_map(file_path: &str) -> Result<Mmap, AconfigStorageError 
}



/// Get a mapped storage file given the container and file type

pub fn get_mapped_file(

///

/// # Safety

///

/// The memory mapped file may have undefined behavior if there are writes to this

/// file after being mapped. Ensure no writes can happen to this file while this

/// mapping stays alive.

pub unsafe fn get_mapped_file(

    location_pb_file: &str,

    container: &str,

    file_type: StorageFileType,

) -> Result<Mmap, AconfigStorageError> {

    let files_location = find_container_storage_location(location_pb_file, container)?;

    match file_type {

        StorageFileType::PackageMap => verify_read_only_and_map(files_location.package_map()),

        StorageFileType::FlagMap => verify_read_only_and_map(files_location.flag_map()),

        StorageFileType::FlagVal => verify_read_only_and_map(files_location.flag_val()),

        StorageFileType::PackageMap => unsafe { map_file(files_location.package_map()) },

        StorageFileType::FlagMap => unsafe { map_file(files_location.flag_map()) },

        StorageFileType::FlagVal => unsafe { map_file(files_location.flag_val()) },

    }

}
@@ -155,14 +151,15 @@ files { 
        let mut content = Vec::new();

        opened_file.read_to_end(&mut content).unwrap();



        let mmaped_file = get_mapped_file(location_pb_file, "system", file_type).unwrap();

        let mmaped_file =

            unsafe { get_mapped_file(location_pb_file, "system", file_type).unwrap() };

        assert_eq!(mmaped_file[..], content[..]);

    }



    fn create_test_storage_files(read_only: bool) -> [NamedTempFile; 4] {

        let package_map = copy_to_temp_file("./tests/package.map", read_only).unwrap();

        let flag_map = copy_to_temp_file("./tests/flag.map", read_only).unwrap();

        let flag_val = copy_to_temp_file("./tests/package.map", read_only).unwrap();

    fn create_test_storage_files() -> [NamedTempFile; 4] {

        let package_map = copy_to_temp_file("./tests/package.map").unwrap();

        let flag_map = copy_to_temp_file("./tests/flag.map").unwrap();

        let flag_val = copy_to_temp_file("./tests/package.map").unwrap();



        let text_proto = format!(

            r#"
@@ -185,7 +182,7 @@ files {{ 


    #[test]

    fn test_mapped_file_contents() {

        let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);

        let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files();

        let pb_file_path = pb_file.path().display().to_string();

        map_and_verify(

            &pb_file_path,
@@ -203,35 +200,4 @@ files {{ 
            &flag_val.path().display().to_string(),

        );

    }



    #[test]

    fn test_map_non_read_only_file() {

        let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(false);

        let pb_file_path = pb_file.path().display().to_string();

        let error =

            get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap_err();

        assert_eq!(

            format!("{:?}", error),

            format!(

                "MapFileFail(fail to map non read only storage file {})",

                package_map.path().display()

            )

        );

        let error = get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap_err();

        assert_eq!(

            format!("{:?}", error),

            format!(

                "MapFileFail(fail to map non read only storage file {})",

                flag_map.path().display()

            )

        );

        let error = get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap_err();

        assert_eq!(

            format!("{:?}", error),

            format!(

                "MapFileFail(fail to map non read only storage file {})",

                flag_val.path().display()

            )

        );

    }

}

tools/aconfig/aconfig_storage_read_api/src/test_utils.rs

+1 −7
Original line number Diff line number Diff line
@@ -19,14 +19,8 @@ use std::fs; 
use tempfile::NamedTempFile;



/// Create temp file copy

pub(crate) fn copy_to_temp_file(source_file: &str, read_only: bool) -> Result<NamedTempFile> {

pub(crate) fn copy_to_temp_file(source_file: &str) -> Result<NamedTempFile> {

    let file = NamedTempFile::new()?;

    fs::copy(source_file, file.path())?;

    if read_only {

        let file_name = file.path().display().to_string();

        let mut perms = fs::metadata(file_name).unwrap().permissions();

        perms.set_readonly(true);

        fs::set_permissions(file.path(), perms.clone()).unwrap();

    }

    Ok(file)

}

tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp

+4 −28
Original line number Diff line number Diff line
@@ -33,7 +33,7 @@ namespace private_api = aconfig_storage::private_internal_api; 


class AconfigStorageTest : public ::testing::Test {

 protected:

  Result<std::string> copy_to_ro_temp_file(std::string const& source_file) {

  Result<std::string> copy_to_temp_file(std::string const& source_file) {

    auto temp_file = std::string(std::tmpnam(nullptr));

    auto content = std::string();

    if (!ReadFileToString(source_file, &content)) {
@@ -42,9 +42,6 @@ class AconfigStorageTest : public ::testing::Test { 
    if (!WriteStringToFile(content, temp_file)) {

      return Error() << "failed to copy file: " << source_file;

    }

    if (chmod(temp_file.c_str(), S_IRUSR | S_IRGRP | S_IROTH) == -1) {

      return Error() << "failed to make file read only";

    }

    return temp_file;

  }
@@ -71,9 +68,9 @@ class AconfigStorageTest : public ::testing::Test { 


  void SetUp() override {

    auto const test_dir = android::base::GetExecutableDirectory();

    package_map = *copy_to_ro_temp_file(test_dir + "/package.map");

    flag_map = *copy_to_ro_temp_file(test_dir + "/flag.map");

    flag_val = *copy_to_ro_temp_file(test_dir + "/flag.val");

    package_map = *copy_to_temp_file(test_dir + "/package.map");

    flag_map = *copy_to_temp_file(test_dir + "/flag.map");

    flag_val = *copy_to_temp_file(test_dir + "/flag.val");

    storage_record_pb = *write_storage_location_pb_file(

        package_map, flag_map, flag_val);

  }
@@ -113,27 +110,6 @@ TEST_F(AconfigStorageTest, test_none_exist_storage_file_mapping) { 
            "Unable to find storage files for container vendor");

}



/// Negative test to lock down the error when mapping a writeable storage file

TEST_F(AconfigStorageTest, test_writable_storage_file_mapping) {

  ASSERT_TRUE(chmod(package_map.c_str(), 0666) != -1);

  auto mapped_file = private_api::get_mapped_file_impl(

      storage_record_pb, "system", api::StorageFileType::package_map);

  ASSERT_FALSE(mapped_file.ok());

  ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file");



  ASSERT_TRUE(chmod(flag_map.c_str(), 0666) != -1);

  mapped_file = private_api::get_mapped_file_impl(

      storage_record_pb, "system", api::StorageFileType::flag_map);

  ASSERT_FALSE(mapped_file.ok());

  ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file");



  ASSERT_TRUE(chmod(flag_val.c_str(), 0666) != -1);

  mapped_file = private_api::get_mapped_file_impl(

      storage_record_pb, "system", api::StorageFileType::flag_val);

  ASSERT_FALSE(mapped_file.ok());

  ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file");

}



/// Test to lock down storage package offset query api

TEST_F(AconfigStorageTest, test_package_offset_query) {

  auto mapped_file = private_api::get_mapped_file_impl(