Merge "Track movement of some libcore/tzdata files"
diff --git a/include/ziparchive/zip_archive.h b/include/ziparchive/zip_archive.h
index 31fc2df..ece8693 100644
--- a/include/ziparchive/zip_archive.h
+++ b/include/ziparchive/zip_archive.h
@@ -71,8 +71,17 @@
   // Modification time. The zipfile format specifies
   // that the first two little endian bytes contain the time
   // and the last two little endian bytes contain the date.
+  // See `GetModificationTime`.
+  // TODO: should be overridden by extra time field, if present.
   uint32_t mod_time;
 
+  // Returns `mod_time` as a broken-down struct tm.
+  struct tm GetModificationTime() const;
+
+  // Suggested Unix mode for this entry, from the zip archive if created on
+  // Unix, or a default otherwise.
+  mode_t unix_mode;
+
   // 1 if this entry contains a data descriptor segment, 0
   // otherwise.
   uint8_t has_data_descriptor;
diff --git a/init/Android.bp b/init/Android.bp
index af1e9d3..fce424e 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -20,7 +20,6 @@
     sanitize: {
         misc_undefined: ["integer"],
     },
-    tidy_checks: ["-misc-forwarding-reference-overload"],
     cppflags: [
         "-DLOG_UEVENTS=0",
         "-Wall",
diff --git a/init/Android.mk b/init/Android.mk
index 489d076..6cd47f4 100644
--- a/init/Android.mk
+++ b/init/Android.mk
@@ -40,8 +40,6 @@
 # --
 
 include $(CLEAR_VARS)
-# b/38002385, work around clang-tidy segmentation fault.
-LOCAL_TIDY_CHECKS := -misc-forwarding-reference-overload
 LOCAL_CPPFLAGS := $(init_cflags)
 LOCAL_SRC_FILES:= \
     bootchart.cpp \
diff --git a/init/service.cpp b/init/service.cpp
index 7c931da..1a6474b 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -209,17 +209,6 @@
 }
 
 void Service::KillProcessGroup(int signal) {
-    // We ignore reporting errors of ESRCH as this commonly happens in the below case,
-    // 1) Terminate() is called, which sends SIGTERM to the process
-    // 2) The process successfully exits
-    // 3) ReapOneProcess() is called, which calls waitpid(-1, ...) which removes the pid entry.
-    // 4) Reap() is called, which sends SIGKILL, but the pid no longer exists.
-    // TODO: sigaction for SIGCHLD reports the pid of the exiting process,
-    // we should do this kill with that pid first before calling waitpid().
-    if (kill(-pid_, signal) == -1 && errno != ESRCH) {
-        PLOG(ERROR) << "kill(" << pid_ << ", " << signal << ") failed";
-    }
-
     // If we've already seen a successful result from killProcessGroup*(), then we have removed
     // the cgroup already and calling these functions a second time will simply result in an error.
     // This is true regardless of which signal was sent.
diff --git a/libmemunreachable/MemUnreachable.cpp b/libmemunreachable/MemUnreachable.cpp
index e7c0beb..1c84744 100644
--- a/libmemunreachable/MemUnreachable.cpp
+++ b/libmemunreachable/MemUnreachable.cpp
@@ -502,7 +502,10 @@
 std::string GetUnreachableMemoryString(bool log_contents, size_t limit) {
   UnreachableMemoryInfo info;
   if (!GetUnreachableMemory(info, limit)) {
-    return "Failed to get unreachable memory\n";
+    return "Failed to get unreachable memory\n"
+           "If you are trying to get unreachable memory from a system app\n"
+           "(like com.android.systemui), disable selinux first using\n"
+           "setenforce 0\n";
   }
 
   return info.ToString(log_contents);
diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index 27b4065..f5d4e1c 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -27,10 +27,12 @@
 #include <string.h>
 #include <sys/stat.h>
 #include <sys/types.h>
+#include <unistd.h>
 
 #include <chrono>
 #include <memory>
 #include <mutex>
+#include <set>
 #include <thread>
 
 #include <android-base/logging.h>
@@ -258,6 +260,12 @@
         return -errno;
     }
 
+    // We separate all of the pids in the cgroup into those pids that are also the leaders of
+    // process groups (stored in the pgids set) and those that are not (stored in the pids set).
+    std::set<pid_t> pgids;
+    pgids.emplace(initialPid);
+    std::set<pid_t> pids;
+
     int ret;
     pid_t pid;
     int processes = 0;
@@ -269,8 +277,40 @@
             LOG(WARNING) << "Yikes, we've been told to kill pid 0!  How about we don't do that?";
             continue;
         }
