tequilaOS/platform_system_core
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

storageproxyd: discard writes when checkpointing, if necessary

Browse source 
If a checkpointing operation is in progress, discard any write operations
that are flagged as STORAGE_MSG_FLAG_PRE_COMMIT_CHECKPOINT. In tandem
with trusty-side changes that set the flag appropriately, this avoids
the awkward case where the checkpoint is rolled back, which potentially
leads to inconsistency between the data and the superblock.

Based on Stephen's CL/1845477 "Add helper to check checkpoint state of
mounts".

Test: m storageproxyd
Bug: 194313068
Change-Id: Ib6a432db1bc1b034f803b743b0d7322e3f31d814
This commit is contained in:
Thurston Dang 2021-10-07 23:08:56 +00:00
parent 1132977223
commit 7e8dab775b
5 changed files with 155 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
trusty/storage/interface/include/trusty/interface/storage.h
View file

@ -112,26 +112,30 @@ enum storage_file_open_flag {
/**
* enum storage_msg_flag - protocol-level flags in struct storage_msg
* @STORAGE_MSG_FLAG_BATCH: if set, command belongs to a batch transaction.
* No response will be sent by the server until
* it receives a command with this flag unset, at
* which point a cummulative result for all messages
* sent with STORAGE_MSG_FLAG_BATCH will be sent.
* This is only supported by the non-secure disk proxy
* server.
* @STORAGE_MSG_FLAG_PRE_COMMIT: if set, indicates that server need to commit
* pending changes before processing this message.
* @STORAGE_MSG_FLAG_POST_COMMIT: if set, indicates that server need to commit
* pending changes after processing this message.
* @STORAGE_MSG_FLAG_TRANSACT_COMPLETE: if set, indicates that server need to commit
* current transaction after processing this message.
* It is an alias for STORAGE_MSG_FLAG_POST_COMMIT.
* @STORAGE_MSG_FLAG_BATCH: if set, command belongs to a batch transaction.
* No response will be sent by the server until
* it receives a command with this flag unset, at
* which point a cumulative result for all messages
* sent with STORAGE_MSG_FLAG_BATCH will be sent.
* This is only supported by the non-secure disk proxy
* server.
* @STORAGE_MSG_FLAG_PRE_COMMIT: if set, indicates that server need to commit
* pending changes before processing this message.
* @STORAGE_MSG_FLAG_POST_COMMIT: if set, indicates that server need to commit
* pending changes after processing this message.
* @STORAGE_MSG_FLAG_TRANSACT_COMPLETE: if set, indicates that server need to commit
* current transaction after processing this message.
* It is an alias for STORAGE_MSG_FLAG_POST_COMMIT.
* @STORAGE_MSG_FLAG_PRE_COMMIT_CHECKPOINT: if set, indicates that server needs to ensure
* that there is not a pending checkpoint for
* userdata before processing this message.
*/
enum storage_msg_flag {
STORAGE_MSG_FLAG_BATCH = 0x1,
STORAGE_MSG_FLAG_PRE_COMMIT = 0x2,
STORAGE_MSG_FLAG_POST_COMMIT = 0x4,
STORAGE_MSG_FLAG_TRANSACT_COMPLETE = STORAGE_MSG_FLAG_POST_COMMIT,
STORAGE_MSG_FLAG_BATCH = 0x1,
STORAGE_MSG_FLAG_PRE_COMMIT = 0x2,
STORAGE_MSG_FLAG_POST_COMMIT = 0x4,
STORAGE_MSG_FLAG_TRANSACT_COMPLETE = STORAGE_MSG_FLAG_POST_COMMIT,
STORAGE_MSG_FLAG_PRE_COMMIT_CHECKPOINT = 0x8,
};
/*

3
trusty/storage/proxy/Android.bp
View file

@ -23,6 +23,7 @@ cc_binary {
vendor: true,
srcs: [
"checkpoint_handling.cpp",
"ipc.c",
"rpmb.c",
"storage.c",
@ -30,12 +31,14 @@ cc_binary {
],
shared_libs: [
"libbase",
"liblog",
"libhardware_legacy",
],
header_libs: ["libcutils_headers"],
static_libs: [
"libfstab",
"libtrustystorageinterface",
"libtrusty",
],

77
trusty/storage/proxy/checkpoint_handling.cpp Normal file
View file

@ -0,0 +1,77 @@
/*
* Copyright (C) 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include "checkpoint_handling.h"
#include "log.h"
#include <fstab/fstab.h>
#include <cstring>
#include <string>
namespace {
bool checkpointingDoneForever = false;
} // namespace
int is_data_checkpoint_active(bool* active) {
if (!active) {
ALOGE("active out parameter is null");
return 0;
}
*active = false;
if (checkpointingDoneForever) {
return 0;
}
android::fs_mgr::Fstab procMounts;
bool success = android::fs_mgr::ReadFstabFromFile("/proc/mounts", &procMounts);
if (!success) {
ALOGE("Could not parse /proc/mounts\n");
/* Really bad. Tell the caller to abort the write. */
return -1;
}
android::fs_mgr::FstabEntry* dataEntry =
android::fs_mgr::GetEntryForMountPoint(&procMounts, "/data");
if (dataEntry == NULL) {
ALOGE("/data is not mounted yet\n");
return 0;
}
/* We can't handle e.g., ext4. Nothing we can do about it for now. */
if (dataEntry->fs_type != "f2fs") {
ALOGW("Checkpoint status not supported for filesystem %s\n", dataEntry->fs_type.c_str());
checkpointingDoneForever = true;
return 0;
}
/*
* The data entry looks like "... blah,checkpoint=disable:0,blah ...".
* checkpoint=disable means checkpointing is on (yes, arguably reversed).
*/
size_t checkpointPos = dataEntry->fs_options.find("checkpoint=disable");
if (checkpointPos == std::string::npos) {
/* Assumption is that once checkpointing turns off, it stays off */
checkpointingDoneForever = true;
} else {
*active = true;
}
return 0;
}

