Android-x86
Fork
Spenden

  • R/O
  • HTTP
  • SSH
  • HTTPS

system-core: Commit

system/core


Commit MetaInfo

Revision9f5a889b0c70d5d021672c36cc8b340a6961b6a9 (tree)
Zeit2016-09-07 15:06:47
AutorChih-Wei Huang <cwhuang@linu...>
CommiterChih-Wei Huang

Log Message

Android 6.0.1 release 66
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iEYEABECAAYFAlfO7mkACgkQ6K0/gZqxDniqrQCdFR+6OEohyLsDnMK3AuXJaQA+
rtQAniY0clzoDQSIpoOItDPjsmyGKvBN
=ocxE
-----END PGP SIGNATURE-----

Merge tag 'android-6.0.1_r66' into marshmallow-x86

Android 6.0.1 release 66

Ändern Zusammenfassung

Diff

--- a/adb/Android.mk
+++ b/adb/Android.mk
@@ -200,7 +200,10 @@ endif
200200 # will violate ODR
201201 LOCAL_SHARED_LIBRARIES :=
202202
203+# Don't build the host adb on Windows (this branch should only be used for security updates.)
204+ifneq ($(HOST_OS),windows)
203205 include $(BUILD_HOST_EXECUTABLE)
206+endif
204207
205208 $(call dist-for-goals,dist_files sdk,$(LOCAL_BUILT_MODULE))
206209
--- a/adb/mutex_list.h
+++ b/adb/mutex_list.h
@@ -6,7 +6,6 @@
66 #ifndef ADB_MUTEX
77 #error ADB_MUTEX not defined when including this file
88 #endif
9-ADB_MUTEX(socket_list_lock)
109 ADB_MUTEX(transport_lock)
1110 ADB_MUTEX(fdevent_lock)
1211 #if ADB_HOST
--- a/adb/sockets.cpp
+++ b/adb/sockets.cpp
@@ -25,18 +25,25 @@
2525 #include <string.h>
2626 #include <unistd.h>
2727
28+#include <algorithm>
29+#include <mutex>
30+#include <string>
31+#include <vector>
32+
2833 #if !ADB_HOST
2934 #include "cutils/properties.h"
3035 #endif
3136
3237 #include "adb.h"
3338 #include "adb_io.h"
39+#include "sysdeps/mutex.h"
3440 #include "transport.h"
3541
36-ADB_MUTEX_DEFINE( socket_list_lock );
37-
38-static void local_socket_close_locked(asocket *s);
42+#if !defined(__BIONIC__)
43+using std::recursive_mutex;
44+#endif
3945
46+static recursive_mutex& local_socket_list_lock = *new recursive_mutex();
4047 static unsigned local_socket_next_id = 1;
4148
4249 static asocket local_socket_list = {
@@ -61,7 +68,7 @@ asocket *find_local_socket(unsigned local_id, unsigned peer_id)
6168 asocket *s;
6269 asocket *result = NULL;
6370
64- adb_mutex_lock(&socket_list_lock);
71+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
6572 for (s = local_socket_list.next; s != &local_socket_list; s = s->next) {
6673 if (s->id != local_id)
6774 continue;
@@ -70,7 +77,6 @@ asocket *find_local_socket(unsigned local_id, unsigned peer_id)
7077 }
7178 break;
7279 }
73- adb_mutex_unlock(&socket_list_lock);
7480
7581 return result;
7682 }
@@ -84,20 +90,17 @@ insert_local_socket(asocket* s, asocket* list)
8490 s->next->prev = s;
8591 }
8692
87-
88-void install_local_socket(asocket *s)
89-{
90- adb_mutex_lock(&socket_list_lock);
93+void install_local_socket(asocket* s) {
94+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
9195
9296 s->id = local_socket_next_id++;
9397
9498 // Socket ids should never be 0.
95- if (local_socket_next_id == 0)
96- local_socket_next_id = 1;
99+ if (local_socket_next_id == 0) {
100+ fatal("local socket id overflow");
101+ }
97102
98103 insert_local_socket(s, &local_socket_list);
99-
100- adb_mutex_unlock(&socket_list_lock);
101104 }
102105
103106 void remove_socket(asocket *s)
@@ -116,19 +119,17 @@ void remove_socket(asocket *s)
116119 void close_all_sockets(atransport *t)
117120 {
118121 asocket *s;
119-
120- /* this is a little gross, but since s->close() *will* modify
121- ** the list out from under you, your options are limited.
122- */
123- adb_mutex_lock(&socket_list_lock);
122+ /* this is a little gross, but since s->close() *will* modify
123+ ** the list out from under you, your options are limited.
124+ */
125+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
124126 restart:
125- for(s = local_socket_list.next; s != &local_socket_list; s = s->next){
126- if(s->transport == t || (s->peer && s->peer->transport == t)) {
127- local_socket_close_locked(s);
127+ for (s = local_socket_list.next; s != &local_socket_list; s = s->next) {
128+ if (s->transport == t || (s->peer && s->peer->transport == t)) {
129+ s->close(s);
128130 goto restart;
129131 }
130132 }
131- adb_mutex_unlock(&socket_list_lock);
132133 }
133134
134135 static int local_socket_enqueue(asocket *s, apacket *p)
@@ -191,13 +192,6 @@ static void local_socket_ready(asocket *s)
191192 fdevent_add(&s->fde, FDE_READ);
192193 }
193194
194-static void local_socket_close(asocket *s)
195-{
196- adb_mutex_lock(&socket_list_lock);
197- local_socket_close_locked(s);
198- adb_mutex_unlock(&socket_list_lock);
199-}
200-
201195 // be sure to hold the socket list lock when calling this
202196 static void local_socket_destroy(asocket *s)
203197 {
@@ -226,27 +220,21 @@ static void local_socket_destroy(asocket *s)
226220 }
227221 }
228222
229-
230-static void local_socket_close_locked(asocket *s)
231-{
232- D("entered local_socket_close_locked. LS(%d) fd=%d\n", s->id, s->fd);
233- if(s->peer) {
234- D("LS(%d): closing peer. peer->id=%d peer->fd=%d\n",
235- s->id, s->peer->id, s->peer->fd);
223+static void local_socket_close(asocket* s) {
224+ D("entered local_socket_close. LS(%d) fd=%d", s->id, s->fd);
225+ std::lock_guard<recursive_mutex> lock(local_socket_list_lock);
226+ if (s->peer) {
227+ D("LS(%d): closing peer. peer->id=%d peer->fd=%d", s->id, s->peer->id, s->peer->fd);
236228 /* Note: it's important to call shutdown before disconnecting from
237229 * the peer, this ensures that remote sockets can still get the id
238230 * of the local socket they're connected to, to send a CLOSE()
239231 * protocol event. */
240- if (s->peer->shutdown)
241- s->peer->shutdown(s->peer);
242- s->peer->peer = 0;
243- // tweak to avoid deadlock
244- if (s->peer->close == local_socket_close) {
245- local_socket_close_locked(s->peer);
246- } else {
247- s->peer->close(s->peer);
232+ if (s->peer->shutdown) {
233+ s->peer->shutdown(s->peer);
248234 }
249- s->peer = 0;
235+ s->peer->peer = nullptr;
236+ s->peer->close(s->peer);
237+ s->peer = nullptr;
250238 }
251239
252240 /* If we are already closing, or if there are no
--- /dev/null
+++ b/adb/sysdeps/mutex.h
@@ -0,0 +1,143 @@
1+#pragma once
2+
3+/*
4+ * Copyright (C) 2016 The Android Open Source Project
5+ *
6+ * Licensed under the Apache License, Version 2.0 (the "License");
7+ * you may not use this file except in compliance with the License.
8+ * You may obtain a copy of the License at
9+ *
10+ * http://www.apache.org/licenses/LICENSE-2.0
11+ *
12+ * Unless required by applicable law or agreed to in writing, software
13+ * distributed under the License is distributed on an "AS IS" BASIS,
14+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+ * See the License for the specific language governing permissions and
16+ * limitations under the License.
17+ */
18+
19+#if defined(_WIN32)
20+
21+#include <windows.h>
22+
23+#include <android-base/macros.h>
24+
25+#include "adb.h"
26+
27+// The prebuilt version of mingw we use doesn't support mutex or recursive_mutex.
28+// Therefore, implement our own using the Windows primitives.
29+// Put them directly into the std namespace, so that when they're actually available, the build
30+// breaks until they're removed.
31+
32+#include <mutex>
33+namespace std {
34+
35+// CRITICAL_SECTION is recursive, so just wrap it in a Mutex-compatible class.
36+class recursive_mutex {
37+ public:
38+ recursive_mutex() {
39+ InitializeCriticalSection(&mutex_);
40+ }
41+
42+ ~recursive_mutex() {
43+ DeleteCriticalSection(&mutex_);
44+ }
45+
46+ void lock() {
47+ EnterCriticalSection(&mutex_);
48+ }
49+
50+ bool try_lock() {
51+ return TryEnterCriticalSection(&mutex_);
52+ }
53+
54+ void unlock() {
55+ LeaveCriticalSection(&mutex_);
56+ }
57+
58+ private:
59+ CRITICAL_SECTION mutex_;
60+
61+ DISALLOW_COPY_AND_ASSIGN(recursive_mutex);
62+};
63+
64+class mutex {
65+ public:
66+ mutex() {
67+ }
68+
69+ ~mutex() {
70+ }
71+
72+ void lock() {
73+ mutex_.lock();
74+ if (++lock_count_ != 1) {
75+ fatal("non-recursive mutex locked reentrantly");
76+ }
77+ }
78+
79+ void unlock() {
80+ if (--lock_count_ != 0) {
81+ fatal("non-recursive mutex unlock resulted in unexpected lock count: %d", lock_count_);
82+ }
83+ mutex_.unlock();
84+ }
85+
86+ bool try_lock() {
87+ if (!mutex_.try_lock()) {
88+ return false;
89+ }
90+
91+ if (lock_count_ != 0) {
92+ mutex_.unlock();
93+ return false;
94+ }
95+
96+ ++lock_count_;
97+ return true;
98+ }
99+
100+ private:
101+ recursive_mutex mutex_;
102+ size_t lock_count_ = 0;
103+};
104+
105+}
106+
107+#elif defined(__BIONIC__)
108+
109+// On M, the recovery image uses parts of adb that depends on recursive_mutex, and uses libstdc++,
110+// which lacks it.
111+
112+#include <pthread.h>
113+#include <mutex>
114+
115+#include <base/macros.h>
116+
117+class recursive_mutex {
118+ public:
119+ recursive_mutex() {
120+ }
121+
122+ ~recursive_mutex() {
123+ }
124+
125+ void lock() {
126+ pthread_mutex_lock(&mutex_);
127+ }
128+
129+ bool try_lock() {
130+ return pthread_mutex_trylock(&mutex_);
131+ }
132+
133+ void unlock() {
134+ pthread_mutex_unlock(&mutex_);
135+ }
136+
137+ private:
138+ pthread_mutex_t mutex_ = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
139+
140+ DISALLOW_COPY_AND_ASSIGN(recursive_mutex);
141+};
142+
143+#endif
--- a/debuggerd/backtrace.cpp
+++ b/debuggerd/backtrace.cpp
@@ -67,8 +67,8 @@ static void dump_process_footer(log_t* log, pid_t pid) {
6767 _LOG(log, logtype::BACKTRACE, "\n----- end %d -----\n", pid);
6868 }
6969
70-static void dump_thread(
71- log_t* log, pid_t tid, bool attached, bool* detach_failed, int* total_sleep_time_usec) {
70+static void dump_thread(log_t* log, pid_t pid, pid_t tid, bool attached,
71+ bool* detach_failed, int* total_sleep_time_usec) {
7272 char path[PATH_MAX];
7373 char threadnamebuf[1024];
7474 char* threadname = NULL;
@@ -88,7 +88,7 @@ static void dump_thread(
8888
8989 _LOG(log, logtype::BACKTRACE, "\n\"%s\" sysTid=%d\n", threadname ? threadname : "<unknown>", tid);
9090
91- if (!attached && ptrace(PTRACE_ATTACH, tid, 0, 0) < 0) {
91+ if (!attached && !ptrace_attach_thread(pid, tid)) {
9292 _LOG(log, logtype::BACKTRACE, "Could not attach to thread: %s\n", strerror(errno));
9393 return;
9494 }
@@ -117,7 +117,7 @@ void dump_backtrace(int fd, int amfd, pid_t pid, pid_t tid, bool* detach_failed,
117117 log.amfd = amfd;
118118
119119 dump_process_header(&log, pid);
120- dump_thread(&log, tid, true, detach_failed, total_sleep_time_usec);
120+ dump_thread(&log, pid, tid, true, detach_failed, total_sleep_time_usec);
121121
122122 char task_path[64];
123123 snprintf(task_path, sizeof(task_path), "/proc/%d/task", pid);
@@ -135,7 +135,7 @@ void dump_backtrace(int fd, int amfd, pid_t pid, pid_t tid, bool* detach_failed,
135135 continue;
136136 }
137137
138- dump_thread(&log, new_tid, false, detach_failed, total_sleep_time_usec);
138+ dump_thread(&log, pid, new_tid, false, detach_failed, total_sleep_time_usec);
139139 }
140140 closedir(d);
141141 }
--- a/debuggerd/debuggerd.cpp
+++ b/debuggerd/debuggerd.cpp
@@ -225,12 +225,10 @@ static int read_request(int fd, debugger_request_t* out_request) {
225225
226226 if (msg.action == DEBUGGER_ACTION_CRASH) {
227227 // Ensure that the tid reported by the crashing process is valid.
228- char buf[64];
229- struct stat s;
230- snprintf(buf, sizeof buf, "/proc/%d/task/%d", out_request->pid, out_request->tid);
231- if (stat(buf, &s)) {
228+ // This check needs to happen again after ptracing the requested thread to prevent a race.
229+ if (!pid_contains_tid(out_request->pid, out_request->tid)) {
232230 ALOGE("tid %d does not exist in pid %d. ignoring debug request\n",
233- out_request->tid, out_request->pid);
231+ out_request->tid, out_request->pid);
234232 return -1;
235233 }
236234 } else if (cr.uid == 0
@@ -380,9 +378,32 @@ static void handle_request(int fd) {
380378 // ensure that it can run as soon as we call PTRACE_CONT below.
381379 // See details in bionic/libc/linker/debugger.c, in function
382380 // debugger_signal_handler().
383- if (ptrace(PTRACE_ATTACH, request.tid, 0, 0)) {
381+ if (!ptrace_attach_thread(request.pid, request.tid)) {
384382 ALOGE("ptrace attach failed: %s\n", strerror(errno));
385383 } else {
384+ // DEBUGGER_ACTION_CRASH requests can come from arbitrary processes and the tid field in
385+ // the request is sent from the other side. If an attacker can cause a process to be
386+ // spawned with the pid of their process, they could trick debuggerd into dumping that
387+ // process by exiting after sending the request. Validate the trusted request.uid/gid
388+ // to defend against this.
389+ if (request.action == DEBUGGER_ACTION_CRASH) {
390+ pid_t pid;
391+ uid_t uid;
392+ gid_t gid;
393+ if (get_process_info(request.tid, &pid, &uid, &gid) != 0) {
394+ ALOGE("debuggerd: failed to get process info for tid '%d'", request.tid);
395+ exit(1);
396+ }
397+
398+ if (pid != request.pid || uid != request.uid || gid != request.gid) {
399+ ALOGE(
400+ "debuggerd: attached task %d does not match request: "
401+ "expected pid=%d,uid=%d,gid=%d, actual pid=%d,uid=%d,gid=%d",
402+ request.tid, request.pid, request.uid, request.gid, pid, uid, gid);
403+ exit(1);
404+ }
405+ }
406+
386407 bool detach_failed = false;
387408 bool tid_unresponsive = false;
388409 bool attach_gdb = should_attach_gdb(&request);
--- a/debuggerd/tombstone.cpp
+++ b/debuggerd/tombstone.cpp
@@ -447,7 +447,7 @@ static bool dump_sibling_thread_report(
447447 }
448448
449449 // Skip this thread if cannot ptrace it
450- if (ptrace(PTRACE_ATTACH, new_tid, 0, 0) < 0) {
450+ if (!ptrace_attach_thread(pid, new_tid)) {
451451 _LOG(log, logtype::ERROR, "ptrace attach to %d failed: %s\n", new_tid, strerror(errno));
452452 continue;
453453 }
--- a/debuggerd/utility.cpp
+++ b/debuggerd/utility.cpp
@@ -20,6 +20,7 @@
2020
2121 #include <errno.h>
2222 #include <signal.h>
23+#include <stdlib.h>
2324 #include <string.h>
2425 #include <unistd.h>
2526 #include <sys/ptrace.h>
@@ -207,3 +208,31 @@ void dump_memory(log_t* log, Backtrace* backtrace, uintptr_t addr, const char* f
207208 _LOG(log, logtype::MEMORY, "%s %s\n", logline.c_str(), ascii.c_str());
208209 }
209210 }
211+
212+bool pid_contains_tid(pid_t pid, pid_t tid) {
213+ char task_path[PATH_MAX];
214+ if (snprintf(task_path, PATH_MAX, "/proc/%d/task/%d", pid, tid) >= PATH_MAX) {
215+ ALOGE("debuggerd: task path overflow (pid = %d, tid = %d)\n", pid, tid);
216+ exit(1);
217+ }
218+
219+ return access(task_path, F_OK) == 0;
220+}
221+
222+// Attach to a thread, and verify that it's still a member of the given process
223+bool ptrace_attach_thread(pid_t pid, pid_t tid) {
224+ if (ptrace(PTRACE_ATTACH, tid, 0, 0) != 0) {
225+ return false;
226+ }
227+
228+ // Make sure that the task we attached to is actually part of the pid we're dumping.
229+ if (!pid_contains_tid(pid, tid)) {
230+ if (ptrace(PTRACE_DETACH, tid, 0, 0) != 0) {
231+ ALOGE("debuggerd: failed to detach from thread '%d'", tid);
232+ exit(1);
233+ }
234+ return false;
235+ }
236+
237+ return true;
238+}
--- a/debuggerd/utility.h
+++ b/debuggerd/utility.h
@@ -79,4 +79,9 @@ int wait_for_sigstop(pid_t, int*, bool*);
7979
8080 void dump_memory(log_t* log, Backtrace* backtrace, uintptr_t addr, const char* fmt, ...);
8181
82+bool pid_contains_tid(pid_t pid, pid_t tid);
83+
84+// Attach to a thread, and verify that it's still a member of the given process
85+bool ptrace_attach_thread(pid_t pid, pid_t tid);
86+
8287 #endif // _DEBUGGERD_UTILITY_H
--- a/include/utils/Unicode.h
+++ b/include/utils/Unicode.h
@@ -87,7 +87,7 @@ ssize_t utf32_to_utf8_length(const char32_t *src, size_t src_len);
8787 * "dst" becomes \xE3\x81\x82\xE3\x81\x84
8888 * (note that "dst" is NOT null-terminated, like strncpy)
8989 */
90-void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst);
90+void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst, size_t dst_len);
9191
9292 /**
9393 * Returns the unicode value at "index".
@@ -109,7 +109,7 @@ ssize_t utf16_to_utf8_length(const char16_t *src, size_t src_len);
109109 * enough to fit the UTF-16 as measured by utf16_to_utf8_length with an added
110110 * NULL terminator.
111111 */
112-void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst);
112+void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst, size_t dst_len);
113113
114114 /**
115115 * Returns the length of "src" when "src" is valid UTF-8 string.
--- a/libutils/String8.cpp
+++ b/libutils/String8.cpp
@@ -102,20 +102,21 @@ static char* allocFromUTF16(const char16_t* in, size_t len)
102102 {
103103 if (len == 0) return getEmptyString();
104104
105- const ssize_t bytes = utf16_to_utf8_length(in, len);
106- if (bytes < 0) {
105+ // Allow for closing '\0'
106+ const ssize_t resultStrLen = utf16_to_utf8_length(in, len) + 1;
107+ if (resultStrLen < 1) {
107108 return getEmptyString();
108109 }
109110
110- SharedBuffer* buf = SharedBuffer::alloc(bytes+1);
111+ SharedBuffer* buf = SharedBuffer::alloc(resultStrLen);
111112 ALOG_ASSERT(buf, "Unable to allocate shared buffer");
112113 if (!buf) {
113114 return getEmptyString();
114115 }
115116
116- char* str = (char*)buf->data();
117- utf16_to_utf8(in, len, str);
118- return str;
117+ char* resultStr = (char*)buf->data();
118+ utf16_to_utf8(in, len, resultStr, resultStrLen);
119+ return resultStr;
119120 }
120121
121122 static char* allocFromUTF32(const char32_t* in, size_t len)
@@ -124,21 +125,21 @@ static char* allocFromUTF32(const char32_t* in, size_t len)
124125 return getEmptyString();
125126 }
126127
127- const ssize_t bytes = utf32_to_utf8_length(in, len);
128- if (bytes < 0) {
128+ const ssize_t resultStrLen = utf32_to_utf8_length(in, len) + 1;
129+ if (resultStrLen < 1) {
129130 return getEmptyString();
130131 }
131132
132- SharedBuffer* buf = SharedBuffer::alloc(bytes+1);
133+ SharedBuffer* buf = SharedBuffer::alloc(resultStrLen);
133134 ALOG_ASSERT(buf, "Unable to allocate shared buffer");
134135 if (!buf) {
135136 return getEmptyString();
136137 }
137138
138- char* str = (char*) buf->data();
139- utf32_to_utf8(in, len, str);
139+ char* resultStr = (char*) buf->data();
140+ utf32_to_utf8(in, len, resultStr, resultStrLen);
140141
141- return str;
142+ return resultStr;
142143 }
143144
144145 // ---------------------------------------------------------------------------
--- a/libutils/Unicode.cpp
+++ b/libutils/Unicode.cpp
@@ -14,6 +14,7 @@
1414 * limitations under the License.
1515 */
1616
17+#include <log/log.h>
1718 #include <utils/Unicode.h>
1819
1920 #include <stddef.h>
@@ -182,7 +183,7 @@ ssize_t utf32_to_utf8_length(const char32_t *src, size_t src_len)
182183 return ret;
183184 }
184185
185-void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst)
186+void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst, size_t dst_len)
186187 {
187188 if (src == NULL || src_len == 0 || dst == NULL) {
188189 return;
@@ -193,9 +194,12 @@ void utf32_to_utf8(const char32_t* src, size_t src_len, char* dst)
193194 char *cur = dst;
194195 while (cur_utf32 < end_utf32) {
195196 size_t len = utf32_codepoint_utf8_length(*cur_utf32);
197+ LOG_ALWAYS_FATAL_IF(dst_len < len, "%zu < %zu", dst_len, len);
196198 utf32_codepoint_to_utf8((uint8_t *)cur, *cur_utf32++, len);
197199 cur += len;
200+ dst_len -= len;
198201 }
202+ LOG_ALWAYS_FATAL_IF(dst_len < 1, "dst_len < 1: %zu < 1", dst_len);
199203 *cur = '\0';
200204 }
201205
@@ -324,7 +328,7 @@ int strzcmp16_h_n(const char16_t *s1H, size_t n1, const char16_t *s2N, size_t n2
324328 : 0);
325329 }
326330
327-void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst)
331+void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst, size_t dst_len)
328332 {
329333 if (src == NULL || src_len == 0 || dst == NULL) {
330334 return;
@@ -345,9 +349,12 @@ void utf16_to_utf8(const char16_t* src, size_t src_len, char* dst)
345349 utf32 = (char32_t) *cur_utf16++;
346350 }
347351 const size_t len = utf32_codepoint_utf8_length(utf32);
352+ LOG_ALWAYS_FATAL_IF(dst_len < len, "%zu < %zu", dst_len, len);
348353 utf32_codepoint_to_utf8((uint8_t*)cur, utf32, len);
349354 cur += len;
355+ dst_len -= len;
350356 }
357+ LOG_ALWAYS_FATAL_IF(dst_len < 1, "%zu < 1", dst_len);
351358 *cur = '\0';
352359 }
353360
@@ -408,10 +415,10 @@ ssize_t utf16_to_utf8_length(const char16_t *src, size_t src_len)
408415 const char16_t* const end = src + src_len;
409416 while (src < end) {
410417 if ((*src & 0xFC00) == 0xD800 && (src + 1) < end
411- && (*++src & 0xFC00) == 0xDC00) {
418+ && (*(src + 1) & 0xFC00) == 0xDC00) {
412419 // surrogate pairs are always 4 bytes.
413420 ret += 4;
414- src++;
421+ src += 2;
415422 } else {
416423 ret += utf32_codepoint_utf8_length((char32_t) *src++);
417424 }
--- a/libutils/tests/String8_test.cpp
+++ b/libutils/tests/String8_test.cpp
@@ -17,6 +17,7 @@
1717 #define LOG_TAG "String8_test"
1818 #include <utils/Log.h>
1919 #include <utils/String8.h>
20+#include <utils/String16.h>
2021
2122 #include <gtest/gtest.h>
2223
@@ -72,4 +73,22 @@ TEST_F(String8Test, OperatorPlusEquals) {
7273 EXPECT_STREQ(src3, " Verify me.");
7374 }
7475
76+// http://b/29250543
77+TEST_F(String8Test, CorrectInvalidSurrogate) {
78+ // d841d8 is an invalid start for a surrogate pair. Make sure this is handled by ignoring the
79+ // first character in the pair and handling the rest correctly.
80+ String16 string16(u"\xd841\xd841\xdc41\x0000");
81+ String8 string8(string16);
82+
83+ EXPECT_EQ(4U, string8.length());
84+}
85+
86+TEST_F(String8Test, CheckUtf32Conversion) {
87+ // Since bound checks were added, check the conversion can be done without fatal errors.
88+ // The utf8 lengths of these are chars are 1 + 2 + 3 + 4 = 10.
89+ const char32_t string32[] = U"\x0000007f\x000007ff\x0000911\x0010fffe";
90+ String8 string8(string32);
91+ EXPECT_EQ(10U, string8.length());
92+}
93+
7594 }
Show on old repository browser