+        pid_t pgid = getpgid(pid);
+        if (pgid == -1) PLOG(ERROR) << "getpgid(" << pid << ") failed";
+        if (pgid == pid) {
+            pgids.emplace(pid);
+        } else {
+            pids.emplace(pid);
+        }
+    }
+
+    // Erase all pids that will be killed when we kill the process groups.
+    for (auto it = pids.begin(); it != pids.end();) {
+        pid_t pgid = getpgid(pid);
+        if (pgids.count(pgid) == 1) {
+            it = pids.erase(it);
+        } else {
+            ++it;
+        }
+    }
+
+    // Kill all process groups.
+    for (const auto pgid : pgids) {
+        LOG(VERBOSE) << "Killing process group " << -pgid << " in uid " << uid
+                     << " as part of process cgroup " << initialPid;
+
+        if (kill(-pgid, signal) == -1) {
+            PLOG(WARNING) << "kill(" << -pgid << ", " << signal << ") failed";
+        }
+    }
+
+    // Kill remaining pids.
+    for (const auto pid : pids) {
         LOG(VERBOSE) << "Killing pid " << pid << " in uid " << uid << " as part of process cgroup "
                      << initialPid;
+
         if (kill(pid, signal) == -1) {
             PLOG(WARNING) << "kill(" << pid << ", " << signal << ") failed";
         }
diff --git a/libziparchive/Android.bp b/libziparchive/Android.bp
index 287a99c..1084d59 100644
--- a/libziparchive/Android.bp
+++ b/libziparchive/Android.bp
@@ -52,17 +52,25 @@
     ],
 }
 
-
 cc_library {
     name: "libziparchive",
     host_supported: true,
-    vendor_available:true,
+    vendor_available: true,
 
-    defaults: ["libziparchive_defaults", "libziparchive_flags"],
-    shared_libs: ["liblog", "libbase"],
+    defaults: [
+        "libziparchive_defaults",
+        "libziparchive_flags",
+    ],
+    shared_libs: [
+        "liblog",
+        "libbase",
+    ],
     target: {
         android: {
-            shared_libs: ["libz", "libutils"],
+            shared_libs: [
+                "libz",
+                "libutils",
+            ],
         },
         host: {
             static_libs: ["libutils"],
@@ -88,7 +96,10 @@
     name: "libziparchive-host",
     host_supported: true,
     device_supported: false,
-    defaults: ["libziparchive_defaults", "libziparchive_flags"],
+    defaults: [
+        "libziparchive_defaults",
+        "libziparchive_flags",
+    ],
     shared_libs: ["libz-host"],
     static_libs: ["libutils"],
 }
@@ -150,3 +161,13 @@
         },
     },
 }
