Merge "Revert "Re-land removing RECOVERY_AS_BOOT check for init_first_stage"" am: d939aa2405 am: eafdcd0b27

Original change: https://android-review.googlesource.com/c/platform/system/core/+/1765788

Change-Id: I0b54cb36881d6e31233d83e4a81e0aab04a41fa8
diff --git a/init/Android.bp b/init/Android.bp
index 3e8d4e3..d0b58ca 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -424,6 +424,7 @@
 
     srcs: [
         "devices_test.cpp",
+        "epoll_test.cpp",
         "firmware_handler_test.cpp",
         "init_test.cpp",
         "keychords_test.cpp",
@@ -431,6 +432,7 @@
         "persistent_properties_test.cpp",
         "property_service_test.cpp",
         "property_type_test.cpp",
+        "reboot_test.cpp",
         "rlimit_parser_test.cpp",
         "service_test.cpp",
         "subcontext_test.cpp",
diff --git a/init/epoll.cpp b/init/epoll.cpp
index 17d63fa..74d8aac 100644
--- a/init/epoll.cpp
+++ b/init/epoll.cpp
@@ -38,11 +38,12 @@
     return {};
 }
 
-Result<void> Epoll::RegisterHandler(int fd, std::function<void()> handler, uint32_t events) {
+Result<void> Epoll::RegisterHandler(int fd, Handler handler, uint32_t events) {
     if (!events) {
         return Error() << "Must specify events";
     }
-    auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(handler));
+    auto sp = std::make_shared<decltype(handler)>(std::move(handler));
+    auto [it, inserted] = epoll_handlers_.emplace(fd, std::move(sp));
     if (!inserted) {
         return Error() << "Cannot specify two epoll handlers for a given FD";
     }
@@ -69,7 +70,7 @@
     return {};
 }
 
