summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephen Crane <cranes@google.com>2021-12-13 16:41:17 -0800
committerStephen Crane <cranes@google.com>2022-01-26 14:10:17 -0800
commit2727e16517c243874739c26fb7cf2a8aa5d9871f (patch)
tree9ba1af3cd5e7e93433270f6ce1a47ed2f4793d2f
parent437522958814ef523d62d7fa988fef495cf7f6f4 (diff)
downloadcore-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.bp5
-rw-r--r--trusty/storage/proxy/checkpoint_handling.cpp15
-rw-r--r--trusty/storage/proxy/checkpoint_handling.h2
-rw-r--r--trusty/storage/proxy/proxy.c7
-rw-r--r--trusty/storage/proxy/storage.c46
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++) {