+
+cc_binary {
+    name: "unzip",
+    defaults: ["libziparchive_flags"],
+    srcs: ["unzip.cpp"],
+    shared_libs: [
+        "libbase",
+        "libziparchive",
+    ],
+}
diff --git a/libziparchive/unzip.cpp b/libziparchive/unzip.cpp
new file mode 100644
index 0000000..6756007
--- /dev/null
+++ b/libziparchive/unzip.cpp
@@ -0,0 +1,345 @@
+/*
+ * Copyright (C) 2017 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 <errno.h>
+#include <error.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <time.h>
+#include <unistd.h>
+
+#include <set>
+#include <string>
+
+#include <android-base/file.h>
+#include <android-base/strings.h>
+#include <ziparchive/zip_archive.h>
+
+enum OverwriteMode {
+  kAlways,
+  kNever,
+  kPrompt,
+};
+
+static OverwriteMode overwrite_mode = kPrompt;
+static const char* flag_d = nullptr;
+static bool flag_l = false;
+static bool flag_p = false;
+static bool flag_q = false;
+static bool flag_v = false;
+static const char* archive_name = nullptr;
+static std::set<std::string> includes;
+static std::set<std::string> excludes;
+static uint64_t total_uncompressed_length = 0;
+static uint64_t total_compressed_length = 0;
+static size_t file_count = 0;
+
+static bool Filter(const std::string& name) {
+  if (!excludes.empty() && excludes.find(name) != excludes.end()) return true;
+  if (!includes.empty() && includes.find(name) == includes.end()) return true;
+  return false;
+}
+
+static bool MakeDirectoryHierarchy(const std::string& path) {
+  // stat rather than lstat because a symbolic link to a directory is fine too.
+  struct stat sb;
+  if (stat(path.c_str(), &sb) != -1 && S_ISDIR(sb.st_mode)) return true;
+
+  // Ensure the parent directories exist first.
+  if (!MakeDirectoryHierarchy(android::base::Dirname(path))) return false;
+
+  // Then try to create this directory.
+  return (mkdir(path.c_str(), 0777) != -1);
+}
+
+static int CompressionRatio(int64_t uncompressed, int64_t compressed) {
+  if (uncompressed == 0) return 0;
+  return (100LL * (uncompressed - compressed)) / uncompressed;
+}
+
+static void MaybeShowHeader() {
+  if (!flag_q) printf("Archive:  %s\n", archive_name);
+  if (flag_v) {
+    printf(
+        " Length   Method    Size  Cmpr    Date    Time   CRC-32   Name\n"
+        "--------  ------  ------- ---- ---------- ----- --------  ----\n");
+  } else if (flag_l) {
+    printf(
+        "  Length      Date    Time    Name\n"
+        "---------  ---------- -----   ----\n");
+  }
+}
+
+static void MaybeShowFooter() {
+  if (flag_v) {
+    printf(
+        "--------          -------  ---                            -------\n"
+        "%8" PRId64 "         %8" PRId64 " %3d%%                            %zu file%s\n",
+        total_uncompressed_length, total_compressed_length,
+        CompressionRatio(total_uncompressed_length, total_compressed_length), file_count,
+        (file_count == 1) ? "" : "s");
+  } else if (flag_l) {
+    printf(
+        "---------                     -------\n"
+        "%9" PRId64 "                     %zu file%s\n",
+        total_uncompressed_length, file_count, (file_count == 1) ? "" : "s");
+  }
+}
+
+static bool PromptOverwrite(const std::string& dst) {
+  // TODO: [r]ename not implemented because it doesn't seem useful.
+  printf("replace %s? [y]es, [n]o, [A]ll, [N]one: ", dst.c_str());
+  fflush(stdout);
+  while (true) {
+    char* line = nullptr;
+    size_t n;
+    if (getline(&line, &n, stdin) == -1) {
+      error(1, 0, "(EOF/read error; assuming [N]one...)");
+      overwrite_mode = kNever;
+      return false;
+    }
+    if (n == 0) continue;
+    char cmd = line[0];
+    free(line);
+    switch (cmd) {
+      case 'y':
+        return true;
+      case 'n':
+        return false;
+      case 'A':
+        overwrite_mode = kAlways;
+        return true;
+      case 'N':
+        overwrite_mode = kNever;
+        return false;
+    }
+  }
+}
+
+static void ExtractToPipe(ZipArchiveHandle zah, ZipEntry& entry, const std::string& name) {
+  // We need to extract to memory because ExtractEntryToFile insists on
+  // being able to seek and truncate, and you can't do that with stdout.
+  uint8_t* buffer = new uint8_t[entry.uncompressed_length];
+  int err = ExtractToMemory(zah, &entry, buffer, entry.uncompressed_length);
+  if (err < 0) {
+    error(1, 0, "failed to extract %s: %s", name.c_str(), ErrorCodeString(err));
+  }
+  if (!android::base::WriteFully(1, buffer, entry.uncompressed_length)) {
+    error(1, errno, "failed to write %s to stdout", name.c_str());
+  }
+  delete[] buffer;
+}
+
+static void ExtractOne(ZipArchiveHandle zah, ZipEntry& entry, const std::string& name) {
+  // Bad filename?
+  if (android::base::StartsWith(name, "/") || android::base::StartsWith(name, "../") ||
+      name.find("/../") != std::string::npos) {
+    error(1, 0, "bad filename %s", name.c_str());
+  }
+
+  // Where are we actually extracting to (for human-readable output)?
+  std::string dst;
+  if (flag_d) {
+    dst = flag_d;
+    if (!android::base::EndsWith(dst, "/")) dst += '/';
+  }
+  dst += name;
+
+  // Ensure the directory hierarchy exists.
+  if (!MakeDirectoryHierarchy(android::base::Dirname(name))) {
+    error(1, errno, "couldn't create directory hierarchy for %s", dst.c_str());
+  }
+
+  // An entry in a zip file can just be a directory itself.
+  if (android::base::EndsWith(name, "/")) {
+    if (mkdir(name.c_str(), entry.unix_mode) == -1) {
+      // If the directory already exists, that's fine.
+      if (errno == EEXIST) {
+        struct stat sb;
+        if (stat(name.c_str(), &sb) != -1 && S_ISDIR(sb.st_mode)) return;
+      }
+      error(1, errno, "couldn't extract directory %s", dst.c_str());
+    }
+    return;
+  }
+
+  // Create the file.
+  int fd = open(name.c_str(), O_CREAT | O_WRONLY | O_CLOEXEC | O_EXCL, entry.unix_mode);
+  if (fd == -1 && errno == EEXIST) {
+    if (overwrite_mode == kNever) return;
+    if (overwrite_mode == kPrompt && !PromptOverwrite(dst)) return;
+    // Either overwrite_mode is kAlways or the user consented to this specific case.
+    fd = open(name.c_str(), O_WRONLY | O_CREAT | O_CLOEXEC | O_TRUNC, entry.unix_mode);
+  }
+  if (fd == -1) error(1, errno, "couldn't create file %s", dst.c_str());
+
+  // Actually extract into the file.
+  if (!flag_q) printf("  inflating: %s\n", dst.c_str());
+  int err = ExtractEntryToFile(zah, &entry, fd);
+  if (err < 0) error(1, 0, "failed to extract %s: %s", dst.c_str(), ErrorCodeString(err));
+  close(fd);
+}
+
+static void ListOne(const ZipEntry& entry, const std::string& name) {
+  tm t = entry.GetModificationTime();
+  char time[32];
+  snprintf(time, sizeof(time), "%04d-%02d-%02d %02d:%02d", t.tm_year + 1900, t.tm_mon + 1,
+           t.tm_mday, t.tm_hour, t.tm_min);
+  if (flag_v) {
+    printf("%8d  %s  %7d %3d%% %s %08x  %s\n", entry.uncompressed_length,
+           (entry.method == kCompressStored) ? "Stored" : "Defl:N", entry.compressed_length,
+           CompressionRatio(entry.uncompressed_length, entry.compressed_length), time, entry.crc32,
+           name.c_str());
+  } else {
+    printf("%9d  %s   %s\n", entry.uncompressed_length, time, name.c_str());
+  }
+}
+
+static void ProcessOne(ZipArchiveHandle zah, ZipEntry& entry, const std::string& name) {
+  if (flag_l || flag_v) {
+    // -l or -lv or -lq or -v.
+    ListOne(entry, name);
+  } else {
+    // Actually extract.
+    if (flag_p) {
+      ExtractToPipe(zah, entry, name);
+    } else {
+      ExtractOne(zah, entry, name);
+    }
+  }
+  total_uncompressed_length += entry.uncompressed_length;
+  total_compressed_length += entry.compressed_length;
+  ++file_count;
+}
+
+static void ProcessAll(ZipArchiveHandle zah) {
+  MaybeShowHeader();
+
+  // libziparchive iteration order doesn't match the central directory.
+  // We could sort, but that would cost extra and wouldn't match either.
+  void* cookie;
+  int err = StartIteration(zah, &cookie, nullptr, nullptr);
+  if (err != 0) {
+    error(1, 0, "couldn't iterate %s: %s", archive_name, ErrorCodeString(err));
+  }
+
+  ZipEntry entry;
+  ZipString string;
+  while ((err = Next(cookie, &entry, &string)) >= 0) {
+    std::string name(string.name, string.name + string.name_length);
+    if (!Filter(name)) ProcessOne(zah, entry, name);
+  }
+
+  if (err < -1) error(1, 0, "failed iterating %s: %s", archive_name, ErrorCodeString(err));
+  EndIteration(cookie);
+
+  MaybeShowFooter();
+}
+
+static void ShowHelp(bool full) {
+  fprintf(full ? stdout : stderr, "usage: unzip [-d DIR] [-lnopqv] ZIP [FILE...] [-x FILE...]\n");
+  if (!full) exit(EXIT_FAILURE);
+
+  printf(
+      "\n"
+      "Extract FILEs from ZIP archive. Default is all files.\n"
+      "\n"
+      "-d DIR	Extract into DIR\n"
+      "-l	List contents (-lq excludes archive name, -lv is verbose)\n"
+      "-n	Never overwrite files (default: prompt)\n"
+      "-o	Always overwrite files\n"
+      "-p	Pipe to stdout\n"
+      "-q	Quiet\n"
+      "-v	List contents verbosely\n"
+      "-x FILE	Exclude files\n");
+  exit(EXIT_SUCCESS);
+}
+
+int main(int argc, char* argv[]) {
+  static struct option opts[] = {
+      {"help", no_argument, 0, 'h'},
+  };
+  bool saw_x = false;
+  int opt;
+  while ((opt = getopt_long(argc, argv, "-d:hlnopqvx", opts, nullptr)) != -1) {
+    switch (opt) {
+      case 'd':
+        flag_d = optarg;
+        break;
+      case 'h':
+        ShowHelp(true);
+        break;
+      case 'l':
+        flag_l = true;
+        break;
+      case 'n':
+        overwrite_mode = kNever;
+        break;
+      case 'o':
+        overwrite_mode = kAlways;
+        break;
+      case 'p':
+        flag_p = flag_q = true;
+        break;
+      case 'q':
+        flag_q = true;
+        break;
+      case 'v':
+        flag_v = true;
+        break;
+      case 'x':
+        saw_x = true;
+        break;
+      case 1:
+        // -x swallows all following arguments, so we use '-' in the getopt
+        // string and collect files here.
+        if (!archive_name) {
+          archive_name = optarg;
+        } else if (saw_x) {
+          excludes.insert(optarg);
+        } else {
+          includes.insert(optarg);
+        }
+        break;
+      default:
+        ShowHelp(false);
+    }
+  }
+
+  if (!archive_name) error(1, 0, "missing archive filename");
+
+  // We can't support "-" to unzip from stdin because libziparchive relies on mmap.
+  ZipArchiveHandle zah;
+  int32_t err;
+  if ((err = OpenArchive(archive_name, &zah)) != 0) {
+    error(1, 0, "couldn't open %s: %s", archive_name, ErrorCodeString(err));
+  }
+
+  // Implement -d by changing into that directory.
+  // We'll create implicit directories based on paths in the zip file, but we
+  // require that the -d directory already exists.
+  if (flag_d && chdir(flag_d) == -1) error(1, errno, "couldn't chdir to %s", flag_d);
+
+  ProcessAll(zah);
+
+  CloseArchive(zah);
+  return 0;
+}
diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index a3896fc..a64b3b1 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -27,6 +27,7 @@
 #include <limits.h>
 #include <stdlib.h>
 #include <string.h>
+#include <time.h>
 #include <unistd.h>
 
 #include <memory>
@@ -48,6 +49,10 @@
 
 using android::base::get_unaligned;
 
+// Used to turn on crc checks - verify that the content CRC matches the values
+// specified in the local file header and the central directory.
+static const bool kCrcChecksEnabled = false;
+
 // This is for windows. If we don't open a file in binary mode, weird
 // things will happen.
 #ifndef O_BINARY
@@ -483,8 +488,7 @@
   delete archive;
 }
 
-static int32_t UpdateEntryFromDataDescriptor(MappedZipFile& mapped_zip,
-                                             ZipEntry *entry) {
+static int32_t ValidateDataDescriptor(MappedZipFile& mapped_zip, ZipEntry* entry) {
   uint8_t ddBuf[sizeof(DataDescriptor) + sizeof(DataDescriptor::kOptSignature)];
   if (!mapped_zip.ReadData(ddBuf, sizeof(ddBuf))) {
     return kIoError;
@@ -494,9 +498,17 @@
   const uint16_t offset = (ddSignature == DataDescriptor::kOptSignature) ? 4 : 0;
   const DataDescriptor* descriptor = reinterpret_cast<const DataDescriptor*>(ddBuf + offset);
 
-  entry->crc32 = descriptor->crc32;
-  entry->compressed_length = descriptor->compressed_size;
-  entry->uncompressed_length = descriptor->uncompressed_size;
+  // Validate that the values in the data descriptor match those in the central
+  // directory.
+  if (entry->compressed_length != descriptor->compressed_size ||
+      entry->uncompressed_length != descriptor->uncompressed_size ||
+      entry->crc32 != descriptor->crc32) {
+    ALOGW("Zip: size/crc32 mismatch. expected {%" PRIu32 ", %" PRIu32 ", %" PRIx32
+          "}, was {%" PRIu32 ", %" PRIu32 ", %" PRIx32 "}",
+          entry->compressed_length, entry->uncompressed_length, entry->crc32,
+          descriptor->compressed_size, descriptor->uncompressed_size, descriptor->crc32);
+    return kInconsistentInformation;
+  }
 
   return 0;
 }
@@ -564,12 +576,22 @@
   // Paranoia: Match the values specified in the local file header
   // to those specified in the central directory.
 
-  // Verify that the central directory and local file header have the same general purpose bit
-  // flags set.
-  if (lfh->gpb_flags != cdr->gpb_flags) {
-    ALOGW("Zip: gpb flag mismatch. expected {%04" PRIx16 "}, was {%04" PRIx16 "}",
+  // Warn if central directory and local file header don't agree on the use
+  // of a trailing Data Descriptor. The reference implementation is inconsistent
+  // and appears to use the LFH value during extraction (unzip) but the CD value
+  // while displayng information about archives (zipinfo). The spec remains
+  // silent on this inconsistency as well.
+  //
+  // For now, always use the version from the LFH but make sure that the values
+  // specified in the central directory match those in the data descriptor.
+  //
+  // NOTE: It's also worth noting that unzip *does* warn about inconsistencies in
+  // bit 11 (EFS: The language encoding flag, marking that filename and comment are
+  // encoded using UTF-8). This implementation does not check for the presence of
+  // that flag and always enforces that entry names are valid UTF-8.
+  if ((lfh->gpb_flags & kGPBDDFlagMask) != (cdr->gpb_flags & kGPBDDFlagMask)) {
+    ALOGW("Zip: gpb flag mismatch at bit 3. expected {%04" PRIx16 "}, was {%04" PRIx16 "}",
           cdr->gpb_flags, lfh->gpb_flags);
-    return kInconsistentInformation;
   }
 
   // If there is no trailing data descriptor, verify that the central directory and local file
@@ -589,6 +611,13 @@
     data->has_data_descriptor = 1;
   }
 
+  // 4.4.2.1: the upper byte of `version_made_by` gives the source OS. Unix is 3.
+  if ((cdr->version_made_by >> 8) == 3) {
+    data->unix_mode = (cdr->external_file_attributes >> 16) & 0xffff;
+  } else {
+    data->unix_mode = 0777;
+  }
+
   // Check that the local file header name matches the declared
   // name in the central directory.
   if (lfh->file_name_length == nameLen) {
@@ -932,6 +961,7 @@
 
   const uint32_t uncompressed_length = entry->uncompressed_length;
 
+  uint64_t crc = 0;
   uint32_t compressed_length = entry->compressed_length;
   do {
     /* read as much as we can */
