Skip to content

Commit 42ab814

Browse files
fmayerfmayer
committed
Properly handle unsetting of override
When the override went from force_on -> default, we would keep the memtag setting on, even if the user hadn't manually turned it on. Bug: 291106070 Test: atest mtectrl_test Change-Id: I67e357d20e1c8ce2608b92a55e7b3d295b9ea87d
1 parent 7164c99 commit 42ab814

File tree

2 files changed

+215
-39
lines changed

2 files changed

+215
-39
lines changed

‎mtectrl/mtectrl.cc

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ bool UpdateProp(const char* prop_name, const misc_memtag_message& m) {
4747
if (CheckAndUnset(mode, MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE))
4848
AddItem(&prop_str, "memtag-kernel-once");
4949
if (CheckAndUnset(mode, MISC_MEMTAG_MODE_MEMTAG_OFF)) AddItem(&prop_str, "memtag-off");
50+
if (CheckAndUnset(mode, MISC_MEMTAG_MODE_FORCED)) AddItem(&prop_str, "forced");
5051
if (prop_str.empty()) prop_str = "none";
5152
if (android::base::GetProperty(prop_name, "") != prop_str)
5253
android::base::SetProperty(prop_name, prop_str);
@@ -94,6 +95,9 @@ void PrintUsage(const char* progname) {
9495
" - memtag-kernel-once: MTE is enabled in the kernel, only for the next reboot.\n"
9596
" - memtag-off: MTE is persistently disabled in both userspace and kernel upon \n"
9697
" the next reboot.\n"
98+
" - forced: the current state is the result of force_on or force_off in the next\n"
99+
" argument. When the next argument is set back to \"default\", the\n"
100+
" state will be cleared.\n"
97101
" [default|force_on|force_off]\n"
98102
" An alternative method of configuring the MTE options to be applied, if provided.\n"
99103
" This control is generally to be used by device_config only, and it overwrites\n"
@@ -121,6 +125,8 @@ int StringToMode(const char* value) {
121125
memtag_mode |= MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE;
122126
} else if (field == "memtag-off") {
123127
memtag_mode |= MISC_MEMTAG_MODE_MEMTAG_OFF;
128+
} else if (field == "forced") {
129+
memtag_mode |= MISC_MEMTAG_MODE_FORCED;
124130
} else if (field != "none") {
125131
LOG(ERROR) << "Unknown value for mode: " << field;
126132
return -1;
@@ -129,15 +135,103 @@ int StringToMode(const char* value) {
129135
return memtag_mode;
130136
}
131137

138+
// Handles the override flag and applies it to the memtag message.
139+
// The logic is as follows:
140+
// If the override changes the configuration (i.e., if MTE was not enabled
141+
// through MODE_MEMTAG and the override is force_on, or MTE was not
142+
// disabled through MEMTAG_OFF and the override is force_off), the MTE
143+
// state is considered FORCED. In that case, if the override gets reset
144+
// to "default" (i.e. no override), the default state of memtag config
145+
// is restored. The theory for this is that disabling the override should
146+
// only keep the non-default state if it has been active throughout the
147+
// override, not restore it if it had been dormant for the duration of the
148+
// override.
149+
//
150+
// State machine diagrams of the MTE state and the effect of override below:
151+
//
152+
// default,force_off
153+
// ┌───┐
154+
// │ │
155+
// ┌──┴───▼───┐
156+
// │memtag-off│
157+
// └─────┬────┘
158+
// │
159+
// force_on │ ┌────┐
160+
// │ │ │ force_on
161+
// force_off┌───────▼───┴─┐ │
162+
// ┌────────┤memtag,forced│◄─┘
163+
// │ └▲─────────┬──┘
164+
// force_off │ │ │
165+
// ┌────┐ │ force_on│ │ default
166+
// │ │ │ │ │
167+
// │ ┌─┴────▼─────────┴┐ ┌▼──────┐
168+
// └─►│memtag-off,forced├───────►none │
169+
// └─────────────────┘default└───────┘
170+
//
171+
//
172+
//
173+
// default,force_on
174+
// ┌───┐
175+
// │ │
176+
// ┌──┴───▼───┐
177+
// │memtag │
178+
// └─────┬────┘
179+
// │
180+
// force_off│ ┌────┐
181+
// │ │ │ force_off
182+
// force_on ┌───────┴───────┴─┐ │
183+
// ┌────────┤memtag-off,forced◄──┘
184+
// │ └▲─────────┬──────┘
185+
// force_on │ │ │
186+
// ┌────┐ │force_off│ │ default
187+
// │ │ │ │ │
188+
// │ ┌─┴────▼─────────┴┐ ┌▼──────┐
189+
// └─►│memtag,forced ├───────►none │
190+
// └─────────────────┘default└───────┘
191+
//
192+
//
193+
//
194+
// default
195+
// ┌───┐
196+
// │ │
197+
// force_off ┌──┴───▼───┐
198+
// ┌─────────────┤none │
199+
// │ └─────┬────┘
200+
// │ │
201+
// │ force_on │ ┌────┐
202+
// │ │ │ │ force_on
203+
// │ force_off┌───────▼───┴─┐ │
204+
// │ ┌────────┤memtag,forced│◄─┘
205+
// │ │ └▲─────────┬──┘
206+
// force_off│ │ │ │
207+
// ┌────┐ │ │ force_on│ │ default
208+
// │ │ │ │ │ │
209+
// │ ┌─┴─▼──▼─────────┴┐ ┌▼──────┐
210+
// └─►│memtag-off,forced├───────►none │
211+
// └─────────────────┘default└───────┘
132212
bool HandleOverride(const std::string& override_value, misc_memtag_message* m) {
133213
if (override_value == "force_off") {
134214
// If the force_off override is active, only allow MEMTAG_MODE_MEMTAG_ONCE.
215+
if ((m->memtag_mode & MISC_MEMTAG_MODE_MEMTAG_OFF) == 0) {
216+
m->memtag_mode |= MISC_MEMTAG_MODE_FORCED;
217+
}
135218
m->memtag_mode |= MISC_MEMTAG_MODE_MEMTAG_OFF;
136219
m->memtag_mode &= ~MISC_MEMTAG_MODE_MEMTAG;
137220
} else if (override_value == "force_on") {
221+
if ((m->memtag_mode & MISC_MEMTAG_MODE_MEMTAG) == 0) {
222+
m->memtag_mode |= MISC_MEMTAG_MODE_FORCED;
223+
}
138224
m->memtag_mode |= MISC_MEMTAG_MODE_MEMTAG;
139225
m->memtag_mode &= ~MISC_MEMTAG_MODE_MEMTAG_OFF;
140-
} else if (!override_value.empty() && override_value != "default") {
226+
} else if (override_value.empty() || override_value == "default") {
227+
// The mode changed from forced_on or forced_off to default, which means we
228+
// restore the normal state.
229+
if (m->memtag_mode & MISC_MEMTAG_MODE_FORCED) {
230+
m->memtag_mode &= ~MISC_MEMTAG_MODE_MEMTAG;
231+
m->memtag_mode &= ~MISC_MEMTAG_MODE_MEMTAG_OFF;
232+
m->memtag_mode &= ~MISC_MEMTAG_MODE_FORCED;
233+
}
234+
} else {
141235
return false;
142236
}
143237
return true;

‎mtectrl/mtectrl_test.cc

Lines changed: 120 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,55 @@
2323
#include <android-base/logging.h>
2424
#include <android-base/properties.h>
2525
#include <bootloader_message/bootloader_message.h>
26+
#include <string_view>
2627

2728
namespace {
2829
using ::testing::StartsWith;
2930

30-
int mtectrl(const char* arg) {
31+
int mtectrl(std::string_view arg) {
3132
std::string cmd = "mtectrl -t /data/local/tmp/misc_memtag ";
3233
cmd += arg;
3334
return system(cmd.c_str());
3435
}
3536

37+
int RunMteCtrl() {
38+
CHECK(android::base::GetIntProperty("arm64.memtag.test_bootctl_loaded", 0) == 1);
39+
std::string arg = android::base::GetProperty("arm64.memtag.test_bootctl", "none");
40+
arg += " ";
41+
arg += android::base::GetProperty("arm64.memtag.test_bootctl_override", "default");
42+
return mtectrl(arg);
43+
}
44+
45+
void Boot(misc_memtag_message m) {
46+
std::string m_str(reinterpret_cast<char*>(&m), sizeof(m));
47+
android::base::WriteStringToFile(m_str, "/data/local/tmp/misc_memtag");
48+
mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded");
49+
// arm64.memtag.test_bootctl got updated, so we trigger ourselves.
50+
RunMteCtrl();
51+
}
52+
53+
void Reboot() {
54+
android::base::SetProperty("arm64.memtag.test_bootctl", "INVALID");
55+
android::base::SetProperty("arm64.memtag.test_bootctl_loaded", "0");
56+
std::string m_str;
57+
ASSERT_TRUE(android::base::ReadFileToString("/data/local/tmp/misc_memtag", &m_str));
58+
misc_memtag_message m;
59+
ASSERT_EQ(m_str.size(), sizeof(m));
60+
memcpy(&m, m_str.c_str(), sizeof(m));
61+
m.memtag_mode &= ~MISC_MEMTAG_MODE_MEMTAG_ONCE;
62+
Boot(m);
63+
}
64+
65+
void SetMemtagProp(const std::string& s) {
66+
android::base::SetProperty("arm64.memtag.test_bootctl", s);
67+
RunMteCtrl();
68+
}
69+
70+
void SetOverrideProp(const std::string& s) {
71+
android::base::SetProperty("arm64.memtag.test_bootctl_override", s);
72+
RunMteCtrl();
73+
}
74+
3675
std::string GetMisc() {
3776
std::string data;
3877
CHECK(android::base::ReadFileToString("/data/local/tmp/misc_memtag", &data, false));
@@ -55,6 +94,7 @@ class MteCtrlTest : public ::testing::Test {
5594
CHECK(ftruncate(fd, sizeof(misc_memtag_message)) != -1);
5695
close(fd);
5796
android::base::SetProperty("arm64.memtag.test_bootctl", "INVALID");
97+
android::base::SetProperty("arm64.memtag.test_bootctl_override", "");
5898
android::base::SetProperty("arm64.memtag.test_bootctl_loaded", "0");
5999
}
60100
void TearDown() override {
@@ -68,37 +108,28 @@ TEST_F(MteCtrlTest, invalid) {
68108
}
69109

70110
TEST_F(MteCtrlTest, set_once) {
71-
ASSERT_EQ(mtectrl("memtag-once"), 0);
111+
Boot({});
112+
SetMemtagProp("memtag-once");
72113
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x02"));
73114
}
74115

75116
TEST_F(MteCtrlTest, set_once_kernel) {
76-
ASSERT_EQ(mtectrl("memtag-once,memtag-kernel"), 0);
117+
Boot({});
118+
SetMemtagProp("memtag-once,memtag-kernel");
77119
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x06"));
78120
}
79121

80-
TEST_F(MteCtrlTest, set_memtag) {
81-
ASSERT_EQ(mtectrl("memtag"), 0);
82-
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x01"));
83-
}
84-
85-
TEST_F(MteCtrlTest, set_memtag_force_off) {
86-
ASSERT_EQ(mtectrl("memtag force_off"), 0);
87-
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x10"));
88-
}
89-
90122
TEST_F(MteCtrlTest, read_memtag) {
91-
ASSERT_EQ(mtectrl("memtag"), 0);
92-
ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
123+
Boot({});
124+
SetMemtagProp("memtag");
125+
Reboot();
93126
EXPECT_EQ(TestProperty(), "memtag");
94127
EXPECT_EQ(TestFlag(), "1");
95128
}
96129

97130
TEST_F(MteCtrlTest, read_invalid_memtag_message) {
98131
misc_memtag_message m = {.version = 1, .magic = 0xffff, .memtag_mode = MISC_MEMTAG_MODE_MEMTAG};
99-
std::string m_str(reinterpret_cast<char*>(&m), sizeof(m));
100-
android::base::WriteStringToFile(m_str, "/data/local/tmp/misc_memtag");
101-
ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
132+
Boot(m);
102133
EXPECT_EQ(TestProperty(), "none");
103134
EXPECT_EQ(TestFlag(), "1");
104135
}
@@ -107,49 +138,100 @@ TEST_F(MteCtrlTest, read_invalid_memtag_mode) {
107138
misc_memtag_message m = {.version = MISC_MEMTAG_MESSAGE_VERSION,
108139
.magic = MISC_MEMTAG_MAGIC_HEADER,
109140
.memtag_mode = MISC_MEMTAG_MODE_MEMTAG | 1u << 31};
110-
std::string m_str(reinterpret_cast<char*>(&m), sizeof(m));
111-
android::base::WriteStringToFile(m_str, "/data/local/tmp/misc_memtag");
112-
ASSERT_NE(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
141+
Boot(m);
113142
EXPECT_EQ(TestProperty(), "memtag");
114143
EXPECT_EQ(TestFlag(), "1");
115144
}
116145

117-
TEST_F(MteCtrlTest, set_read_memtag) {
118-
ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl memtag"), 0);
119-
EXPECT_EQ(TestProperty(), "memtag");
146+
TEST_F(MteCtrlTest, set_read_force_off) {
147+
Boot({});
148+
SetMemtagProp("memtag,memtag-once");
149+
SetOverrideProp("force_off");
150+
Reboot();
151+
EXPECT_EQ(TestProperty(), "memtag-off,forced");
152+
SetOverrideProp("default");
153+
Reboot();
154+
EXPECT_EQ(TestProperty(), "none");
120155
}
121156

122-
TEST_F(MteCtrlTest, set_read_force_off) {
123-
ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl memtag,memtag-once force_off"), 0);
124-
EXPECT_EQ(TestProperty(), "memtag-once,memtag-off");
157+
TEST_F(MteCtrlTest, set_read_force_off_already) {
158+
Boot({});
159+
SetMemtagProp("memtag-off,memtag-once");
160+
SetOverrideProp("force_off");
161+
Reboot();
162+
EXPECT_EQ(TestProperty(), "memtag-off");
163+
SetOverrideProp("default");
164+
Reboot();
165+
EXPECT_EQ(TestProperty(), "memtag-off");
166+
}
167+
168+
TEST_F(MteCtrlTest, set_read_force_on) {
169+
Boot({});
170+
SetMemtagProp("memtag-once");
171+
SetOverrideProp("force_on");
172+
Reboot();
173+
EXPECT_EQ(TestProperty(), "memtag,forced");
174+
SetOverrideProp("default");
175+
Reboot();
176+
EXPECT_EQ(TestProperty(), "none");
177+
}
178+
179+
TEST_F(MteCtrlTest, set_read_force_on_already) {
180+
Boot({});
181+
SetMemtagProp("memtag,memtag-once");
182+
SetOverrideProp("force_on");
183+
Reboot();
184+
EXPECT_EQ(TestProperty(), "memtag");
185+
SetOverrideProp("default");
186+
Reboot();
187+
EXPECT_EQ(TestProperty(), "memtag");
125188
}
126189

127190
TEST_F(MteCtrlTest, override) {
128-
ASSERT_EQ(mtectrl("memtag"), 0);
129-
ASSERT_EQ(mtectrl("memtag-once"), 0);
191+
Boot({});
192+
SetMemtagProp(("memtag"));
193+
SetMemtagProp(("memtag-once"));
130194
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x02"));
131195
}
132196

133197
TEST_F(MteCtrlTest, read_empty) {
134-
ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
198+
Boot({});
135199
EXPECT_EQ(TestProperty(), "none");
136200
EXPECT_EQ(TestFlag(), "1");
137201
}
138202

139203
TEST_F(MteCtrlTest, force_off_invalid_mode) {
140-
mtectrl("-s arm64.memtag.test_bootctl memtag-invalid force_off");
141-
EXPECT_EQ(TestProperty(), "memtag-off");
142-
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x10"));
204+
Boot({});
205+
SetMemtagProp("memtag-invalid");
206+
SetOverrideProp("force_off");
207+
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x30"));
208+
Reboot();
209+
EXPECT_EQ(TestProperty(), "memtag-off,forced");
210+
SetOverrideProp("default");
211+
Reboot();
212+
EXPECT_EQ(TestProperty(), "none");
143213
}
144214

145215
TEST_F(MteCtrlTest, force_on_invalid_mode) {
146-
mtectrl("-s arm64.memtag.test_bootctl memtag-invalid force_on");
147-
EXPECT_EQ(TestProperty(), "memtag");
148-
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x01"));
216+
Boot({});
217+
SetMemtagProp("memtag-invalid");
218+
SetOverrideProp("force_on");
219+
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x21"));
220+
Reboot();
221+
EXPECT_EQ(TestProperty(), "memtag,forced");
222+
SetOverrideProp("default");
223+
Reboot();
224+
EXPECT_EQ(TestProperty(), "none");
149225
}
150226

151227
TEST_F(MteCtrlTest, mode_invalid_override) {
152-
mtectrl("-s arm64.memtag.test_bootctl memtag force_invalid");
153-
EXPECT_EQ(TestProperty(), "memtag");
228+
Boot({});
229+
SetMemtagProp("memtag");
230+
SetOverrideProp("force_invalid");
154231
EXPECT_THAT(GetMisc(), StartsWith("\x01\x5a\xfe\xfe\x5a\x01"));
232+
Reboot();
233+
EXPECT_EQ(TestProperty(), "memtag");
234+
SetOverrideProp("default");
235+
Reboot();
236+
EXPECT_EQ(TestProperty(), "memtag");
155237
}

0 commit comments

Comments
 (0)