Enable clang-tidy for security sensitive domain.

Start with clang-analyzer-security* and cert-*, but disable two
specific errors:

-- cert-err34-c, which checks for atoi(); heavily triggered by
CommandListener, but will disappear when we move to Binder.
-- cert-err58-cpp, which checks for exceptions before main(); it's
a "Low" severity issue, and filed 36656327 to track cleanup.

Fix all other triggered errors along the way.

Test: builds, boots
Bug: 36655947
Change-Id: I1391693fb521ed39700e25ab6b16bc741293bb79

Android.mk

@@ -69,6 +69,11 @@ common_static_libraries := \
 	libbatteryservice \
 	libavb \
 
+# TODO: include "cert-err34-c" once we move to Binder
+# TODO: include "cert-err58-cpp" once 36656327 is fixed
+common_local_tidy_flags := -warnings-as-errors=clang-analyzer-security*,cert-*
+common_local_tidy_checks := -*,clang-analyzer-security*,cert-*,-cert-err34-c,-cert-err58-cpp
+
 vold_conlyflags := -std=c11
 vold_cflags := -Werror -Wall -Wno-missing-field-initializers -Wno-unused-variable -Wno-unused-parameter
 
@@ -87,6 +92,9 @@ include $(CLEAR_VARS)
 LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
 LOCAL_MODULE := libvold
 LOCAL_CLANG := true
+LOCAL_TIDY := true
+LOCAL_TIDY_FLAGS := $(common_local_tidy_flags)
+LOCAL_TIDY_CHECKS := $(common_local_tidy_checks)
 LOCAL_SRC_FILES := $(common_src_files)
 LOCAL_C_INCLUDES := $(common_c_includes)
 LOCAL_SHARED_LIBRARIES := $(common_shared_libraries)
@@ -103,6 +111,9 @@ include $(CLEAR_VARS)
 LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
 LOCAL_MODULE := vold
 LOCAL_CLANG := true
+LOCAL_TIDY := true
+LOCAL_TIDY_FLAGS := $(common_local_tidy_flags)
+LOCAL_TIDY_CHECKS := $(common_local_tidy_checks)
 LOCAL_SRC_FILES := \
 	main.cpp \
 	$(common_src_files)
@@ -123,6 +134,9 @@ include $(CLEAR_VARS)
 
 LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
 LOCAL_CLANG := true
+LOCAL_TIDY := true
+LOCAL_TIDY_FLAGS := $(common_local_tidy_flags)
+LOCAL_TIDY_CHECKS := $(common_local_tidy_checks)
 LOCAL_SRC_FILES := vdc.cpp
 LOCAL_MODULE := vdc
 LOCAL_SHARED_LIBRARIES := libcutils libbase
@@ -136,6 +150,9 @@ include $(CLEAR_VARS)
 
 LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
 LOCAL_CLANG := true
+LOCAL_TIDY := true
+LOCAL_TIDY_FLAGS := $(common_local_tidy_flags)
+LOCAL_TIDY_CHECKS := $(common_local_tidy_checks)
 LOCAL_SRC_FILES:= secdiscard.cpp
 LOCAL_MODULE:= secdiscard
 LOCAL_SHARED_LIBRARIES := libbase

CommandListener.cpp

@@ -327,8 +327,8 @@ int CommandListener::StorageCmd::runCommand(SocketClient *cli,
                 continue;
             }
 
-            char processName[255];
-            Process::getProcessName(pid, processName, sizeof(processName));
+            std::string processName;
+            Process::getProcessName(pid, processName);
 
             if (Process::checkFileDescriptorSymLinks(pid, argv[2]) ||
                 Process::checkFileMaps(pid, argv[2]) ||
@@ -337,7 +337,7 @@ int CommandListener::StorageCmd::runCommand(SocketClient *cli,
                 Process::checkSymLink(pid, argv[2], "exe")) {
 
                 char msg[1024];
-                snprintf(msg, sizeof(msg), "%d %s", pid, processName);
+                snprintf(msg, sizeof(msg), "%d %s", pid, processName.c_str());
                 cli->sendMsg(ResponseCode::StorageUsersListResult, msg, false);
             }
         }