@@ -964,6 +994,8 @@
       if (!writer->Append(&write_buf[0], write_size)) {
         // The file might have declared a bogus length.
         return kInconsistentInformation;
+      } else {
+        crc = crc32(crc, &write_buf[0], write_size);
       }
 
       zstream.next_out = &write_buf[0];
@@ -973,8 +1005,13 @@
 
   assert(zerr == Z_STREAM_END);     /* other errors should've been caught */
 
-  // stream.adler holds the crc32 value for such streams.
-  *crc_out = zstream.adler;
+  // NOTE: zstream.adler is always set to 0, because we're using the -MAX_WBITS
+  // "feature" of zlib to tell it there won't be a zlib file header. zlib
+  // doesn't bother calculating the checksum in that scenario. We just do
+  // it ourselves above because there are no additional gains to be made by
+  // having zlib calculate it for us, since they do it by calling crc32 in
+  // the same manner that we have above.
+  *crc_out = crc;
 
   if (zstream.total_out != uncompressed_length || compressed_length != 0) {
     ALOGW("Zip: size mismatch on inflated file (%lu vs %" PRIu32 ")",
@@ -1037,15 +1074,14 @@
   }
 
   if (!return_value && entry->has_data_descriptor) {
-    return_value = UpdateEntryFromDataDescriptor(archive->mapped_zip, entry);
+    return_value = ValidateDataDescriptor(archive->mapped_zip, entry);
     if (return_value) {
       return return_value;
     }
   }
 
-  // TODO: Fix this check by passing the right flags to inflate2 so that
-  // it calculates the CRC for us.
-  if (entry->crc32 != crc && false) {
+  // Validate that the CRC matches the calculated value.
+  if (kCrcChecksEnabled && (entry->crc32 != static_cast<uint32_t>(crc))) {
     ALOGW("Zip: crc mismatch: expected %" PRIu32 ", was %" PRIu64, entry->crc32, crc);
     return kInconsistentInformation;
   }
@@ -1227,3 +1263,17 @@
   }
   return true;
 }