-Result<std::vector<std::function<void()>*>> Epoll::Wait(
+Result<std::vector<std::shared_ptr<Epoll::Handler>>> Epoll::Wait(
         std::optional<std::chrono::milliseconds> timeout) {
     int timeout_ms = -1;
     if (timeout && timeout->count() < INT_MAX) {
@@ -81,9 +82,10 @@
     if (num_events == -1) {
         return ErrnoError() << "epoll_wait failed";
     }
-    std::vector<std::function<void()>*> pending_functions;
+    std::vector<std::shared_ptr<Handler>> pending_functions;
     for (int i = 0; i < num_events; ++i) {
-        pending_functions.emplace_back(reinterpret_cast<std::function<void()>*>(ev[i].data.ptr));
+        auto sp = *reinterpret_cast<std::shared_ptr<Handler>*>(ev[i].data.ptr);
+        pending_functions.emplace_back(std::move(sp));
     }
 
     return pending_functions;
diff --git a/init/epoll.h b/init/epoll.h
index c32a661..0df5289 100644
--- a/init/epoll.h
+++ b/init/epoll.h
@@ -22,6 +22,7 @@
 #include <chrono>
 #include <functional>
 #include <map>
+#include <memory>
 #include <optional>
 #include <vector>
 
@@ -36,15 +37,17 @@
   public:
     Epoll();
 
+    typedef std::function<void()> Handler;
+
     Result<void> Open();
-    Result<void> RegisterHandler(int fd, std::function<void()> handler, uint32_t events = EPOLLIN);
+    Result<void> RegisterHandler(int fd, Handler handler, uint32_t events = EPOLLIN);
     Result<void> UnregisterHandler(int fd);
-    Result<std::vector<std::function<void()>*>> Wait(
+    Result<std::vector<std::shared_ptr<Handler>>> Wait(
             std::optional<std::chrono::milliseconds> timeout);
 
   private:
     android::base::unique_fd epoll_fd_;
-    std::map<int, std::function<void()>> epoll_handlers_;
+    std::map<int, std::shared_ptr<Handler>> epoll_handlers_;
 };
 
 }  // namespace init
diff --git a/init/epoll_test.cpp b/init/epoll_test.cpp
new file mode 100644
index 0000000..9236cd5
--- /dev/null
+++ b/init/epoll_test.cpp
@@ -0,0 +1,76 @@
+/*
+ * 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 "epoll.h"
+
+#include <sys/unistd.h>
+
+#include <unordered_set>
+
+#include <android-base/file.h>
+#include <gtest/gtest.h>
+
+namespace android {
+namespace init {
+
+std::unordered_set<void*> sValidObjects;
+
+class CatchDtor final {
+  public:
+    CatchDtor() { sValidObjects.emplace(this); }
+    CatchDtor(const CatchDtor&) { sValidObjects.emplace(this); }
+    ~CatchDtor() {
+        auto iter = sValidObjects.find(this);
+        if (iter != sValidObjects.end()) {
+            sValidObjects.erase(iter);
+        }
+    }
+};
+
+TEST(epoll, UnregisterHandler) {
+    Epoll epoll;
+    ASSERT_RESULT_OK(epoll.Open());
+
+    int fds[2];
+    ASSERT_EQ(pipe(fds), 0);
+
+    CatchDtor catch_dtor;
+    bool handler_invoked;
+    auto handler = [&, catch_dtor]() -> void {
+        auto result = epoll.UnregisterHandler(fds[0]);
+        ASSERT_EQ(result.ok(), !handler_invoked);
+        handler_invoked = true;
+        ASSERT_NE(sValidObjects.find((void*)&catch_dtor), sValidObjects.end());
+    };
+
+    epoll.RegisterHandler(fds[0], std::move(handler));
+
+    uint8_t byte = 0xee;
+    ASSERT_TRUE(android::base::WriteFully(fds[1], &byte, sizeof(byte)));
+
+    auto results = epoll.Wait({});
+    ASSERT_RESULT_OK(results);
+    ASSERT_EQ(results->size(), size_t(1));
+
+    for (const auto& function : *results) {
+        (*function)();
+        (*function)();
+    }
+    ASSERT_TRUE(handler_invoked);
+}
+
+}  // namespace init
+}  // namespace android
diff --git a/init/reboot.cpp b/init/reboot.cpp
index 0e788e4..ed137ed 100644
--- a/init/reboot.cpp
+++ b/init/reboot.cpp
@@ -550,8 +550,8 @@
 
 // Like StopServices, but also logs all the services that failed to stop after the provided timeout.
 // Returns number of violators.
-static int StopServicesAndLogViolations(const std::set<std::string>& services,
-                                        std::chrono::milliseconds timeout, bool terminate) {
+int StopServicesAndLogViolations(const std::set<std::string>& services,
+                                 std::chrono::milliseconds timeout, bool terminate) {
     StopServices(services, timeout, terminate);
     int still_running = 0;
     for (const auto& s : ServiceList::GetInstance()) {
diff --git a/init/reboot.h b/init/reboot.h
index 81c3edc..551a114 100644
--- a/init/reboot.h
+++ b/init/reboot.h
@@ -17,11 +17,17 @@
 #ifndef _INIT_REBOOT_H
 #define _INIT_REBOOT_H
 
+#include <chrono>
+#include <set>
 #include <string>
 
 namespace android {
 namespace init {
 
+// Like StopServices, but also logs all the services that failed to stop after the provided timeout.
+// Returns number of violators.
+int StopServicesAndLogViolations(const std::set<std::string>& services,
+                                 std::chrono::milliseconds timeout, bool terminate);
 // Parses and handles a setprop sys.powerctl message.
 void HandlePowerctlMessage(const std::string& command);
 
diff --git a/init/reboot_test.cpp b/init/reboot_test.cpp
new file mode 100644
index 0000000..b3d038d
--- /dev/null
+++ b/init/reboot_test.cpp
@@ -0,0 +1,196 @@
+/*
+ * Copyright (C) 2020 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 "reboot.h"
+
+#include <errno.h>
+#include <unistd.h>
+
+#include <memory>
+#include <string_view>
+
+#include <android-base/file.h>
+#include <android-base/properties.h>
+#include <android-base/strings.h>
+#include <gtest/gtest.h>
+#include <selinux/selinux.h>
+
+#include "builtin_arguments.h"
+#include "builtins.h"
+#include "parser.h"
+#include "service_list.h"
+#include "service_parser.h"
+#include "subcontext.h"
+#include "util.h"
+
+using namespace std::literals;
+
+using android::base::GetProperty;
+using android::base::Join;
+using android::base::SetProperty;
+using android::base::Split;
+using android::base::StringReplace;
+using android::base::WaitForProperty;
+using android::base::WriteStringToFd;
+
+namespace android {
+namespace init {
+
+class RebootTest : public ::testing::Test {
+  public:
+    RebootTest() {
+        std::vector<std::string> names = GetServiceNames();
+        if (!names.empty()) {
+            ADD_FAILURE() << "Expected empty ServiceList but found: [" << Join(names, ',') << "]";
+        }
+    }
+
+    ~RebootTest() {
+        std::vector<std::string> names = GetServiceNames();
+        for (const auto& name : names) {
+            auto s = ServiceList::GetInstance().FindService(name);
+            auto pid = s->pid();
+            ServiceList::GetInstance().RemoveService(*s);
+            if (pid > 0) {
+                kill(pid, SIGTERM);
+                kill(pid, SIGKILL);
+            }
+        }
+    }
+
+  private:
+    std::vector<std::string> GetServiceNames() const {
+        std::vector<std::string> names;
+        for (const auto& s : ServiceList::GetInstance()) {
+            names.push_back(s->name());
+        }
+        return names;
+    }
+};
+
+std::string GetSecurityContext() {
+    char* ctx;
+    if (getcon(&ctx) == -1) {
+        ADD_FAILURE() << "Failed to call getcon : " << strerror(errno);
+    }
+    std::string result = std::string(ctx);
+    freecon(ctx);
+    return result;
+}
+
+void AddTestService(const std::string& name) {
+    static constexpr std::string_view kScriptTemplate = R"init(
+service $name /system/bin/yes
+    user shell
+    group shell
+    seclabel $selabel
+)init";
+
+    std::string script = StringReplace(StringReplace(kScriptTemplate, "$name", name, false),
+                                       "$selabel", GetSecurityContext(), false);
+    ServiceList& service_list = ServiceList::GetInstance();
+    Parser parser;
+    parser.AddSectionParser("service",
+                            std::make_unique<ServiceParser>(&service_list, nullptr, std::nullopt));
+
+    TemporaryFile tf;
+    ASSERT_TRUE(tf.fd != -1);
+    ASSERT_TRUE(WriteStringToFd(script, tf.fd));
+    ASSERT_TRUE(parser.ParseConfig(tf.path));
+}
+
+TEST_F(RebootTest, StopServicesSIGTERM) {
+    if (getuid() != 0) {
+        GTEST_SKIP() << "Skipping test, must be run as root.";
+        return;
+    }
+
+    AddTestService("A");
+    AddTestService("B");
+
+    auto service_a = ServiceList::GetInstance().FindService("A");
+    ASSERT_NE(nullptr, service_a);
+    auto service_b = ServiceList::GetInstance().FindService("B");
+    ASSERT_NE(nullptr, service_b);
+
+    ASSERT_RESULT_OK(service_a->Start());
+    ASSERT_TRUE(service_a->IsRunning());
+    ASSERT_RESULT_OK(service_b->Start());
+    ASSERT_TRUE(service_b->IsRunning());
+
+    std::unique_ptr<Service> oneshot_service;
+    {
+        auto result = Service::MakeTemporaryOneshotService(
+                {"exec", GetSecurityContext(), "--", "/system/bin/yes"});
+        ASSERT_RESULT_OK(result);
+        oneshot_service = std::move(*result);
+    }
+    std::string oneshot_service_name = oneshot_service->name();
+    oneshot_service->Start();
+    ASSERT_TRUE(oneshot_service->IsRunning());
+    ServiceList::GetInstance().AddService(std::move(oneshot_service));
+
+    EXPECT_EQ(0, StopServicesAndLogViolations({"A", "B", oneshot_service_name}, 10s,
+                                              /* terminate= */ true));
+    EXPECT_FALSE(service_a->IsRunning());
+    EXPECT_FALSE(service_b->IsRunning());
+    // Oneshot services are deleted from the ServiceList after they are destroyed.
+    auto oneshot_service_after_stop = ServiceList::GetInstance().FindService(oneshot_service_name);
+    EXPECT_EQ(nullptr, oneshot_service_after_stop);
+}
+
+TEST_F(RebootTest, StopServicesSIGKILL) {
+    if (getuid() != 0) {
+        GTEST_SKIP() << "Skipping test, must be run as root.";
+        return;
+    }
+
+    AddTestService("A");
+    AddTestService("B");
+
+    auto service_a = ServiceList::GetInstance().FindService("A");
+    ASSERT_NE(nullptr, service_a);
+    auto service_b = ServiceList::GetInstance().FindService("B");
+    ASSERT_NE(nullptr, service_b);
+
+    ASSERT_RESULT_OK(service_a->Start());
+    ASSERT_TRUE(service_a->IsRunning());
+    ASSERT_RESULT_OK(service_b->Start());
+    ASSERT_TRUE(service_b->IsRunning());
+
+    std::unique_ptr<Service> oneshot_service;
+    {
+        auto result = Service::MakeTemporaryOneshotService(
+                {"exec", GetSecurityContext(), "--", "/system/bin/yes"});
+        ASSERT_RESULT_OK(result);
+        oneshot_service = std::move(*result);
+    }
+    std::string oneshot_service_name = oneshot_service->name();
+    oneshot_service->Start();
+    ASSERT_TRUE(oneshot_service->IsRunning());
+    ServiceList::GetInstance().AddService(std::move(oneshot_service));
+
+    EXPECT_EQ(0, StopServicesAndLogViolations({"A", "B", oneshot_service_name}, 10s,
+                                              /* terminate= */ false));
+    EXPECT_FALSE(service_a->IsRunning());
+    EXPECT_FALSE(service_b->IsRunning());
+    // Oneshot services are deleted from the ServiceList after they are destroyed.
+    auto oneshot_service_after_stop = ServiceList::GetInstance().FindService(oneshot_service_name);
+    EXPECT_EQ(nullptr, oneshot_service_after_stop);
+}
+
+}  // namespace init
+}  // namespace android
diff --git a/init/subcontext.cpp b/init/subcontext.cpp
index f1fbffe..fa48bea 100644
--- a/init/subcontext.cpp
+++ b/init/subcontext.cpp
@@ -351,6 +351,9 @@
 }
 
 bool SubcontextChildReap(pid_t pid) {
+    if (!subcontext) {
+        return false;
+    }
     if (subcontext->pid() == pid) {
         if (!subcontext_terminated_by_shutdown) {
             subcontext->Restart();
diff --git a/libutils/String16.cpp b/libutils/String16.cpp
index c42cada..68642d8 100644
--- a/libutils/String16.cpp
+++ b/libutils/String16.cpp
@@ -199,99 +199,59 @@
     return NO_MEMORY;
 }
 
-status_t String16::append(const String16& other)
-{
-    const size_t myLen = size();
-    const size_t otherLen = other.size();
-    if (myLen == 0) {
-        setTo(other);
-        return OK;
-    } else if (otherLen == 0) {
-        return OK;
-    }
-
-    if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) {
-        android_errorWriteLog(0x534e4554, "73826242");
-        abort();
-    }
-
-    SharedBuffer* buf =
-            static_cast<SharedBuffer*>(editResize((myLen + otherLen + 1) * sizeof(char16_t)));
-    if (buf) {
-        char16_t* str = (char16_t*)buf->data();
-        memcpy(str+myLen, other, (otherLen+1)*sizeof(char16_t));
-        mString = str;
-        return OK;
-    }
-    return NO_MEMORY;
+status_t String16::append(const String16& other) {
+    return append(other.string(), other.size());
 }
 
-status_t String16::append(const char16_t* chrs, size_t otherLen)
-{
+status_t String16::append(const char16_t* chrs, size_t otherLen) {
     const size_t myLen = size();
-    if (myLen == 0) {
-        setTo(chrs, otherLen);
-        return OK;
-    } else if (otherLen == 0) {
-        return OK;
-    }
 
-    if (myLen >= SIZE_MAX / sizeof(char16_t) - otherLen) {
-        android_errorWriteLog(0x534e4554, "73826242");
-        abort();
-    }
+    if (myLen == 0) return setTo(chrs, otherLen);
 
-    SharedBuffer* buf =
-            static_cast<SharedBuffer*>(editResize((myLen + otherLen + 1) * sizeof(char16_t)));
-    if (buf) {
-        char16_t* str = (char16_t*)buf->data();
-        memcpy(str+myLen, chrs, otherLen*sizeof(char16_t));
-        str[myLen+otherLen] = 0;
-        mString = str;
-        return OK;
-    }
-    return NO_MEMORY;
+    if (otherLen == 0) return OK;
+
+    size_t size = myLen;
+    if (__builtin_add_overflow(size, otherLen, &size) ||
+        __builtin_add_overflow(size, 1, &size) ||
+        __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY;
+
+    SharedBuffer* buf = static_cast<SharedBuffer*>(editResize(size));
+    if (!buf) return NO_MEMORY;
+
+    char16_t* str = static_cast<char16_t*>(buf->data());
+    memcpy(str + myLen, chrs, otherLen * sizeof(char16_t));
+    str[myLen + otherLen] = 0;
+    mString = str;
+    return OK;
 }
 
-status_t String16::insert(size_t pos, const char16_t* chrs)
-{
+status_t String16::insert(size_t pos, const char16_t* chrs) {
     return insert(pos, chrs, strlen16(chrs));
 }
 
-status_t String16::insert(size_t pos, const char16_t* chrs, size_t len)
-{
+status_t String16::insert(size_t pos, const char16_t* chrs, size_t otherLen) {
     const size_t myLen = size();
-    if (myLen == 0) {
-        return setTo(chrs, len);
-        return OK;
-    } else if (len == 0) {
-        return OK;
-    }
+
+    if (myLen == 0) return setTo(chrs, otherLen);
+
+    if (otherLen == 0) return OK;
 
     if (pos > myLen) pos = myLen;
 
-    #if 0
-    printf("Insert in to %s: pos=%d, len=%d, myLen=%d, chrs=%s\n",
-           String8(*this).string(), pos,
-           len, myLen, String8(chrs, len).string());
-    #endif
+    size_t size = myLen;
+    if (__builtin_add_overflow(size, otherLen, &size) ||
+        __builtin_add_overflow(size, 1, &size) ||
+        __builtin_mul_overflow(size, sizeof(char16_t), &size)) return NO_MEMORY;
 
-    SharedBuffer* buf =
-            static_cast<SharedBuffer*>(editResize((myLen + len + 1) * sizeof(char16_t)));
-    if (buf) {
-        char16_t* str = (char16_t*)buf->data();
-        if (pos < myLen) {
-            memmove(str+pos+len, str+pos, (myLen-pos)*sizeof(char16_t));
-        }
-        memcpy(str+pos, chrs, len*sizeof(char16_t));
-        str[myLen+len] = 0;
-        mString = str;
-        #if 0
-        printf("Result (%d chrs): %s\n", size(), String8(*this).string());
-        #endif
-        return OK;
-    }
-    return NO_MEMORY;
+    SharedBuffer* buf = static_cast<SharedBuffer*>(editResize(size));
+    if (!buf) return NO_MEMORY;
+
+    char16_t* str = static_cast<char16_t*>(buf->data());
+    if (pos < myLen) memmove(str + pos + otherLen, str + pos, (myLen - pos) * sizeof(char16_t));
+    memcpy(str + pos, chrs, otherLen * sizeof(char16_t));
+    str[myLen + otherLen] = 0;
+    mString = str;
+    return OK;
 }
 
 ssize_t String16::findFirst(char16_t c) const
diff --git a/libutils/String16_test.cpp b/libutils/String16_test.cpp
index 7d7230e..c6e6f74 100644
--- a/libutils/String16_test.cpp
+++ b/libutils/String16_test.cpp
@@ -19,7 +19,7 @@
 
 #include <gtest/gtest.h>
 
-namespace android {
+using namespace android;
 
 ::testing::AssertionResult Char16_tStringEquals(const char16_t* a, const char16_t* b) {
     if (strcmp16(a, b) != 0) {
@@ -224,4 +224,36 @@
     EXPECT_STR16EQ(another, u"abcdef");
 }
 
-}  // namespace android
+TEST(String16Test, append) {
+    String16 s;
+    EXPECT_EQ(OK, s.append(String16(u"foo")));
+    EXPECT_STR16EQ(u"foo", s);
+    EXPECT_EQ(OK, s.append(String16(u"bar")));
+    EXPECT_STR16EQ(u"foobar", s);
+    EXPECT_EQ(OK, s.append(u"baz", 0));
+    EXPECT_STR16EQ(u"foobar", s);
+    EXPECT_EQ(NO_MEMORY, s.append(u"baz", SIZE_MAX));
+    EXPECT_STR16EQ(u"foobar", s);
+}
+
+TEST(String16Test, insert) {
+    String16 s;
+
+    // Inserting into the empty string inserts at the start.
+    EXPECT_EQ(OK, s.insert(123, u"foo"));
+    EXPECT_STR16EQ(u"foo", s);
+
+    // Inserting zero characters at any position is okay, but won't expand the string.
+    EXPECT_EQ(OK, s.insert(123, u"foo", 0));
+    EXPECT_STR16EQ(u"foo", s);
+
+    // Inserting past the end of a non-empty string appends.
+    EXPECT_EQ(OK, s.insert(123, u"bar"));
+    EXPECT_STR16EQ(u"foobar", s);
+
+    EXPECT_EQ(OK, s.insert(3, u"!"));
+    EXPECT_STR16EQ(u"foo!bar", s);
+
+    EXPECT_EQ(NO_MEMORY, s.insert(3, u"", SIZE_MAX));
+    EXPECT_STR16EQ(u"foo!bar", s);
+}
diff --git a/libutils/String8.cpp b/libutils/String8.cpp
index 8511da9..419b2de 100644
--- a/libutils/String8.cpp
+++ b/libutils/String8.cpp
@@ -313,8 +313,8 @@
 
     if (n > 0) {
         size_t oldLength = length();
-        if ((size_t)n > SIZE_MAX - 1 ||
-            oldLength > SIZE_MAX - (size_t)n - 1) {
+        if (n > std::numeric_limits<size_t>::max() - 1 ||
+            oldLength > std::numeric_limits<size_t>::max() - n - 1) {
             return NO_MEMORY;
         }
         char* buf = lockBuffer(oldLength + n);
@@ -327,21 +327,23 @@
     return result;
 }
 
-status_t String8::real_append(const char* other, size_t otherLen)
-{
+status_t String8::real_append(const char* other, size_t otherLen) {
     const size_t myLen = bytes();
 
-    SharedBuffer* buf = SharedBuffer::bufferFromData(mString)
-        ->editResize(myLen+otherLen+1);
-    if (buf) {
-        char* str = (char*)buf->data();
-        mString = str;
-        str += myLen;
-        memcpy(str, other, otherLen);
-        str[otherLen] = '\0';
-        return OK;
+    SharedBuffer* buf;
+    size_t newLen;
+    if (__builtin_add_overflow(myLen, otherLen, &newLen) ||
+        __builtin_add_overflow(newLen, 1, &newLen) ||
+        (buf = SharedBuffer::bufferFromData(mString)->editResize(newLen)) == nullptr) {
+        return NO_MEMORY;
     }
-    return NO_MEMORY;
+
+    char* str = (char*)buf->data();
+    mString = str;
+    str += myLen;
+    memcpy(str, other, otherLen);
+    str[otherLen] = '\0';
+    return OK;
 }
 
 char* String8::lockBuffer(size_t size)
diff --git a/libutils/String8_test.cpp b/libutils/String8_test.cpp
index 9efcc6f..1356cd0 100644
--- a/libutils/String8_test.cpp
+++ b/libutils/String8_test.cpp
@@ -15,13 +15,14 @@
  */
 
 #define LOG_TAG "String8_test"
+
 #include <utils/Log.h>
 #include <utils/String8.h>
 #include <utils/String16.h>
 
 #include <gtest/gtest.h>
 
-namespace android {
+using namespace android;
 
 class String8Test : public testing::Test {
 protected:
@@ -101,4 +102,15 @@
     String8 valid = String8(String16(tmp));
     EXPECT_STREQ(valid, "abcdef");
 }
+
+TEST_F(String8Test, append) {
+    String8 s;
+    EXPECT_EQ(OK, s.append("foo"));
+    EXPECT_STREQ("foo", s);
+    EXPECT_EQ(OK, s.append("bar"));
+    EXPECT_STREQ("foobar", s);
+    EXPECT_EQ(OK, s.append("baz", 0));
+    EXPECT_STREQ("foobar", s);
+    EXPECT_EQ(NO_MEMORY, s.append("baz", SIZE_MAX));
+    EXPECT_STREQ("foobar", s);
 }
diff --git a/trusty/keymaster/Android.bp b/trusty/keymaster/Android.bp
index aa610e7..ff6460d 100644
--- a/trusty/keymaster/Android.bp
+++ b/trusty/keymaster/Android.bp
@@ -129,6 +129,27 @@
     src: "set_attestation_key/keymaster_soft_attestation_keys.xml",
 }
 
+cc_library {
+    name: "libtrusty_ipc",
+    vendor: true,
+    srcs: ["ipc/trusty_keymaster_ipc.cpp"],
+    local_include_dirs: ["include"],
+    shared_libs: [
+        "libc",
+        "libcrypto",
+        "liblog",
+        "libtrusty",
+        "libhardware",
+        "libkeymaster_messages",
+        "libxml2",
+    ],
+    export_include_dirs: ["include"],
+    cflags: [
+        "-Wall",
+        "-Werror",
+    ],
+}
+
 cc_binary {
     name: "trusty_keymaster_set_attestation_key",
     vendor: true,
diff --git a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h
index 6f4713b..17fee15 100644
--- a/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h
+++ b/trusty/keymaster/include/trusty_keymaster/ipc/keymaster_ipc.h
@@ -71,6 +71,7 @@
     KM_SET_PRODUCT_ID = (0x9000 << KEYMASTER_REQ_SHIFT),
     KM_CLEAR_ATTESTATION_CERT_CHAIN = (0xa000 << KEYMASTER_REQ_SHIFT),
     KM_SET_WRAPPED_ATTESTATION_KEY = (0xb000 << KEYMASTER_REQ_SHIFT),
+    KM_SET_ATTESTATION_IDS = (0xc000 << KEYMASTER_REQ_SHIFT)
 };
 
 #ifdef __ANDROID__
diff --git a/trusty/trusty-base.mk b/trusty/trusty-base.mk
index 12521b0..b42d665 100644
--- a/trusty/trusty-base.mk
+++ b/trusty/trusty-base.mk
@@ -23,7 +23,7 @@
 # HAL loading of gatekeeper.trusty.
 
 PRODUCT_PACKAGES += \
-	android.hardware.keymaster@4.0-service.trusty \
+	android.hardware.security.keymint-service.trusty \
 	android.hardware.gatekeeper@1.0-service.trusty \
 	trusty_apploader