From 4b3a08c2b7ae700dc42bced3eba95cb876a650d5 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Mon, 28 Jan 2019 10:39:10 -0800 Subject: [PATCH] jni: more O_CLOEXECs Prevent FDs from leaking across an execve() boundary. Test: compiles and boots Change-Id: I5f91046917b55b6abead0a5c6e939f82814e6f18 --- core/jni/android/graphics/BitmapFactory.cpp | 2 +- core/jni/android/graphics/ImageDecoder.cpp | 2 +- core/jni/android_content_res_ApkAssets.cpp | 2 +- core/jni/android_ddm_DdmHandleNativeHeap.cpp | 2 +- core/jni/android_hardware_SerialPort.cpp | 2 +- core/jni/android_hardware_UsbDeviceConnection.cpp | 2 +- core/jni/android_os_Debug.cpp | 2 +- core/jni/android_text_Hyphenator.cpp | 2 +- core/jni/android_util_Binder.cpp | 2 +- core/jni/android_util_FileObserver.cpp | 2 +- core/jni/com_android_internal_content_NativeLibraryHelper.cpp | 4 ++-- core/jni/com_android_internal_net_NetworkStatsFactory.cpp | 2 +- 12 files changed, 13 insertions(+), 13 deletions(-) diff --git a/core/jni/android/graphics/BitmapFactory.cpp b/core/jni/android/graphics/BitmapFactory.cpp index 03a463e9b55c2..e344f2a83838d 100644 --- a/core/jni/android/graphics/BitmapFactory.cpp +++ b/core/jni/android/graphics/BitmapFactory.cpp @@ -551,7 +551,7 @@ static jobject nativeDecodeFileDescriptor(JNIEnv* env, jobject clazz, jobject fi // if we only close the file descriptor and not the file object it is used to // create. If we don't explicitly clean up the file (which in turn closes the // descriptor) the buffers allocated internally by fseek will be leaked. - int dupDescriptor = dup(descriptor); + int dupDescriptor = fcntl(descriptor, F_DUPFD_CLOEXEC, 0); FILE* file = fdopen(dupDescriptor, "r"); if (file == NULL) { diff --git a/core/jni/android/graphics/ImageDecoder.cpp b/core/jni/android/graphics/ImageDecoder.cpp index df735ae12feb9..42f1a62d5abd9 100644 --- a/core/jni/android/graphics/ImageDecoder.cpp +++ b/core/jni/android/graphics/ImageDecoder.cpp @@ -132,7 +132,7 @@ static jobject ImageDecoder_nCreateFd(JNIEnv* env, jobject /*clazz*/, "broken file descriptor; fstat returned -1", nullptr, source); } - int dupDescriptor = dup(descriptor); + int dupDescriptor = fcntl(descriptor, F_DUPFD_CLOEXEC, 0); FILE* file = fdopen(dupDescriptor, "r"); if (file == NULL) { close(dupDescriptor); diff --git a/core/jni/android_content_res_ApkAssets.cpp b/core/jni/android_content_res_ApkAssets.cpp index 7738d849be39c..f40b461a6dfd6 100644 --- a/core/jni/android_content_res_ApkAssets.cpp +++ b/core/jni/android_content_res_ApkAssets.cpp @@ -72,7 +72,7 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, jobject file_descri return 0; } - unique_fd dup_fd(::dup(fd)); + unique_fd dup_fd(::fcntl(fd, F_DUPFD_CLOEXEC, 0)); if (dup_fd < 0) { jniThrowIOException(env, errno); return 0; diff --git a/core/jni/android_ddm_DdmHandleNativeHeap.cpp b/core/jni/android_ddm_DdmHandleNativeHeap.cpp index 2f25d8f2d3a43..e22f581e14e2f 100644 --- a/core/jni/android_ddm_DdmHandleNativeHeap.cpp +++ b/core/jni/android_ddm_DdmHandleNativeHeap.cpp @@ -54,7 +54,7 @@ struct Header { namespace android { static void ReadFile(const char* path, String8& s) { - int fd = open(path, O_RDONLY); + int fd = open(path, O_RDONLY | O_CLOEXEC); if (fd != -1) { char bytes[1024]; ssize_t byteCount; diff --git a/core/jni/android_hardware_SerialPort.cpp b/core/jni/android_hardware_SerialPort.cpp index 190ddced73127..3ff24468e3aa6 100644 --- a/core/jni/android_hardware_SerialPort.cpp +++ b/core/jni/android_hardware_SerialPort.cpp @@ -134,7 +134,7 @@ android_hardware_SerialPort_open(JNIEnv *env, jobject thiz, jobject fileDescript int fd = jniGetFDFromFileDescriptor(env, fileDescriptor); // duplicate the file descriptor, since ParcelFileDescriptor will eventually close its copy - fd = dup(fd); + fd = fcntl(fd, F_DUPFD_CLOEXEC, 0); if (fd < 0) { jniThrowException(env, "java/io/IOException", "Could not open serial port"); return; diff --git a/core/jni/android_hardware_UsbDeviceConnection.cpp b/core/jni/android_hardware_UsbDeviceConnection.cpp index d953aee6b753e..b885c285e3803 100644 --- a/core/jni/android_hardware_UsbDeviceConnection.cpp +++ b/core/jni/android_hardware_UsbDeviceConnection.cpp @@ -49,7 +49,7 @@ android_hardware_UsbDeviceConnection_open(JNIEnv *env, jobject thiz, jstring dev { int fd = jniGetFDFromFileDescriptor(env, fileDescriptor); // duplicate the file descriptor, since ParcelFileDescriptor will eventually close its copy - fd = dup(fd); + fd = fcntl(fd, F_DUPFD_CLOEXEC, 0); if (fd < 0) return JNI_FALSE; diff --git a/core/jni/android_os_Debug.cpp b/core/jni/android_os_Debug.cpp index 8df028a6af65b..8be617f08ac3f 100644 --- a/core/jni/android_os_Debug.cpp +++ b/core/jni/android_os_Debug.cpp @@ -943,7 +943,7 @@ static bool openFile(JNIEnv* env, jobject fileDescriptor, UniqueFile& fp) } /* dup() the descriptor so we don't close the original with fclose() */ - int fd = dup(origFd); + int fd = fcntl(origFd, F_DUPFD_CLOEXEC, 0); if (fd < 0) { ALOGW("dup(%d) failed: %s\n", origFd, strerror(errno)); jniThrowRuntimeException(env, "dup() failed"); diff --git a/core/jni/android_text_Hyphenator.cpp b/core/jni/android_text_Hyphenator.cpp index 6f9cc22fb3ab5..de307737493eb 100644 --- a/core/jni/android_text_Hyphenator.cpp +++ b/core/jni/android_text_Hyphenator.cpp @@ -37,7 +37,7 @@ static std::string buildFileName(const std::string& locale) { static const uint8_t* mmapPatternFile(const std::string& locale) { const std::string hyFilePath = buildFileName(locale); - const int fd = open(hyFilePath.c_str(), O_RDONLY); + const int fd = open(hyFilePath.c_str(), O_RDONLY | O_CLOEXEC); if (fd == -1) { return nullptr; // Open failed. } diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index adff4d644c087..9556333dbf86a 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -1158,7 +1158,7 @@ static int getprocname(pid_t pid, char *buf, size_t len) { FILE *f; snprintf(filename, sizeof(filename), "/proc/%d/cmdline", pid); - f = fopen(filename, "r"); + f = fopen(filename, "re"); if (!f) { *buf = '\0'; return 1; diff --git a/core/jni/android_util_FileObserver.cpp b/core/jni/android_util_FileObserver.cpp index 6f975b23e5bdb..578788c0f8d27 100644 --- a/core/jni/android_util_FileObserver.cpp +++ b/core/jni/android_util_FileObserver.cpp @@ -40,7 +40,7 @@ static jmethodID method_onEvent; static jint android_os_fileobserver_init(JNIEnv* env, jobject object) { #if defined(__linux__) - return (jint)inotify_init(); + return (jint)inotify_init1(IN_CLOEXEC); #else return -1; #endif diff --git a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp index 5eefc8196d30a..dc536b2d2dee2 100644 --- a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp +++ b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp @@ -125,7 +125,7 @@ isFileDifferent(const char* filePath, uint32_t fileSize, time_t modifiedTime, return true; } - int fd = TEMP_FAILURE_RETRY(open(filePath, O_RDONLY)); + int fd = TEMP_FAILURE_RETRY(open(filePath, O_RDONLY | O_CLOEXEC)); if (fd < 0) { ALOGV("Couldn't open file %s: %s", filePath, strerror(errno)); return true; @@ -565,7 +565,7 @@ com_android_internal_content_NativeLibraryHelper_openApkFd(JNIEnv *env, jclass, return 0; } - int dupedFd = dup(fd); + int dupedFd = fcntl(fd, F_DUPFD_CLOEXEC, 0); if (dupedFd == -1) { jniThrowExceptionFmt(env, "java/lang/IllegalArgumentException", "Failed to dup FileDescriptor: %s", strerror(errno)); diff --git a/core/jni/com_android_internal_net_NetworkStatsFactory.cpp b/core/jni/com_android_internal_net_NetworkStatsFactory.cpp index 24bafca9c3864..8259ffcd419de 100644 --- a/core/jni/com_android_internal_net_NetworkStatsFactory.cpp +++ b/core/jni/com_android_internal_net_NetworkStatsFactory.cpp @@ -95,7 +95,7 @@ static jlongArray get_long_array(JNIEnv* env, jobject obj, jfieldID field, int s static int legacyReadNetworkStatsDetail(std::vector* lines, const std::vector& limitIfaces, int limitTag, int limitUid, const char* path) { - FILE* fp = fopen(path, "r"); + FILE* fp = fopen(path, "re"); if (fp == NULL) { return -1; }