+
+tm ZipEntry::GetModificationTime() const {
+  tm t = {};
+
+  t.tm_hour = (mod_time >> 11) & 0x1f;
+  t.tm_min = (mod_time >> 5) & 0x3f;
+  t.tm_sec = (mod_time & 0x1f) << 1;
+
+  t.tm_year = ((mod_time >> 25) & 0x7f) + 80;
+  t.tm_mon = ((mod_time >> 21) & 0xf) - 1;
+  t.tm_mday = (mod_time >> 16) & 0x1f;
+
+  return t;
+}
diff --git a/libziparchive/zip_archive_common.h b/libziparchive/zip_archive_common.h
index ca42509..bc1ebb4 100644
--- a/libziparchive/zip_archive_common.h
+++ b/libziparchive/zip_archive_common.h
@@ -73,7 +73,7 @@
 
   // The start of record signature. Must be |kSignature|.
   uint32_t record_signature;
-  // Tool version. Ignored by this implementation.
+  // Source tool version. Top byte gives source OS.
   uint16_t version_made_by;
   // Tool version. Ignored by this implementation.
   uint16_t version_needed;
@@ -106,7 +106,7 @@
   uint16_t file_start_disk;
   // File attributes. Ignored by this implementation.
   uint16_t internal_file_attributes;
