diff options
author | Stephen Crane <cranes@google.com> | 2021-12-13 16:41:17 -0800 |
---|---|---|
committer | Stephen Crane <cranes@google.com> | 2022-01-26 14:10:17 -0800 |
commit | 2727e16517c243874739c26fb7cf2a8aa5d9871f (patch) | |
tree | 9ba1af3cd5e7e93433270f6ce1a47ed2f4793d2f | |
parent | 437522958814ef523d62d7fa988fef495cf7f6f4 (diff) | |
download | core-2727e16517c243874739c26fb7cf2a8aa5d9871f.tar.gz |
storageproxyd: Use alternate data path if in DSU state
Adds a check for a DSU mode boot in storageproxyd. Changes path handling
so that storageproxyd will not allow opening a file in the root data
path in DSU mode. Instead, storageproxyd creates an "alternate/"
directory in the data directory and the TA must use this directory to
store its backing file.
Re-landing reverted change: Iad68872dc6915f64eaf26cd3c92c04d9071ef169
Test: Boot into DSU and inspect logs for "Cannot open root data file"
Test: Test that TD writes in DSU mode don't corrupt host image storage
when using a compatible storage TA that supports alternate data mode.
Bug: 203719297
Merged-In: I1d07e7c3d15dc1beba2d340181d1b11a7988f869
Change-Id: I1d07e7c3d15dc1beba2d340181d1b11a7988f869
-rw-r--r-- | trusty/storage/proxy/Android.bp | 5 | ||||
-rw-r--r-- | trusty/storage/proxy/checkpoint_handling.cpp | 15 | ||||
-rw-r--r-- | trusty/storage/proxy/checkpoint_handling.h | 2 | ||||
-rw-r--r-- | trusty/storage/proxy/proxy.c | 7 | ||||
-rw-r--r-- | trusty/storage/proxy/storage.c | 46 |
5 files changed, 71 insertions, 4 deletions
diff --git a/trusty/storage/proxy/Android.bp b/trusty/storage/proxy/Android.bp index 38d868508..94f26d8a6 100644 --- a/trusty/storage/proxy/Android.bp +++ b/trusty/storage/proxy/Android.bp @@ -35,7 +35,10 @@ cc_binary { "liblog", "libhardware_legacy", ], - header_libs: ["libcutils_headers"], + header_libs: [ + "libcutils_headers", + "libgsi_headers", + ], static_libs: [ "libfstab", diff --git a/trusty/storage/proxy/checkpoint_handling.cpp b/trusty/storage/proxy/checkpoint_handling.cpp index 6c2fd363e..3305d8da2 100644 --- a/trusty/storage/proxy/checkpoint_handling.cpp +++ b/trusty/storage/proxy/checkpoint_handling.cpp @@ -18,9 +18,12 @@ #include "log.h" #include <fstab/fstab.h> +#include <unistd.h> #include <cstring> #include <string> +#include <libgsi/libgsi.h> + namespace { bool checkpointingDoneForever = false; @@ -75,3 +78,15 @@ int is_data_checkpoint_active(bool* active) { return 0; } + +/** + * is_gsi_running() - Check if a GSI image is running via DSU. + * + * This function is equivalent to android::gsi::IsGsiRunning(), but this API is + * not yet vendor-accessible although the underlying metadata file is. + * + */ +bool is_gsi_running() { + /* TODO(b/210501710): Expose GSI image running state to vendor storageproxyd */ + return !access(android::gsi::kGsiBootedIndicatorFile, F_OK); +} diff --git a/trusty/storage/proxy/checkpoint_handling.h b/trusty/storage/proxy/checkpoint_handling.h index f1bf27c8d..dfe294778 100644 --- a/trusty/storage/proxy/checkpoint_handling.h +++ b/trusty/storage/proxy/checkpoint_handling.h @@ -32,6 +32,8 @@ extern "C" { */ int is_data_checkpoint_active(bool* active); +bool is_gsi_running(); + #ifdef __cplusplus } #endif diff --git a/trusty/storage/proxy/proxy.c b/trusty/storage/proxy/proxy.c index c690a2876..262003427 100644 --- a/trusty/storage/proxy/proxy.c +++ b/trusty/storage/proxy/proxy.c @@ -104,8 +104,11 @@ static int drop_privs(void) { return -1; } - /* no-execute for user, no access for group and other */ - umask(S_IXUSR | S_IRWXG | S_IRWXO); + /* + * No access for group and other. We need execute access for user to create + * an accessible directory. + */ + umask(S_IRWXG | S_IRWXO); return 0; } diff --git a/trusty/storage/proxy/storage.c b/trusty/storage/proxy/storage.c index 2fde30f84..d74a70807 100644 --- a/trusty/storage/proxy/storage.c +++ b/trusty/storage/proxy/storage.c @@ -16,6 +16,7 @@ #include <errno.h> #include <fcntl.h> #include <inttypes.h> +#include <libgen.h> #include <stdbool.h> #include <stdlib.h> #include <string.h> @@ -24,13 +25,16 @@ #include <sys/types.h> #include <unistd.h> -#include "log.h" +#include "checkpoint_handling.h" #include "ipc.h" +#include "log.h" #include "storage.h" #define FD_TBL_SIZE 64 #define MAX_READ_SIZE 4096 +#define ALTERNATE_DATA_DIR "alternate/" + enum sync_state { SS_UNUSED = -1, SS_CLEAN = 0, @@ -44,6 +48,8 @@ static enum sync_state fs_state; static enum sync_state dir_state; static enum sync_state fd_state[FD_TBL_SIZE]; +static bool alternate_mode; + static struct { struct storage_file_read_resp hdr; uint8_t data[MAX_READ_SIZE]; @@ -216,6 +222,7 @@ int storage_file_open(struct storage_msg *msg, const void *r, size_t req_len) { char *path = NULL; + char* parent_path; const struct storage_file_open_req *req = r; struct storage_file_open_resp resp = {0}; @@ -234,6 +241,24 @@ int storage_file_open(struct storage_msg *msg, goto err_response; } + /* + * TODO(b/210501710): Expose GSI image running state to vendor + * storageproxyd. We want to control data file paths in vendor_init, but we + * don't have access to the necessary property there yet. When we have + * access to that property we can set the root data path read-only and only + * allow creation of files in alternate/. Checking paths here temporarily + * until that is fixed. + * + * We are just checking for "/" instead of "alternate/" because we still + * want to still allow access to "persist/" in alternate mode (for now, this + * may change in the future). + */ + if (alternate_mode && !strchr(req->name, '/')) { + ALOGE("%s: Cannot open root data file \"%s\" in alternate mode\n", __func__, req->name); + msg->result = STORAGE_ERR_ACCESS; + goto err_response; + } + int rc = asprintf(&path, "%s/%s", ssdir_name, req->name); if (rc < 0) { ALOGE("%s: asprintf failed\n", __func__); @@ -246,7 +271,23 @@ int storage_file_open(struct storage_msg *msg, if (req->flags & STORAGE_FILE_OPEN_TRUNCATE) open_flags |= O_TRUNC; + parent_path = dirname(path); if (req->flags & STORAGE_FILE_OPEN_CREATE) { + /* + * Create the alternate parent dir if needed & allowed. + * + * TODO(b/210501710): Expose GSI image running state to vendor + * storageproxyd. This directory should be created by vendor_init, once + * it has access to the necessary bit of information. + */ + if (strstr(req->name, ALTERNATE_DATA_DIR) == req->name) { + rc = mkdir(parent_path, S_IRWXU); + if (rc && errno != EEXIST) { + ALOGE("%s: Could not create parent directory \"%s\": %s\n", __func__, parent_path, + strerror(errno)); + } + } + /* open or create */ if (req->flags & STORAGE_FILE_OPEN_CREATE_EXCLUSIVE) { /* create exclusive */ @@ -467,6 +508,9 @@ err_response: int storage_init(const char *dirname) { + /* If there is an active DSU image, use the alternate fs mode. */ + alternate_mode = is_gsi_running(); + fs_state = SS_CLEAN; dir_state = SS_CLEAN; for (uint i = 0; i < FD_TBL_SIZE; i++) { |