37
trusty/storage/proxy/checkpoint_handling.h Normal file
View file

@ -0,0 +1,37 @@
/*
* Copyright (C) 2021 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once
#include <stdbool.h>
#ifdef __cplusplus
extern "C" {
#endif
/**
* is_data_checkpoint_active() - Check for an active, uncommitted checkpoint of
* /data. If a checkpoint is active, storage should not commit any
* rollback-protected writes to /data.
* @active: Out parameter that will be set to the result of the check.
*
* Return: 0 if active was set and is valid, non-zero otherwise.
*/
int is_data_checkpoint_active(bool* active);
#ifdef __cplusplus
}
#endif

16
trusty/storage/proxy/proxy.c
View file

@ -26,6 +26,7 @@
#include <cutils/android_filesystem_config.h>
#include "checkpoint_handling.h"
#include "ipc.h"
#include "log.h"
#include "rpmb.h"
@ -130,6 +131,21 @@ static int handle_req(struct storage_msg* msg, const void* req, size_t req_len)
}
}
if (msg->flags & STORAGE_MSG_FLAG_PRE_COMMIT_CHECKPOINT) {
bool is_checkpoint_active = false;
rc = is_data_checkpoint_active(&is_checkpoint_active);
if (rc != 0) {
ALOGE("is_data_checkpoint_active failed in an unexpected way. Aborting.\n");
msg->result = STORAGE_ERR_GENERIC;
return ipc_respond(msg, NULL, 0);
} else if (is_checkpoint_active) {
ALOGE("Checkpoint in progress, dropping write ...\n");
msg->result = STORAGE_ERR_GENERIC;
return ipc_respond(msg, NULL, 0);
}
}
switch (msg->cmd) {
case STORAGE_FILE_DELETE:
rc = storage_file_delete(msg, req, req_len);