From e336b9219c206318f866398df09b9216b281761c Mon Sep 17 00:00:00 2001 From: Devin Moore Date: Thu, 28 Mar 2024 21:05:13 +0000 Subject: [PATCH 1/9] init_kill_services_test: don't always try to kill hwservicemanager When HIDL is not supported, hwservicemanager will not be running and may not even be installed on the device. Ignore-AOSP-First: Disabling HIDL internally first. Will cherry-pick these test CLs after. Test: init_kill_services_test Bug: 218588089 Change-Id: Iae41e35e4669dd62c99ab9f138fc419be2f5fa29 --- init/test_kill_services/Android.bp | 5 ++++- init/test_kill_services/init_kill_services_test.cpp | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/init/test_kill_services/Android.bp b/init/test_kill_services/Android.bp index 37361a813..ada87d804 100644 --- a/init/test_kill_services/Android.bp +++ b/init/test_kill_services/Android.bp @@ -10,7 +10,10 @@ package { cc_test { name: "init_kill_services_test", srcs: ["init_kill_services_test.cpp"], - shared_libs: ["libbase"], + shared_libs: [ + "libbase", + "libhidlbase", + ], test_suites: ["general-tests"], // TODO(b/153565474): switch back to auto-generation diff --git a/init/test_kill_services/init_kill_services_test.cpp b/init/test_kill_services/init_kill_services_test.cpp index 510ad8aee..3af92bb55 100644 --- a/init/test_kill_services/init_kill_services_test.cpp +++ b/init/test_kill_services/init_kill_services_test.cpp @@ -18,15 +18,20 @@ #include #include +#include #include using ::android::base::GetProperty; using ::android::base::SetProperty; using ::android::base::WaitForProperty; +using ::android::hardware::isHidlSupported; using std::literals::chrono_literals::operator""s; void ExpectKillingServiceRecovers(const std::string& service_name) { + if (!isHidlSupported() && service_name == "hwservicemanager") { + GTEST_SKIP() << "No HIDL support on device so hwservicemanager will not be running"; + } LOG(INFO) << "before we say hi to " << service_name << ", I can't have apexd around!"; // b/280514080 - servicemanager will restart apexd, and apexd will restart the From 916ed66659ee2778906a81aa6fc681e2e82d8785 Mon Sep 17 00:00:00 2001 From: Satish Yalla Date: Tue, 2 Apr 2024 06:45:44 +0000 Subject: [PATCH 2/9] Revert "init_kill_services_test: don't always try to kill hwserv..." Revert submission 26722372-nomo_hidl Reason for revert: Reverted changes: /q/submissionid:26722372-nomo_hidl Change-Id: I6fe936e3c27f528020b36def91c541f37e638b98 --- init/test_kill_services/Android.bp | 5 +---- init/test_kill_services/init_kill_services_test.cpp | 5 ----- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/init/test_kill_services/Android.bp b/init/test_kill_services/Android.bp index ada87d804..37361a813 100644 --- a/init/test_kill_services/Android.bp +++ b/init/test_kill_services/Android.bp @@ -10,10 +10,7 @@ package { cc_test { name: "init_kill_services_test", srcs: ["init_kill_services_test.cpp"], - shared_libs: [ - "libbase", - "libhidlbase", - ], + shared_libs: ["libbase"], test_suites: ["general-tests"], // TODO(b/153565474): switch back to auto-generation diff --git a/init/test_kill_services/init_kill_services_test.cpp b/init/test_kill_services/init_kill_services_test.cpp index 3af92bb55..510ad8aee 100644 --- a/init/test_kill_services/init_kill_services_test.cpp +++ b/init/test_kill_services/init_kill_services_test.cpp @@ -18,20 +18,15 @@ #include #include -#include #include using ::android::base::GetProperty; using ::android::base::SetProperty; using ::android::base::WaitForProperty; -using ::android::hardware::isHidlSupported; using std::literals::chrono_literals::operator""s; void ExpectKillingServiceRecovers(const std::string& service_name) { - if (!isHidlSupported() && service_name == "hwservicemanager") { - GTEST_SKIP() << "No HIDL support on device so hwservicemanager will not be running"; - } LOG(INFO) << "before we say hi to " << service_name << ", I can't have apexd around!"; // b/280514080 - servicemanager will restart apexd, and apexd will restart the From 3e0d4b9f77567ab02a7c7892bfedbed21937360a Mon Sep 17 00:00:00 2001 From: Donnie Pollitz Date: Mon, 22 Apr 2024 18:57:40 +0000 Subject: [PATCH 3/9] Merge "storageproxyd: Add arguments for storage mapping and max file" into main am: 63ef65c138 am: 0d49c512be Original change: https://android-review.googlesource.com/c/platform/system/core/+/2994377 Bug: 324989972 Signed-off-by: Automerger Merge Worker (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:b041bf19a182c311424f9647f7a06da25b0fe8b5) Merged-In: I67261a520bef86d2cf8f13e1c0f9731c4ed0b6f6 Change-Id: I67261a520bef86d2cf8f13e1c0f9731c4ed0b6f6 --- trusty/storage/proxy/proxy.c | 73 ++++++++- trusty/storage/proxy/storage.c | 287 +++++++++++++++++++++++++++++---- trusty/storage/proxy/storage.h | 11 +- 3 files changed, 340 insertions(+), 31 deletions(-) diff --git a/trusty/storage/proxy/proxy.c b/trusty/storage/proxy/proxy.c index 67e935e39..6cb72d597 100644 --- a/trusty/storage/proxy/proxy.c +++ b/trusty/storage/proxy/proxy.c @@ -41,9 +41,13 @@ static const char* ss_data_root; static const char* trusty_devname; static const char* rpmb_devname; static const char* ss_srv_name = STORAGE_DISK_PROXY_PORT; +static const char* max_file_size_from; static enum dev_type dev_type = MMC_RPMB; +/* List head for storage mapping, elements added at init, and never removed */ +static struct storage_mapping_node* storage_mapping_head; + static enum dev_type parse_dev_type(const char* dev_type_name) { if (!strcmp(dev_type_name, "mmc")) { return MMC_RPMB; @@ -58,17 +62,61 @@ static enum dev_type parse_dev_type(const char* dev_type_name) { } } -static const char* _sopts = "hp:d:r:t:"; +static int parse_and_append_file_mapping(const char* file_mapping) { + if (file_mapping == NULL) { + ALOGE("Provided file mapping is null\n"); + return -1; + } + char* file_mapping_dup = strdup(file_mapping); + if (file_mapping_dup == NULL) { + ALOGE("Couldn't duplicate string: %s\n", file_mapping); + return -1; + } + const char* file_name = strtok(file_mapping_dup, ":"); + if (file_name == NULL) { + ALOGE("No file name found\n"); + return -1; + } + const char* backing_storage = strtok(NULL, ":"); + if (backing_storage == NULL) { + ALOGE("No backing storage found\n"); + return -1; + } + + struct storage_mapping_node* new_node = malloc(sizeof(struct storage_mapping_node)); + if (new_node == NULL) { + ALOGE("Couldn't allocate additional storage_mapping_node\n"); + return -1; + } + *new_node = (struct storage_mapping_node){.file_name = file_name, + .backing_storage = backing_storage, + .next = storage_mapping_head, + .fd = -1}; + storage_mapping_head = new_node; + return 0; +} + +static const char* _sopts = "hp:d:r:t:m:f:"; static const struct option _lopts[] = {{"help", no_argument, NULL, 'h'}, {"trusty_dev", required_argument, NULL, 'd'}, {"data_path", required_argument, NULL, 'p'}, {"rpmb_dev", required_argument, NULL, 'r'}, {"dev_type", required_argument, NULL, 't'}, + {"max_file_size_from", required_argument, NULL, 'm'}, + {"file_storage_mapping", required_argument, NULL, 'f'}, {0, 0, 0, 0}}; static void show_usage_and_exit(int code) { - ALOGE("usage: storageproxyd -d -p -r -t \n"); + ALOGE("usage: storageproxyd -d -p -r -t [-m " + "] [-f :]\n"); ALOGE("Available dev types: mmc, virt\n"); + ALOGE("-f = Maps secure storage files like `0` and `persist/0`\n" + "to block devices. Storageproxyd will handle creating the\n" + "appropriate symlinks in the root datapath.\n"); + ALOGE("-m = Specifies the max size constraint for file backed storages.\n" + "The constraint is chosen by giving a file, this allows for passing a\n" + "block device for which a max file size can be queried. File based\n" + "storages will be constrained to that size as well.\n"); exit(code); } @@ -187,6 +235,7 @@ static int proxy_loop(void) { static void parse_args(int argc, char* argv[]) { int opt; int oidx = 0; + int rc = 0; while ((opt = getopt_long(argc, argv, _sopts, _lopts, &oidx)) != -1) { switch (opt) { @@ -210,6 +259,18 @@ static void parse_args(int argc, char* argv[]) { } break; + case 'f': + rc = parse_and_append_file_mapping(optarg); + if (rc < 0) { + ALOGE("Failed to parse file mapping: %s\n", optarg); + show_usage_and_exit(EXIT_FAILURE); + } + break; + + case 'm': + max_file_size_from = strdup(optarg); + break; + default: ALOGE("unrecognized option (%c):\n", opt); show_usage_and_exit(EXIT_FAILURE); @@ -225,6 +286,12 @@ static void parse_args(int argc, char* argv[]) { ALOGI("storage data root: %s\n", ss_data_root); ALOGI("trusty dev: %s\n", trusty_devname); ALOGI("rpmb dev: %s\n", rpmb_devname); + ALOGI("File Mappings: \n"); + const struct storage_mapping_node* curr = storage_mapping_head; + for (; curr != NULL; curr = curr->next) { + ALOGI("\t%s -> %s\n", curr->file_name, curr->backing_storage); + } + ALOGI("max file size from: %s\n", max_file_size_from ? max_file_size_from : "(unset)"); } int main(int argc, char* argv[]) { @@ -252,7 +319,7 @@ int main(int argc, char* argv[]) { ABinderProcess_startThreadPool(); /* initialize secure storage directory */ - rc = storage_init(ss_data_root); + rc = storage_init(ss_data_root, storage_mapping_head, max_file_size_from); if (rc < 0) return EXIT_FAILURE; /* open rpmb device */ diff --git a/trusty/storage/proxy/storage.c b/trusty/storage/proxy/storage.c index 8c8edb779..1dab93d9c 100644 --- a/trusty/storage/proxy/storage.c +++ b/trusty/storage/proxy/storage.c @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#include #include #include #include @@ -39,16 +40,20 @@ #define ALTERNATE_DATA_DIR "alternate/" /* Maximum file size for filesystem backed storage (i.e. not block dev backed storage) */ -#define MAX_FILE_SIZE (0x10000000000) +static size_t max_file_size = 0x10000000000; enum sync_state { SS_UNUSED = -1, - SS_CLEAN = 0, - SS_DIRTY = 1, + SS_CLEAN = 0, + SS_DIRTY = 1, + SS_CLEAN_NEED_SYMLINK = 2, }; static const char *ssdir_name; +/* List head for storage mapping, elements added at init, and never removed */ +static struct storage_mapping_node* storage_mapping_head; + /* * Property set to 1 after we have opened a file under ssdir_name. The backing * files for both TD and TDP are currently located under /data/vendor/ss and can @@ -75,24 +80,103 @@ static struct { uint8_t data[MAX_READ_SIZE]; } read_rsp; -static uint32_t insert_fd(int open_flags, int fd) -{ +static uint32_t insert_fd(int open_flags, int fd, struct storage_mapping_node* node) { uint32_t handle = fd; if (handle < FD_TBL_SIZE) { - fd_state[fd] = SS_CLEAN; /* fd clean */ - if (open_flags & O_TRUNC) { - fd_state[fd] = SS_DIRTY; /* set fd dirty */ - } + fd_state[fd] = SS_CLEAN; /* fd clean */ + if (open_flags & O_TRUNC) { + assert(node == NULL); + fd_state[fd] = SS_DIRTY; /* set fd dirty */ + } + + if (node != NULL) { + fd_state[fd] = SS_CLEAN_NEED_SYMLINK; + } } else { ALOGW("%s: untracked fd %u\n", __func__, fd); if (open_flags & (O_TRUNC | O_CREAT)) { fs_state = SS_DIRTY; } } + + if (node != NULL) { + node->fd = fd; + } + return handle; } +static void clear_fd_symlink_status(uint32_t handle, struct storage_mapping_node* entry) { + /* Always clear FD, in case fd is not in FD_TBL */ + entry->fd = -1; + + if (handle >= FD_TBL_SIZE) { + ALOGE("%s: untracked fd=%u\n", __func__, handle); + return; + } + + if (fd_state[handle] == SS_CLEAN_NEED_SYMLINK) { + fd_state[handle] = SS_CLEAN; + } +} + +static struct storage_mapping_node* get_pending_symlink_mapping(uint32_t handle) { + /* Fast lookup failure, is it in FD TBL */ + if (handle < FD_TBL_SIZE && fd_state[handle] != SS_CLEAN_NEED_SYMLINK) { + return NULL; + } + + /* Go find our mapping */ + struct storage_mapping_node* curr = storage_mapping_head; + for (; curr != NULL; curr = curr->next) { + if (curr->fd == handle) { + return curr; + } + } + + /* Safety check: state inconsistent if we get here with handle inside table range */ + assert(handle >= FD_TBL_SIZE); + + return NULL; +}; + +static int possibly_symlink_and_clear_mapping(uint32_t handle) { + struct storage_mapping_node* entry = get_pending_symlink_mapping(handle); + if (entry == NULL) { + /* No mappings pending */ + return 0; + } + + /* Create full path */ + char* path = NULL; + int rc = asprintf(&path, "%s/%s", ssdir_name, entry->file_name); + if (rc < 0) { + ALOGE("%s: asprintf failed\n", __func__); + return -1; + } + + /* Try and setup the symlinking */ + ALOGI("Creating symlink %s->%s\n", path, entry->backing_storage); + rc = symlink(entry->backing_storage, path); + if (rc < 0) { + ALOGE("%s: error symlinking %s->%s (%s)\n", __func__, path, entry->backing_storage, + strerror(errno)); + free(path); + return rc; + } + free(path); + + clear_fd_symlink_status(handle, entry); + + return rc; +} + +static bool is_pending_symlink(uint32_t handle) { + struct storage_mapping_node* entry = get_pending_symlink_mapping(handle); + return entry != NULL; +} + static int lookup_fd(uint32_t handle, bool dirty) { if (dirty) { @@ -107,6 +191,12 @@ static int lookup_fd(uint32_t handle, bool dirty) static int remove_fd(uint32_t handle) { + /* Cleanup fd in symlink mapping if it exists */ + struct storage_mapping_node* entry = get_pending_symlink_mapping(handle); + if (entry != NULL) { + entry->fd = -1; + } + if (handle < FD_TBL_SIZE) { fd_state[handle] = SS_UNUSED; /* set to uninstalled */ } @@ -247,11 +337,73 @@ static void sync_parent(const char* path, struct watcher* watcher) { watch_progress(watcher, "done syncing parent"); } +static struct storage_mapping_node* get_storage_mapping_entry(const char* source) { + struct storage_mapping_node* curr = storage_mapping_head; + for (; curr != NULL; curr = curr->next) { + if (!strcmp(source, curr->file_name)) { + ALOGI("Found backing file %s for %s\n", curr->backing_storage, source); + return curr; + } + } + return NULL; +} + +static bool is_backing_storage_mapped(const char* source) { + const struct storage_mapping_node* curr = storage_mapping_head; + for (; curr != NULL; curr = curr->next) { + if (!strcmp(source, curr->backing_storage)) { + ALOGI("Backed storage mapping exists for %s\n", curr->backing_storage); + return true; + } + } + return false; +} + +/* Attempts to open a backed file, if mapped, without creating the symlink. Symlink will be created + * later on the first write. This allows us to continue reporting zero read sizes until the first + * write. */ +static int open_possibly_mapped_file(const char* short_path, const char* full_path, int open_flags, + struct storage_mapping_node** entry) { + /* See if mapping exists, report upstream if there is no mapping. */ + struct storage_mapping_node* mapping_entry = get_storage_mapping_entry(short_path); + if (mapping_entry == NULL) { + return TEMP_FAILURE_RETRY(open(full_path, open_flags, S_IRUSR | S_IWUSR)); + } + + /* Check for existence of root path, we don't allow mappings during early boot */ + struct stat buf = {0}; + if (stat(ssdir_name, &buf) != 0) { + ALOGW("Root path not accessible yet, refuse to open mappings for now.\n"); + return -1; + } + + /* We don't support exclusive opening of mapped files */ + if (open_flags & O_EXCL) { + ALOGE("Requesting exclusive open on backed storage isn't supported: %s\n", full_path); + return -1; + } + + /* Try and open mapping file */ + open_flags &= ~(O_CREAT | O_EXCL); + ALOGI("%s Attempting to open mapped file: %s\n", __func__, mapping_entry->backing_storage); + int fd = + TEMP_FAILURE_RETRY(open(mapping_entry->backing_storage, open_flags, S_IRUSR | S_IWUSR)); + if (fd < 0) { + ALOGE("%s Failed to open mapping file: %s\n", __func__, mapping_entry->backing_storage); + return -1; + } + + /* Let caller know which entry we used for opening */ + *entry = mapping_entry; + return fd; +} + int storage_file_open(struct storage_msg* msg, const void* r, size_t req_len, struct watcher* watcher) { char* path = NULL; const struct storage_file_open_req *req = r; struct storage_file_open_resp resp = {0}; + struct storage_mapping_node* mapping_entry = NULL; if (req_len < sizeof(*req)) { ALOGE("%s: invalid request length (%zd < %zd)\n", @@ -321,14 +473,18 @@ int storage_file_open(struct storage_msg* msg, const void* r, size_t req_len, if (req->flags & STORAGE_FILE_OPEN_CREATE_EXCLUSIVE) { /* create exclusive */ open_flags |= O_CREAT | O_EXCL; - rc = TEMP_FAILURE_RETRY(open(path, open_flags, S_IRUSR | S_IWUSR)); + + /* Look for and attempt opening a mapping, else just do normal open. */ + rc = open_possibly_mapped_file(req->name, path, open_flags, &mapping_entry); } else { /* try open first */ rc = TEMP_FAILURE_RETRY(open(path, open_flags, S_IRUSR | S_IWUSR)); if (rc == -1 && errno == ENOENT) { /* then try open with O_CREATE */ open_flags |= O_CREAT; - rc = TEMP_FAILURE_RETRY(open(path, open_flags, S_IRUSR | S_IWUSR)); + + /* Look for and attempt opening a mapping, else just do normal open. */ + rc = open_possibly_mapped_file(req->name, path, open_flags, &mapping_entry); } } @@ -356,7 +512,7 @@ int storage_file_open(struct storage_msg* msg, const void* r, size_t req_len, /* at this point rc contains storage file fd */ msg->result = STORAGE_NO_ERROR; - resp.handle = insert_fd(open_flags, rc); + resp.handle = insert_fd(open_flags, rc, mapping_entry); ALOGV("%s: \"%s\": fd = %u: handle = %d\n", __func__, path, rc, resp.handle); @@ -433,6 +589,14 @@ int storage_file_write(struct storage_msg* msg, const void* r, size_t req_len, goto err_response; } + /* Handle any delayed symlinking for this handle if any */ + rc = possibly_symlink_and_clear_mapping(req->handle); + if (rc < 0) { + ALOGE("Failed to symlink storage\n"); + msg->result = STORAGE_ERR_GENERIC; + goto err_response; + } + int fd = lookup_fd(req->handle, true); watch_progress(watcher, "writing"); if (write_with_retry(fd, &req->data[0], req_len - sizeof(*req), @@ -479,6 +643,14 @@ int storage_file_read(struct storage_msg* msg, const void* r, size_t req_len, goto err_response; } + /* If this handle has a delayed symlink we should report 0 size reads until first write occurs + */ + if (is_pending_symlink(req->handle)) { + ALOGI("Pending symlink: Forcing read result 0.\n"); + msg->result = STORAGE_NO_ERROR; + return ipc_respond(msg, &read_rsp, sizeof(read_rsp.hdr)); + } + int fd = lookup_fd(req->handle, false); watch_progress(watcher, "reading"); ssize_t read_res = read_with_retry(fd, read_rsp.hdr.data, req->size, @@ -592,7 +764,7 @@ int storage_file_get_max_size(struct storage_msg* msg, const void* r, size_t req goto err_response; } } else { - max_size = MAX_FILE_SIZE; + max_size = max_file_size; } resp.max_size = max_size; @@ -603,17 +775,78 @@ err_response: return ipc_respond(msg, NULL, 0); } -int storage_init(const char *dirname) -{ +int determine_max_file_size(const char* max_file_size_from) { + /* Use default if none passed in */ + if (max_file_size_from == NULL) { + ALOGI("No max file source given, continuing to use default: 0x%lx\n", max_file_size); + return 0; + } + + /* Check that max_file_size_from is part of our mapping list. */ + if (!is_backing_storage_mapped(max_file_size_from)) { + ALOGE("%s: file doesn't match mapped storages (filename=%s)\n", __func__, + max_file_size_from); + return -1; + } + + ALOGI("Using %s to determine max file size.\n", max_file_size_from); + + /* Error if max file size source not found, possible misconfig. */ + struct stat buf = {0}; + int rc = stat(max_file_size_from, &buf); + if (rc < 0) { + ALOGE("%s: error stat'ing file (filename=%s): %s\n", __func__, max_file_size_from, + strerror(errno)); + return -1; + } + + /* Currently only support block device as max file size source */ + if ((buf.st_mode & S_IFMT) != S_IFBLK) { + ALOGE("Unsupported max file size source type: %d\n", buf.st_mode); + return -1; + } + + ALOGI("%s is a block device, determining block device size\n", max_file_size_from); + uint64_t max_size = 0; + int fd = TEMP_FAILURE_RETRY(open(max_file_size_from, O_RDONLY | O_NONBLOCK)); + if (fd < 0) { + ALOGE("%s: failed to open backing file %s for ioctl: %s\n", __func__, max_file_size_from, + strerror(errno)); + return -1; + } + rc = ioctl(fd, BLKGETSIZE64, &max_size); + if (rc < 0) { + ALOGE("%s: error calling ioctl on file (fd=%d): %s\n", __func__, fd, strerror(errno)); + close(fd); + return -1; + } + close(fd); + max_file_size = max_size; + + ALOGI("Using 0x%lx as max file size\n", max_file_size); + return 0; +} + +int storage_init(const char* dirname, struct storage_mapping_node* mappings, + const char* max_file_size_from) { /* If there is an active DSU image, use the alternate fs mode. */ alternate_mode = is_gsi_running(); fs_state = SS_CLEAN; for (uint i = 0; i < FD_TBL_SIZE; i++) { - fd_state[i] = SS_UNUSED; /* uninstalled */ + fd_state[i] = SS_UNUSED; /* uninstalled */ } ssdir_name = dirname; + + storage_mapping_head = mappings; + + /* Set the max file size based on incoming configuration */ + int rc = determine_max_file_size(max_file_size_from); + if (rc < 0) { + return rc; + } + return 0; } @@ -623,17 +856,17 @@ int storage_sync_checkpoint(struct watcher* watcher) { watch_progress(watcher, "sync fd table"); /* sync fd table and reset it to clean state first */ for (uint fd = 0; fd < FD_TBL_SIZE; fd++) { - if (fd_state[fd] == SS_DIRTY) { - if (fs_state == SS_CLEAN) { - /* need to sync individual fd */ - rc = fsync(fd); - if (rc < 0) { - ALOGE("fsync for fd=%d failed: %s\n", fd, strerror(errno)); - return rc; - } - } - fd_state[fd] = SS_CLEAN; /* set to clean */ - } + if (fd_state[fd] == SS_DIRTY) { + if (fs_state == SS_CLEAN) { + /* need to sync individual fd */ + rc = fsync(fd); + if (rc < 0) { + ALOGE("fsync for fd=%d failed: %s\n", fd, strerror(errno)); + return rc; + } + } + fd_state[fd] = SS_CLEAN; /* set to clean */ + } } /* check if we need to sync all filesystems */ diff --git a/trusty/storage/proxy/storage.h b/trusty/storage/proxy/storage.h index f29fdf2e8..6dbfe3706 100644 --- a/trusty/storage/proxy/storage.h +++ b/trusty/storage/proxy/storage.h @@ -21,6 +21,14 @@ /* Defined in watchdog.h */ struct watcher; +/* Is used for managing alternate backing storage, generally will be a block device. */ +struct storage_mapping_node { + struct storage_mapping_node* next; + const char* file_name; + const char* backing_storage; + int fd; +}; + int storage_file_delete(struct storage_msg* msg, const void* req, size_t req_len, struct watcher* watcher); @@ -45,6 +53,7 @@ int storage_file_set_size(struct storage_msg* msg, const void* req, size_t req_l int storage_file_get_max_size(struct storage_msg* msg, const void* req, size_t req_len, struct watcher* watcher); -int storage_init(const char* dirname); +int storage_init(const char* dirname, struct storage_mapping_node* head, + const char* max_file_size_from); int storage_sync_checkpoint(struct watcher* watcher); From a474b174ff98a31fb19bfb9b339c9732f2c5c822 Mon Sep 17 00:00:00 2001 From: Treehugger Robot Date: Mon, 22 Apr 2024 22:48:49 +0000 Subject: [PATCH 4/9] Merge "storageproxyd: Fix x86 builds" into main am: bbdc19b7e1 am: 2b029e300b Original change: https://android-review.googlesource.com/c/platform/system/core/+/3053402 Bug: 324989972 Signed-off-by: Automerger Merge Worker (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:60f55841c5573a466fad716c283156aad1e369c2) Merged-In: Ibf2e73ec1b08bfb44a6b92e11376b6b59555c8d6 Change-Id: Ibf2e73ec1b08bfb44a6b92e11376b6b59555c8d6 --- trusty/storage/proxy/storage.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/trusty/storage/proxy/storage.c b/trusty/storage/proxy/storage.c index 1dab93d9c..6d0c61671 100644 --- a/trusty/storage/proxy/storage.c +++ b/trusty/storage/proxy/storage.c @@ -40,7 +40,7 @@ #define ALTERNATE_DATA_DIR "alternate/" /* Maximum file size for filesystem backed storage (i.e. not block dev backed storage) */ -static size_t max_file_size = 0x10000000000; +static uint64_t max_file_size = 0x10000000000; enum sync_state { SS_UNUSED = -1, @@ -778,7 +778,8 @@ err_response: int determine_max_file_size(const char* max_file_size_from) { /* Use default if none passed in */ if (max_file_size_from == NULL) { - ALOGI("No max file source given, continuing to use default: 0x%lx\n", max_file_size); + ALOGI("No max file source given, continuing to use default: 0x%" PRIx64 "\n", + max_file_size); return 0; } @@ -823,7 +824,7 @@ int determine_max_file_size(const char* max_file_size_from) { close(fd); max_file_size = max_size; - ALOGI("Using 0x%lx as max file size\n", max_file_size); + ALOGI("Using 0x%" PRIx64 " as max file size\n", max_file_size); return 0; } From aaf11ec896d7458ddbadbd4c33888d9703cd650c Mon Sep 17 00:00:00 2001 From: David Anderson Date: Mon, 17 Jun 2024 19:05:13 +0000 Subject: [PATCH 5/9] Revert "snapuserd: Don't statically link outside of ramdisk." This reverts commit c9fa93f4e892ea06c0c4f8a5270a90161ae4516d. Reason for revert: b/347670914 Bug: 347670914 (cherry picked from https://android-review.googlesource.com/q/commit:14fbf6d3909dab706568d1c409a941d5fd2a7428) Merged-In: I9d63a69ccf1f8de98ab7cc23b9fbf400863cddfb Change-Id: I9d63a69ccf1f8de98ab7cc23b9fbf400863cddfb --- fs_mgr/libsnapshot/snapuserd/Android.bp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/fs_mgr/libsnapshot/snapuserd/Android.bp b/fs_mgr/libsnapshot/snapuserd/Android.bp index f04020549..649309de1 100644 --- a/fs_mgr/libsnapshot/snapuserd/Android.bp +++ b/fs_mgr/libsnapshot/snapuserd/Android.bp @@ -145,6 +145,14 @@ cc_defaults { ], include_dirs: ["bionic/libc/kernel"], + system_shared_libs: [], + + // snapuserd is started during early boot by first-stage init. At that + // point, /system is mounted using the "dm-user" device-mapper kernel + // module. dm-user routes all I/O to userspace to be handled by + // snapuserd, which would lead to deadlock if we had to handle page + // faults for its code pages. + static_executable: true, } cc_binary { @@ -157,10 +165,10 @@ cc_binary { "libsnapuserd_client", ], ramdisk_available: false, - vendor_ramdisk_available: false, + vendor_ramdisk_available: true, } -// This target will install to /system/bin/snapuserd_ramdisk +// This target will install to /system/bin/snapuserd_ramdisk // It will also create a symblink on /system/bin/snapuserd that point to // /system/bin/snapuserd_ramdisk . // This way, init can check if generic ramdisk copy exists. @@ -176,15 +184,6 @@ cc_binary { vendor_ramdisk_available: false, ramdisk: true, symlinks: ["snapuserd"], - - system_shared_libs: [], - - // snapuserd is started during early boot by first-stage init. At that - // point, /system is mounted using the "dm-user" device-mapper kernel - // module. dm-user routes all I/O to userspace to be handled by - // snapuserd, which would lead to deadlock if we had to handle page - // faults for its code pages. - static_executable: true, } cc_defaults { From e3a3d903aa0d724de5bce81d84d876798f801a0b Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 2 Jul 2024 10:57:23 -0700 Subject: [PATCH 6/9] Make snapuserd recovery_available. Bug: 349287459 Test: adb reboot recovery adb root adb shell ls -l /system/bin/snapuserd (cherry picked from https://android-review.googlesource.com/q/commit:dcc81d427a9cfcb7560b4ba590ace3348b314860) Merged-In: I69a3f8d2fd2d7dc157d14a0f743650881eec473d Change-Id: I69a3f8d2fd2d7dc157d14a0f743650881eec473d --- fs_mgr/libsnapshot/snapuserd/Android.bp | 1 + 1 file changed, 1 insertion(+) diff --git a/fs_mgr/libsnapshot/snapuserd/Android.bp b/fs_mgr/libsnapshot/snapuserd/Android.bp index 649309de1..eb5443c1c 100644 --- a/fs_mgr/libsnapshot/snapuserd/Android.bp +++ b/fs_mgr/libsnapshot/snapuserd/Android.bp @@ -166,6 +166,7 @@ cc_binary { ], ramdisk_available: false, vendor_ramdisk_available: true, + recovery_available: true, } // This target will install to /system/bin/snapuserd_ramdisk From 77d388f2858357f4d5229878c3e8f9ce0a359984 Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Thu, 11 Jul 2024 10:06:47 -0700 Subject: [PATCH 7/9] Skip tests if vendor partition is on Android S Bug: 349278999 Test: vts_libsnapshot_test on GSI config Signed-off-by: Akilesh Kailash (cherry picked from https://android-review.googlesource.com/q/commit:6906249312be46b64b6b6221ba33bad196bf6cd6) Merged-In: I6826b287565e8a78bea21b4830ad30f1c30a01bf Change-Id: I6826b287565e8a78bea21b4830ad30f1c30a01bf --- .../include_test/libsnapshot/test_helpers.h | 18 ++++++++++++++++++ fs_mgr/libsnapshot/snapshot_test.cpp | 7 +++++++ 2 files changed, 25 insertions(+) diff --git a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h index 90813fe79..0afd8bd22 100644 --- a/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h +++ b/fs_mgr/libsnapshot/include_test/libsnapshot/test_helpers.h @@ -30,6 +30,8 @@ #include #include +#include "utility.h" + namespace android { namespace snapshot { @@ -234,5 +236,21 @@ bool IsVirtualAbEnabled(); #define RETURN_IF_NON_VIRTUAL_AB() RETURN_IF_NON_VIRTUAL_AB_MSG("") +#define SKIP_IF_VENDOR_ON_ANDROID_S() \ + do { \ + if (IsVendorFromAndroid12()) \ + GTEST_SKIP() << "Skip test as Vendor partition is on Android S"; \ + } while (0) + +#define RETURN_IF_VENDOR_ON_ANDROID_S_MSG(msg) \ + do { \ + if (IsVendorFromAndroid12()) { \ + std::cerr << (msg); \ + return; \ + } \ + } while (0) + +#define RETURN_IF_VENDOR_ON_ANDROID_S() RETURN_IF_VENDOR_ON_ANDROID_S_MSG("") + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 1435b12bf..07f13014d 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -122,6 +122,7 @@ class SnapshotTest : public ::testing::Test { LOG(INFO) << "Starting test: " << test_name_; SKIP_IF_NON_VIRTUAL_AB(); + SKIP_IF_VENDOR_ON_ANDROID_S(); SetupProperties(); if (!DeviceSupportsMode()) { @@ -168,6 +169,7 @@ class SnapshotTest : public ::testing::Test { void TearDown() override { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); LOG(INFO) << "Tearing down SnapshotTest test: " << test_name_; @@ -1015,6 +1017,7 @@ class SnapshotUpdateTest : public SnapshotTest { public: void SetUp() override { SKIP_IF_NON_VIRTUAL_AB(); + SKIP_IF_VENDOR_ON_ANDROID_S(); SnapshotTest::SetUp(); if (!image_manager_) { @@ -1097,6 +1100,7 @@ class SnapshotUpdateTest : public SnapshotTest { } void TearDown() override { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); LOG(INFO) << "Tearing down SnapshotUpdateTest test: " << test_name_; @@ -2833,6 +2837,7 @@ void SnapshotTestEnvironment::SetUp() { // that is fixed, don't call GTEST_SKIP here, but instead call GTEST_SKIP in individual test // suites. RETURN_IF_NON_VIRTUAL_AB_MSG("Virtual A/B is not enabled, skipping global setup.\n"); + RETURN_IF_VENDOR_ON_ANDROID_S_MSG("Test not enabled for Vendor on Android S.\n"); std::vector paths = { // clang-format off @@ -2887,6 +2892,8 @@ void SnapshotTestEnvironment::SetUp() { void SnapshotTestEnvironment::TearDown() { RETURN_IF_NON_VIRTUAL_AB(); + RETURN_IF_VENDOR_ON_ANDROID_S(); + if (super_images_ != nullptr) { DeleteBackingImage(super_images_.get(), "fake-super"); } From 227254f61602eab532727d3d968dabba9653b0dc Mon Sep 17 00:00:00 2001 From: mingwei xue Date: Thu, 8 Aug 2024 03:38:57 +0000 Subject: [PATCH 8/9] Fix SnapshotTest crash For Android 12 vendor project, SnapshotTestEnvironment setup is skipped. So, test_device is not initialized, which will cause other tests to fail. BUG:350678717 Test:run vts -m vts_libsnapshot_test (cherry picked from https://android-review.googlesource.com/q/commit:a791e913048801a2a74beaea42b992c380eb8c2c) Merged-In: I6ce6a9e7ea2c210e25c2a5ffadaaa6348086ea7f Change-Id: I6ce6a9e7ea2c210e25c2a5ffadaaa6348086ea7f --- fs_mgr/libsnapshot/snapshot_test.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 07f13014d..bf16b0fb3 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -2837,7 +2837,6 @@ void SnapshotTestEnvironment::SetUp() { // that is fixed, don't call GTEST_SKIP here, but instead call GTEST_SKIP in individual test // suites. RETURN_IF_NON_VIRTUAL_AB_MSG("Virtual A/B is not enabled, skipping global setup.\n"); - RETURN_IF_VENDOR_ON_ANDROID_S_MSG("Test not enabled for Vendor on Android S.\n"); std::vector paths = { // clang-format off From deb1de9dc60d84341484deb2f2cc982accd9dbea Mon Sep 17 00:00:00 2001 From: Akilesh Kailash Date: Wed, 7 Aug 2024 10:35:50 -0700 Subject: [PATCH 9/9] snapuserd_test: skip test if dm-user kernel driver is absent Bug: 357487459 Test: snapuserd_test Signed-off-by: Akilesh Kailash (cherry picked from https://android-review.googlesource.com/q/commit:a880e5675b747b9f3d5631b656e0a7408edc6ad7) Merged-In: I8458f223fc35fcfa042588e67a30c5bb273b0277 Change-Id: I8458f223fc35fcfa042588e67a30c5bb273b0277 --- fs_mgr/libsnapshot/snapuserd/Android.bp | 4 ++ .../user-space-merge/snapuserd_test.cpp | 8 ++++ fs_mgr/libsnapshot/snapuserd/utility.cpp | 37 +++++++++++++++++++ fs_mgr/libsnapshot/snapuserd/utility.h | 5 +++ 4 files changed, 54 insertions(+) diff --git a/fs_mgr/libsnapshot/snapuserd/Android.bp b/fs_mgr/libsnapshot/snapuserd/Android.bp index b3a7e8cf1..efbcb5a4e 100644 --- a/fs_mgr/libsnapshot/snapuserd/Android.bp +++ b/fs_mgr/libsnapshot/snapuserd/Android.bp @@ -267,6 +267,10 @@ cc_test { test_suites: [ "vts", ], + test_options: { + // VABC mandatory in Android T per VSR. + min_shipping_api_level: 32, + }, } cc_binary_host { diff --git a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp index 6d0ae3d09..271090fa5 100644 --- a/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp +++ b/fs_mgr/libsnapshot/snapuserd/user-space-merge/snapuserd_test.cpp @@ -1528,6 +1528,14 @@ INSTANTIATE_TEST_SUITE_P(Io, HandlerTest, ::testing::ValuesIn(GetTestConfigs())) int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); +#ifdef __ANDROID__ + if (!android::snapshot::CanUseUserspaceSnapshots() || + android::snapshot::IsVendorFromAndroid12()) { + std::cerr << "snapuserd_test not supported on this device\n"; + return 0; + } +#endif + gflags::ParseCommandLineFlags(&argc, &argv, false); return RUN_ALL_TESTS(); diff --git a/fs_mgr/libsnapshot/snapuserd/utility.cpp b/fs_mgr/libsnapshot/snapuserd/utility.cpp index 684ca3d7b..b44f5abd2 100644 --- a/fs_mgr/libsnapshot/snapuserd/utility.cpp +++ b/fs_mgr/libsnapshot/snapuserd/utility.cpp @@ -14,11 +14,14 @@ #include "utility.h" +#include #include #include #include #include +#include +#include #include #include @@ -27,6 +30,7 @@ namespace android { namespace snapshot { using android::base::unique_fd; +using android::dm::DeviceMapper; bool SetThreadPriority([[maybe_unused]] int priority) { #ifdef __ANDROID__ @@ -61,5 +65,38 @@ bool KernelSupportsIoUring() { return major > 5 || (major == 5 && minor >= 6); } +bool GetUserspaceSnapshotsEnabledProperty() { + return android::base::GetBoolProperty("ro.virtual_ab.userspace.snapshots.enabled", false); +} + +bool KernelSupportsCompressedSnapshots() { + auto& dm = DeviceMapper::Instance(); + return dm.GetTargetByName("user", nullptr); +} + +bool IsVendorFromAndroid12() { + const std::string UNKNOWN = "unknown"; + const std::string vendor_release = + android::base::GetProperty("ro.vendor.build.version.release_or_codename", UNKNOWN); + + if (vendor_release.find("12") != std::string::npos) { + return true; + } + return false; +} + +bool CanUseUserspaceSnapshots() { + if (!GetUserspaceSnapshotsEnabledProperty()) { + LOG(INFO) << "Virtual A/B - Userspace snapshots disabled"; + return false; + } + + if (!KernelSupportsCompressedSnapshots()) { + LOG(ERROR) << "Userspace snapshots requested, but no kernel support is available."; + return false; + } + return true; +} + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/snapuserd/utility.h b/fs_mgr/libsnapshot/snapuserd/utility.h index c3c3cbae9..50be41837 100644 --- a/fs_mgr/libsnapshot/snapuserd/utility.h +++ b/fs_mgr/libsnapshot/snapuserd/utility.h @@ -24,5 +24,10 @@ bool SetThreadPriority(int priority); bool SetProfiles(std::initializer_list profiles); bool KernelSupportsIoUring(); +bool GetUserspaceSnapshotsEnabledProperty(); +bool KernelSupportsCompressedSnapshots(); +bool CanUseUserspaceSnapshots(); +bool IsVendorFromAndroid12(); + } // namespace snapshot } // namespace android