Devmapper.cpp

@@ -195,7 +195,7 @@ int Devmapper::create(const char *name, const char *loopFile, const char *key,
 
     char *geoParams = buffer + sizeof(struct dm_ioctl);
     // bps=512 spc=8 res=32 nft=2 sec=8190 mid=0xf0 spt=63 hds=64 hid=0 bspf=8 rdcl=2 infs=1 bkbs=2
-    strcpy(geoParams, "0 64 63 0");
+    strlcpy(geoParams, "0 64 63 0", DEVMAPPER_BUFFER_SIZE - sizeof(struct dm_ioctl));
     geoParams += strlen(geoParams) + 1;
     geoParams = (char *) _align(geoParams, 8);
     if (ioctl(fd, DM_DEV_SET_GEOMETRY, io)) {

Process.cpp

@@ -28,10 +28,17 @@
 #include <signal.h>
 
 #define LOG_TAG "ProcessKiller"
+
+#include <android-base/file.h>
+#include <android-base/stringprintf.h>
+#include <android-base/logging.h>
 #include <cutils/log.h>
 
 #include "Process.h"
 
+using android::base::ReadFileToString;
+using android::base::StringPrintf;
+
 int Process::readSymLink(const char *path, char *link, size_t max) {
     struct stat s;
     int length;
@@ -40,10 +47,10 @@ int Process::readSymLink(const char *path, char *link, size_t max) {
         return 0;
     if ((s.st_mode & S_IFMT) != S_IFLNK)
         return 0;
-   
-    // we have a symlink    
+
+    // we have a symlink
     length = readlink(path, link, max- 1);
-    if (length <= 0) 
+    if (length <= 0)
         return 0;
     link[length] = 0;
     return 1;
@@ -63,16 +70,9 @@ int Process::pathMatchesMountPoint(const char* path, const char* mountPoint) {
     return 0;
 }
 
-void Process::getProcessName(int pid, char *buffer, size_t max) {
-    int fd;
-    snprintf(buffer, max, "/proc/%d/cmdline", pid);
-    fd = open(buffer, O_RDONLY | O_CLOEXEC);
-    if (fd < 0) {
-        strcpy(buffer, "???");
-    } else {
-        int length = read(fd, buffer, max - 1);
-        buffer[length] = 0;
-        close(fd);
+void Process::getProcessName(int pid, std::string& out_name) {
+    if (!ReadFileToString(StringPrintf("/proc/%d/cmdline", pid), &out_name)) {
+        out_name = "???";
     }
 }
 
@@ -103,7 +103,7 @@ int Process::checkFileDescriptorSymLinks(int pid, const char *mountPoint, char *
         
         // append the file name, after truncating to parent directory
         path[parent_length] = 0;
-        strcat(path, de->d_name);
+        strlcat(path, de->d_name, PATH_MAX);
 
         char link[PATH_MAX];
 
@@ -189,24 +189,24 @@ int Process::killProcessesWithOpenFiles(const char *path, int signal) {
 
     while ((de = readdir(dir))) {
         int pid = getPid(de->d_name);
-        char name[PATH_MAX];
-
         if (pid == -1)
             continue;
-        getProcessName(pid, name, sizeof(name));
+
+        std::string name;
+        getProcessName(pid, name);
 
         char openfile[PATH_MAX];
 
         if (checkFileDescriptorSymLinks(pid, path, openfile, sizeof(openfile))) {
-            SLOGE("Process %s (%d) has open file %s", name, pid, openfile);
+            SLOGE("Process %s (%d) has open file %s", name.c_str(), pid, openfile);
         } else if (checkFileMaps(pid, path, openfile, sizeof(openfile))) {
-            SLOGE("Process %s (%d) has open filemap for %s", name, pid, openfile);
+            SLOGE("Process %s (%d) has open filemap for %s", name.c_str(), pid, openfile);
         } else if (checkSymLink(pid, path, "cwd")) {
-            SLOGE("Process %s (%d) has cwd within %s", name, pid, path);
+            SLOGE("Process %s (%d) has cwd within %s", name.c_str(), pid, path);
         } else if (checkSymLink(pid, path, "root")) {
-            SLOGE("Process %s (%d) has chroot within %s", name, pid, path);
+            SLOGE("Process %s (%d) has chroot within %s", name.c_str(), pid, path);
         } else if (checkSymLink(pid, path, "exe")) {
-            SLOGE("Process %s (%d) has executable path within %s", name, pid, path);
+            SLOGE("Process %s (%d) has executable path within %s", name.c_str(), pid, path);
         } else {
             continue;
         }

Process.h

@@ -28,7 +28,7 @@ public:
     static int checkFileMaps(int pid, const char *path, char *openFilename, size_t max);
     static int checkFileDescriptorSymLinks(int pid, const char *mountPoint);
     static int checkFileDescriptorSymLinks(int pid, const char *mountPoint, char *openFilename, size_t max);
-    static void getProcessName(int pid, char *buffer, size_t max);
+    static void getProcessName(int pid, std::string& out_name);
 private:
     static int readSymLink(const char *path, char *link, size_t max);
     static int pathMatchesMountPoint(const char *path, const char *mountPoint);

Utils.cpp

@@ -292,7 +292,7 @@ status_t ForkExecvp(const std::vector<std::string>& args,
         LOG(ERROR) << "Failed to setexeccon";
         abort();
     }
-    FILE* fp = popen(cmd.c_str(), "r");
+    FILE* fp = popen(cmd.c_str(), "r"); // NOLINT
     if (setexeccon(nullptr)) {
         LOG(ERROR) << "Failed to setexeccon";
         abort();

VolumeManager.cpp

@@ -180,7 +180,7 @@ static int setupDevMapperDevice(char* buffer, size_t len, const char* loopDevice
         }
         *createdDMDevice = true;
     } else {
-        strcpy(buffer, loopDevice);
+        strlcpy(buffer, loopDevice, len);
         *createdDMDevice = false;
     }
     return 0;
@@ -931,7 +931,7 @@ int VolumeManager::createAsec(const char *id, unsigned long numSectors, const ch
         cleanupDm = true;
     } else {
         sb.c_cipher = ASEC_SB_C_CIPHER_NONE;
-        strcpy(dmDevice, loopDevice);
+        strlcpy(dmDevice, loopDevice, sizeof(dmDevice));
     }
 
     /*
@@ -1895,7 +1895,7 @@ int VolumeManager::listMountedObbs(SocketClient* cli) {
     // Create a string to compare against that has a trailing slash
     int loopDirLen = strlen(VolumeManager::LOOPDIR);
     char loopDir[loopDirLen + 2];
-    strcpy(loopDir, VolumeManager::LOOPDIR);
+    strlcpy(loopDir, VolumeManager::LOOPDIR, sizeof(loopDir));
     loopDir[loopDirLen++] = '/';
     loopDir[loopDirLen] = '\0';
 

cryptfs.cpp

@@ -1725,7 +1725,8 @@ int cryptfs_setup_ext_volume(const char* label, const char* real_blkdev,
     memset(&ext_crypt_ftr, 0, sizeof(ext_crypt_ftr));
     ext_crypt_ftr.fs_size = nr_sec;
     ext_crypt_ftr.keysize = keysize;
-    strcpy((char*) ext_crypt_ftr.crypto_type_name, "aes-cbc-essiv:sha256");
+    strlcpy((char*) ext_crypt_ftr.crypto_type_name, "aes-cbc-essiv:sha256",
+            MAX_CRYPTO_TYPE_NAME_LEN);
 
     return create_crypto_blk_dev(&ext_crypt_ftr, key, real_blkdev,
             out_crypto_blkdev, label);
@@ -2238,7 +2239,7 @@ static int cryptfs_enable_inplace_ext4(char *crypto_blkdev,
         }
     }
 
-    if (setjmp(setjmp_env)) {
+    if (setjmp(setjmp_env)) { // NOLINT
         SLOGE("Reading ext4 extent caused an exception\n");
         rc = -1;
         goto errout;