-  // File attributes. Ignored by this implementation.
+  // File attributes. For archives created on Unix, the top bits are the mode.
   uint32_t external_file_attributes;
   // The offset to the local file header for this entry, from the
   // beginning of this archive.
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 9dd6cc0..42167dd 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -632,6 +632,96 @@
   CloseArchive(handle);
 }
 
+// Generated using the following Java program:
+//     public static void main(String[] foo) throws Exception {
+//       FileOutputStream fos = new
+//       FileOutputStream("/tmp/data_descriptor.zip");
+//       ZipOutputStream zos = new ZipOutputStream(fos);
+//       ZipEntry ze = new ZipEntry("name");
+//       ze.setMethod(ZipEntry.DEFLATED);
+//       zos.putNextEntry(ze);
+//       zos.write("abdcdefghijk".getBytes());
+//       zos.closeEntry();
+//       zos.close();
+//     }
+//
+// cat /tmp/data_descriptor.zip | xxd -i
+//
+static const std::vector<uint8_t> kDataDescriptorZipFile{
+    0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x08, 0x08, 0x08, 0x00, 0x30, 0x59, 0xce, 0x4a, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x6e, 0x61,
+    0x6d, 0x65, 0x4b, 0x4c, 0x4a, 0x49, 0x4e, 0x49, 0x4d, 0x4b, 0xcf, 0xc8, 0xcc, 0xca, 0x06, 0x00,
+    //[sig---------------], [crc32---------------], [csize---------------], [size----------------]
+    0x50, 0x4b, 0x07, 0x08, 0x3d, 0x4e, 0x0e, 0xf9, 0x0e, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00,
+    0x50, 0x4b, 0x01, 0x02, 0x14, 0x00, 0x14, 0x00, 0x08, 0x08, 0x08, 0x00, 0x30, 0x59, 0xce, 0x4a,
+    0x3d, 0x4e, 0x0e, 0xf9, 0x0e, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x6e, 0x61,
+    0x6d, 0x65, 0x50, 0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x32, 0x00,
+    0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+// The offsets of the data descriptor in this file, so we can mess with
+// them later in the test.
+static constexpr uint32_t kDataDescriptorOffset = 48;
+static constexpr uint32_t kCSizeOffset = kDataDescriptorOffset + 8;
+static constexpr uint32_t kSizeOffset = kCSizeOffset + 4;
+
+static void ExtractEntryToMemory(const std::vector<uint8_t>& zip_data,
+                                 std::vector<uint8_t>* entry_out, int32_t* error_code_out) {
+  TemporaryFile tmp_file;
+  ASSERT_NE(-1, tmp_file.fd);
+  ASSERT_TRUE(android::base::WriteFully(tmp_file.fd, &zip_data[0], zip_data.size()));
+  ZipArchiveHandle handle;
+  ASSERT_EQ(0, OpenArchiveFd(tmp_file.fd, "ExtractEntryToMemory", &handle));
+
+  // This function expects a variant of kDataDescriptorZipFile, for look for
+  // an entry whose name is "name" and whose size is 12 (contents =
+  // "abdcdefghijk").
+  ZipEntry entry;
+  ZipString empty_name;
+  SetZipString(&empty_name, "name");
+
+  ASSERT_EQ(0, FindEntry(handle, empty_name, &entry));
+  ASSERT_EQ(static_cast<uint32_t>(12), entry.uncompressed_length);
+
+  entry_out->resize(12);
+  (*error_code_out) = ExtractToMemory(handle, &entry, &((*entry_out)[0]), 12);
+
+  CloseArchive(handle);
+}
+
+TEST(ziparchive, ValidDataDescriptors) {
+  std::vector<uint8_t> entry;
+  int32_t error_code = 0;
+  ExtractEntryToMemory(kDataDescriptorZipFile, &entry, &error_code);
+
+  ASSERT_EQ(0, error_code);
+  ASSERT_EQ(12u, entry.size());
+  ASSERT_EQ('a', entry[0]);
+  ASSERT_EQ('k', entry[11]);
+}
+
+TEST(ziparchive, InvalidDataDescriptors) {
+  std::vector<uint8_t> invalid_csize = kDataDescriptorZipFile;
+  invalid_csize[kCSizeOffset] = 0xfe;
+
+  std::vector<uint8_t> entry;
+  int32_t error_code = 0;
+  ExtractEntryToMemory(invalid_csize, &entry, &error_code);
+
+  ASSERT_GT(0, error_code);
+  ASSERT_STREQ("Inconsistent information", ErrorCodeString(error_code));
+
+  std::vector<uint8_t> invalid_size = kDataDescriptorZipFile;
+  invalid_csize[kSizeOffset] = 0xfe;
+
+  error_code = 0;
+  entry.clear();
+  ExtractEntryToMemory(invalid_csize, &entry, &error_code);
+
+  ASSERT_GT(0, error_code);
+  ASSERT_STREQ("Inconsistent information", ErrorCodeString(error_code));
+}
+
 int main(int argc, char** argv) {
   ::testing::InitGoogleTest(&argc, argv);
 
diff --git a/libziparchive/zip_writer_test.cc b/libziparchive/zip_writer_test.cc
index 5b526a4..9ad0252 100644
--- a/libziparchive/zip_writer_test.cc
+++ b/libziparchive/zip_writer_test.cc
@@ -135,17 +135,6 @@
   CloseArchive(handle);
 }
 
-static void ConvertZipTimeToTm(uint32_t& zip_time, struct tm* tm) {
-  memset(tm, 0, sizeof(struct tm));
-  tm->tm_hour = (zip_time >> 11) & 0x1f;
-  tm->tm_min = (zip_time >> 5) & 0x3f;
-  tm->tm_sec = (zip_time & 0x1f) << 1;
-
-  tm->tm_year = ((zip_time >> 25) & 0x7f) + 80;
-  tm->tm_mon = ((zip_time >> 21) & 0xf) - 1;
-  tm->tm_mday = (zip_time >> 16) & 0x1f;
-}
-
 static struct tm MakeTm() {
   struct tm tm;
   memset(&tm, 0, sizeof(struct tm));
@@ -177,8 +166,7 @@
   ASSERT_EQ(0, FindEntry(handle, ZipString("align.txt"), &data));
   EXPECT_EQ(0, data.offset & 0x03);
 
-  struct tm mod;
-  ConvertZipTimeToTm(data.mod_time, &mod);
+  struct tm mod = data.GetModificationTime();
   EXPECT_EQ(tm.tm_sec, mod.tm_sec);
   EXPECT_EQ(tm.tm_min, mod.tm_min);
   EXPECT_EQ(tm.tm_hour, mod.tm_hour);
@@ -228,8 +216,7 @@
   ASSERT_EQ(0, FindEntry(handle, ZipString("align.txt"), &data));
   EXPECT_EQ(0, data.offset & 0xfff);
 
-  struct tm mod;
-  ConvertZipTimeToTm(data.mod_time, &mod);
+  struct tm mod = data.GetModificationTime();
   EXPECT_EQ(tm.tm_sec, mod.tm_sec);
   EXPECT_EQ(tm.tm_min, mod.tm_min);
   EXPECT_EQ(tm.tm_hour, mod.tm_hour);