# HG changeset patch # User asaha # Date 1415641661 28800 # Node ID fb677d6aebea595ee8fda14ebf2b888423edf5fd # Parent b1cf34d57e7846a2e1ff17bd32e3463b62d7d31a 8062675: jmap is unable to display information about java processes and prints only pids Summary: backout fix 8050808 which caused this regression and as requested. Reviewed-by: hseigel diff -r b1cf34d57e78 -r fb677d6aebea src/os/bsd/vm/perfMemory_bsd.cpp --- a/src/os/bsd/vm/perfMemory_bsd.cpp Thu Nov 06 09:39:49 2014 -0800 +++ b/src/os/bsd/vm/perfMemory_bsd.cpp Mon Nov 10 09:47:41 2014 -0800 @@ -197,38 +197,7 @@ } -// Check if the given statbuf is considered a secure directory for -// the backing store files. Returns true if the directory is considered -// a secure location. Returns false if the statbuf is a symbolic link or -// if an error occurred. -// -static bool is_statbuf_secure(struct stat *statp) { - if (S_ISLNK(statp->st_mode) || !S_ISDIR(statp->st_mode)) { - // The path represents a link or some non-directory file type, - // which is not what we expected. Declare it insecure. - // - return false; - } - // We have an existing directory, check if the permissions are safe. - // - if ((statp->st_mode & (S_IWGRP|S_IWOTH)) != 0) { - // The directory is open for writing and could be subjected - // to a symlink or a hard link attack. Declare it insecure. - // - return false; - } - // See if the uid of the directory matches the effective uid of the process. - // - if (statp->st_uid != geteuid()) { - // The directory was not created by this user, declare it insecure. - // - return false; - } - return true; -} - - -// Check if the given path is considered a secure directory for +// check if the given path is considered a secure directory for // the backing store files. Returns true if the directory exists // and is considered a secure location. Returns false if the path // is a symbolic link or if an error occurred. @@ -242,185 +211,27 @@ return false; } - // The path exists, see if it is secure. - return is_statbuf_secure(&statbuf); -} - - -// Check if the given directory file descriptor is considered a secure -// directory for the backing store files. Returns true if the directory -// exists and is considered a secure location. Returns false if the path -// is a symbolic link or if an error occurred. -// -static bool is_dirfd_secure(int dir_fd) { - struct stat statbuf; - int result = 0; - - RESTARTABLE(::fstat(dir_fd, &statbuf), result); - if (result == OS_ERR) { - return false; - } - - // The path exists, now check its mode. - return is_statbuf_secure(&statbuf); -} - - -// Check to make sure fd1 and fd2 are referencing the same file system object. -// -static bool is_same_fsobject(int fd1, int fd2) { - struct stat statbuf1; - struct stat statbuf2; - int result = 0; - - RESTARTABLE(::fstat(fd1, &statbuf1), result); - if (result == OS_ERR) { - return false; - } - RESTARTABLE(::fstat(fd2, &statbuf2), result); - if (result == OS_ERR) { - return false; - } - - if ((statbuf1.st_ino == statbuf2.st_ino) && - (statbuf1.st_dev == statbuf2.st_dev)) { - return true; - } else { + // the path exists, now check it's mode + if (S_ISLNK(statbuf.st_mode) || !S_ISDIR(statbuf.st_mode)) { + // the path represents a link or some non-directory file type, + // which is not what we expected. declare it insecure. + // return false; } -} - - -// Open the directory of the given path and validate it. -// Return a DIR * of the open directory. -// -static DIR *open_directory_secure(const char* dirname) { - // Open the directory using open() so that it can be verified - // to be secure by calling is_dirfd_secure(), opendir() and then check - // to see if they are the same file system object. This method does not - // introduce a window of opportunity for the directory to be attacked that - // calling opendir() and is_directory_secure() does. - int result; - DIR *dirp = NULL; - RESTARTABLE(::open(dirname, O_RDONLY|O_NOFOLLOW), result); - if (result == OS_ERR) { - // Directory doesn't exist or is a symlink, so there is nothing to cleanup. - if (PrintMiscellaneous && Verbose) { - if (errno == ELOOP) { - warning("directory %s is a symlink and is not secure\n", dirname); - } else { - warning("could not open directory %s: %s\n", dirname, strerror(errno)); - } + else { + // we have an existing directory, check if the permissions are safe. + // + if ((statbuf.st_mode & (S_IWGRP|S_IWOTH)) != 0) { + // the directory is open for writing and could be subjected + // to a symlnk attack. declare it insecure. + // + return false; } - return dirp; - } - int fd = result; - - // Determine if the open directory is secure. - if (!is_dirfd_secure(fd)) { - // The directory is not a secure directory. - os::close(fd); - return dirp; - } - - // Open the directory. - dirp = ::opendir(dirname); - if (dirp == NULL) { - // The directory doesn't exist, close fd and return. - os::close(fd); - return dirp; - } - - // Check to make sure fd and dirp are referencing the same file system object. - if (!is_same_fsobject(fd, dirfd(dirp))) { - // The directory is not secure. - os::close(fd); - os::closedir(dirp); - dirp = NULL; - return dirp; - } - - // Close initial open now that we know directory is secure - os::close(fd); - - return dirp; -} - -// NOTE: The code below uses fchdir(), open() and unlink() because -// fdopendir(), openat() and unlinkat() are not supported on all -// versions. Once the support for fdopendir(), openat() and unlinkat() -// is available on all supported versions the code can be changed -// to use these functions. - -// Open the directory of the given path, validate it and set the -// current working directory to it. -// Return a DIR * of the open directory and the saved cwd fd. -// -static DIR *open_directory_secure_cwd(const char* dirname, int *saved_cwd_fd) { - - // Open the directory. - DIR* dirp = open_directory_secure(dirname); - if (dirp == NULL) { - // Directory doesn't exist or is insecure, so there is nothing to cleanup. - return dirp; - } - int fd = dirfd(dirp); - - // Open a fd to the cwd and save it off. - int result; - RESTARTABLE(::open(".", O_RDONLY), result); - if (result == OS_ERR) { - *saved_cwd_fd = -1; - } else { - *saved_cwd_fd = result; - } - - // Set the current directory to dirname by using the fd of the directory. - result = fchdir(fd); - - return dirp; -} - -// Close the directory and restore the current working directory. -// -static void close_directory_secure_cwd(DIR* dirp, int saved_cwd_fd) { - - int result; - // If we have a saved cwd change back to it and close the fd. - if (saved_cwd_fd != -1) { - result = fchdir(saved_cwd_fd); - ::close(saved_cwd_fd); - } - - // Close the directory. - os::closedir(dirp); -} - -// Check if the given file descriptor is considered a secure. -// -static bool is_file_secure(int fd, const char *filename) { - - int result; - struct stat statbuf; - - // Determine if the file is secure. - RESTARTABLE(::fstat(fd, &statbuf), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - warning("fstat failed on %s: %s\n", filename, strerror(errno)); - } - return false; - } - if (statbuf.st_nlink > 1) { - // A file with multiple links is not expected. - if (PrintMiscellaneous && Verbose) { - warning("file %s has multiple links\n", filename); - } - return false; } return true; } + // return the user name for the given user id // // the caller is expected to free the allocated memory. @@ -506,10 +317,9 @@ const char* tmpdirname = os::get_temp_directory(); - // open the temp directory - DIR* tmpdirp = open_directory_secure(tmpdirname); + DIR* tmpdirp = os::opendir(tmpdirname); + if (tmpdirp == NULL) { - // Cannot open the directory to get the user name, return. return NULL; } @@ -534,14 +344,25 @@ strcat(usrdir_name, "/"); strcat(usrdir_name, dentry->d_name); - // open the user directory - DIR* subdirp = open_directory_secure(usrdir_name); + DIR* subdirp = os::opendir(usrdir_name); if (subdirp == NULL) { FREE_C_HEAP_ARRAY(char, usrdir_name, mtInternal); continue; } + // Since we don't create the backing store files in directories + // pointed to by symbolic links, we also don't follow them when + // looking for the files. We check for a symbolic link after the + // call to opendir in order to eliminate a small window where the + // symlink can be exploited. + // + if (!is_directory_secure(usrdir_name)) { + FREE_C_HEAP_ARRAY(char, usrdir_name, mtInternal); + os::closedir(subdirp); + continue; + } + struct dirent* udentry; char* udbuf = NEW_C_HEAP_ARRAY(char, os::readdir_buf_size(usrdir_name), mtInternal); errno = 0; @@ -644,6 +465,26 @@ } +// remove file +// +// this method removes the file with the given file name in the +// named directory. +// +static void remove_file(const char* dirname, const char* filename) { + + size_t nbytes = strlen(dirname) + strlen(filename) + 2; + char* path = NEW_C_HEAP_ARRAY(char, nbytes, mtInternal); + + strcpy(path, dirname); + strcat(path, "/"); + strcat(path, filename); + + remove_file(path); + + FREE_C_HEAP_ARRAY(char, path, mtInternal); +} + + // cleanup stale shared memory resources // // This method attempts to remove all stale shared memory files in @@ -655,11 +496,16 @@ // static void cleanup_sharedmem_resources(const char* dirname) { - int saved_cwd_fd; - // open the directory and set the current working directory to it - DIR* dirp = open_directory_secure_cwd(dirname, &saved_cwd_fd); + // open the user temp directory + DIR* dirp = os::opendir(dirname); + if (dirp == NULL) { - // directory doesn't exist or is insecure, so there is nothing to cleanup + // directory doesn't exist, so there is nothing to cleanup + return; + } + + if (!is_directory_secure(dirname)) { + // the directory is not a secure directory return; } @@ -673,7 +519,6 @@ // struct dirent* entry; char* dbuf = NEW_C_HEAP_ARRAY(char, os::readdir_buf_size(dirname), mtInternal); - errno = 0; while ((entry = os::readdir(dirp, (struct dirent *)dbuf)) != NULL) { @@ -684,7 +529,7 @@ if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) { // attempt to remove all unexpected files, except "." and ".." - unlink(entry->d_name); + remove_file(dirname, entry->d_name); } errno = 0; @@ -707,14 +552,11 @@ if ((pid == os::current_process_id()) || (kill(pid, 0) == OS_ERR && (errno == ESRCH || errno == EPERM))) { - unlink(entry->d_name); + remove_file(dirname, entry->d_name); } errno = 0; } - - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - + os::closedir(dirp); FREE_C_HEAP_ARRAY(char, dbuf, mtInternal); } @@ -771,54 +613,19 @@ return -1; } - int saved_cwd_fd; - // open the directory and set the current working directory to it - DIR* dirp = open_directory_secure_cwd(dirname, &saved_cwd_fd); - if (dirp == NULL) { - // Directory doesn't exist or is insecure, so cannot create shared - // memory file. + int result; + + RESTARTABLE(::open(filename, O_RDWR|O_CREAT|O_TRUNC, S_IREAD|S_IWRITE), result); + if (result == OS_ERR) { + if (PrintMiscellaneous && Verbose) { + warning("could not create file %s: %s\n", filename, strerror(errno)); + } return -1; } - // Open the filename in the current directory. - // Cannot use O_TRUNC here; truncation of an existing file has to happen - // after the is_file_secure() check below. - int result; - RESTARTABLE(::open(filename, O_RDWR|O_CREAT|O_NOFOLLOW, S_IREAD|S_IWRITE), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - if (errno == ELOOP) { - warning("file %s is a symlink and is not secure\n", filename); - } else { - warning("could not create file %s: %s\n", filename, strerror(errno)); - } - } - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - - return -1; - } - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - // save the file descriptor int fd = result; - // check to see if the file is secure - if (!is_file_secure(fd, filename)) { - ::close(fd); - return -1; - } - - // truncate the file to get rid of any existing data - RESTARTABLE(::ftruncate(fd, (off_t)0), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - warning("could not truncate shared memory file: %s\n", strerror(errno)); - } - ::close(fd); - return -1; - } // set the file size RESTARTABLE(::ftruncate(fd, (off_t)size), result); if (result == OS_ERR) { @@ -876,15 +683,8 @@ THROW_MSG_(vmSymbols::java_io_IOException(), strerror(errno), OS_ERR); } } - int fd = result; - // check to see if the file is secure - if (!is_file_secure(fd, filename)) { - ::close(fd); - return -1; - } - - return fd; + return result; } // create a named shared memory region. returns the address of the @@ -916,21 +716,13 @@ char* dirname = get_user_tmp_dir(user_name); char* filename = get_sharedmem_filename(dirname, vmid); - // get the short filename - char* short_filename = strrchr(filename, '/'); - if (short_filename == NULL) { - short_filename = filename; - } else { - short_filename++; - } - // cleanup any stale shared memory files cleanup_sharedmem_resources(dirname); assert(((size > 0) && (size % os::vm_page_size() == 0)), "unexpected PerfMemory region size"); - fd = create_sharedmem_resources(dirname, short_filename, size); + fd = create_sharedmem_resources(dirname, filename, size); FREE_C_HEAP_ARRAY(char, user_name, mtInternal); FREE_C_HEAP_ARRAY(char, dirname, mtInternal); @@ -1045,12 +837,12 @@ // constructs for the file and the shared memory mapping. if (mode == PerfMemory::PERF_MODE_RO) { mmap_prot = PROT_READ; - file_flags = O_RDONLY | O_NOFOLLOW; + file_flags = O_RDONLY; } else if (mode == PerfMemory::PERF_MODE_RW) { #ifdef LATER mmap_prot = PROT_READ | PROT_WRITE; - file_flags = O_RDWR | O_NOFOLLOW; + file_flags = O_RDWR; #else THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Unsupported access mode"); diff -r b1cf34d57e78 -r fb677d6aebea src/os/linux/vm/perfMemory_linux.cpp --- a/src/os/linux/vm/perfMemory_linux.cpp Thu Nov 06 09:39:49 2014 -0800 +++ b/src/os/linux/vm/perfMemory_linux.cpp Mon Nov 10 09:47:41 2014 -0800 @@ -197,38 +197,7 @@ } -// Check if the given statbuf is considered a secure directory for -// the backing store files. Returns true if the directory is considered -// a secure location. Returns false if the statbuf is a symbolic link or -// if an error occurred. -// -static bool is_statbuf_secure(struct stat *statp) { - if (S_ISLNK(statp->st_mode) || !S_ISDIR(statp->st_mode)) { - // The path represents a link or some non-directory file type, - // which is not what we expected. Declare it insecure. - // - return false; - } - // We have an existing directory, check if the permissions are safe. - // - if ((statp->st_mode & (S_IWGRP|S_IWOTH)) != 0) { - // The directory is open for writing and could be subjected - // to a symlink or a hard link attack. Declare it insecure. - // - return false; - } - // See if the uid of the directory matches the effective uid of the process. - // - if (statp->st_uid != geteuid()) { - // The directory was not created by this user, declare it insecure. - // - return false; - } - return true; -} - - -// Check if the given path is considered a secure directory for +// check if the given path is considered a secure directory for // the backing store files. Returns true if the directory exists // and is considered a secure location. Returns false if the path // is a symbolic link or if an error occurred. @@ -242,181 +211,22 @@ return false; } - // The path exists, see if it is secure. - return is_statbuf_secure(&statbuf); -} - - -// Check if the given directory file descriptor is considered a secure -// directory for the backing store files. Returns true if the directory -// exists and is considered a secure location. Returns false if the path -// is a symbolic link or if an error occurred. -// -static bool is_dirfd_secure(int dir_fd) { - struct stat statbuf; - int result = 0; - - RESTARTABLE(::fstat(dir_fd, &statbuf), result); - if (result == OS_ERR) { - return false; - } - - // The path exists, now check its mode. - return is_statbuf_secure(&statbuf); -} - - -// Check to make sure fd1 and fd2 are referencing the same file system object. -// -static bool is_same_fsobject(int fd1, int fd2) { - struct stat statbuf1; - struct stat statbuf2; - int result = 0; - - RESTARTABLE(::fstat(fd1, &statbuf1), result); - if (result == OS_ERR) { - return false; - } - RESTARTABLE(::fstat(fd2, &statbuf2), result); - if (result == OS_ERR) { - return false; - } - - if ((statbuf1.st_ino == statbuf2.st_ino) && - (statbuf1.st_dev == statbuf2.st_dev)) { - return true; - } else { + // the path exists, now check it's mode + if (S_ISLNK(statbuf.st_mode) || !S_ISDIR(statbuf.st_mode)) { + // the path represents a link or some non-directory file type, + // which is not what we expected. declare it insecure. + // return false; } -} - - -// Open the directory of the given path and validate it. -// Return a DIR * of the open directory. -// -static DIR *open_directory_secure(const char* dirname) { - // Open the directory using open() so that it can be verified - // to be secure by calling is_dirfd_secure(), opendir() and then check - // to see if they are the same file system object. This method does not - // introduce a window of opportunity for the directory to be attacked that - // calling opendir() and is_directory_secure() does. - int result; - DIR *dirp = NULL; - RESTARTABLE(::open(dirname, O_RDONLY|O_NOFOLLOW), result); - if (result == OS_ERR) { - // Directory doesn't exist or is a symlink, so there is nothing to cleanup. - if (PrintMiscellaneous && Verbose) { - if (errno == ELOOP) { - warning("directory %s is a symlink and is not secure\n", dirname); - } else { - warning("could not open directory %s: %s\n", dirname, strerror(errno)); - } + else { + // we have an existing directory, check if the permissions are safe. + // + if ((statbuf.st_mode & (S_IWGRP|S_IWOTH)) != 0) { + // the directory is open for writing and could be subjected + // to a symlnk attack. declare it insecure. + // + return false; } - return dirp; - } - int fd = result; - - // Determine if the open directory is secure. - if (!is_dirfd_secure(fd)) { - // The directory is not a secure directory. - os::close(fd); - return dirp; - } - - // Open the directory. - dirp = ::opendir(dirname); - if (dirp == NULL) { - // The directory doesn't exist, close fd and return. - os::close(fd); - return dirp; - } - - // Check to make sure fd and dirp are referencing the same file system object. - if (!is_same_fsobject(fd, dirfd(dirp))) { - // The directory is not secure. - os::close(fd); - os::closedir(dirp); - dirp = NULL; - return dirp; - } - - // Close initial open now that we know directory is secure - os::close(fd); - - return dirp; -} - -// NOTE: The code below uses fchdir(), open() and unlink() because -// fdopendir(), openat() and unlinkat() are not supported on all -// versions. Once the support for fdopendir(), openat() and unlinkat() -// is available on all supported versions the code can be changed -// to use these functions. - -// Open the directory of the given path, validate it and set the -// current working directory to it. -// Return a DIR * of the open directory and the saved cwd fd. -// -static DIR *open_directory_secure_cwd(const char* dirname, int *saved_cwd_fd) { - - // Open the directory. - DIR* dirp = open_directory_secure(dirname); - if (dirp == NULL) { - // Directory doesn't exist or is insecure, so there is nothing to cleanup. - return dirp; - } - int fd = dirfd(dirp); - - // Open a fd to the cwd and save it off. - int result; - RESTARTABLE(::open(".", O_RDONLY), result); - if (result == OS_ERR) { - *saved_cwd_fd = -1; - } else { - *saved_cwd_fd = result; - } - - // Set the current directory to dirname by using the fd of the directory. - result = fchdir(fd); - - return dirp; -} - -// Close the directory and restore the current working directory. -// -static void close_directory_secure_cwd(DIR* dirp, int saved_cwd_fd) { - - int result; - // If we have a saved cwd change back to it and close the fd. - if (saved_cwd_fd != -1) { - result = fchdir(saved_cwd_fd); - ::close(saved_cwd_fd); - } - - // Close the directory. - os::closedir(dirp); -} - -// Check if the given file descriptor is considered a secure. -// -static bool is_file_secure(int fd, const char *filename) { - - int result; - struct stat statbuf; - - // Determine if the file is secure. - RESTARTABLE(::fstat(fd, &statbuf), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - warning("fstat failed on %s: %s\n", filename, strerror(errno)); - } - return false; - } - if (statbuf.st_nlink > 1) { - // A file with multiple links is not expected. - if (PrintMiscellaneous && Verbose) { - warning("file %s has multiple links\n", filename); - } - return false; } return true; } @@ -507,10 +317,9 @@ const char* tmpdirname = os::get_temp_directory(); - // open the temp directory - DIR* tmpdirp = open_directory_secure(tmpdirname); + DIR* tmpdirp = os::opendir(tmpdirname); + if (tmpdirp == NULL) { - // Cannot open the directory to get the user name, return. return NULL; } @@ -535,8 +344,7 @@ strcat(usrdir_name, "/"); strcat(usrdir_name, dentry->d_name); - // open the user directory - DIR* subdirp = open_directory_secure(usrdir_name); + DIR* subdirp = os::opendir(usrdir_name); if (subdirp == NULL) { FREE_C_HEAP_ARRAY(char, usrdir_name, mtInternal); @@ -657,6 +465,26 @@ } +// remove file +// +// this method removes the file with the given file name in the +// named directory. +// +static void remove_file(const char* dirname, const char* filename) { + + size_t nbytes = strlen(dirname) + strlen(filename) + 2; + char* path = NEW_C_HEAP_ARRAY(char, nbytes, mtInternal); + + strcpy(path, dirname); + strcat(path, "/"); + strcat(path, filename); + + remove_file(path); + + FREE_C_HEAP_ARRAY(char, path, mtInternal); +} + + // cleanup stale shared memory resources // // This method attempts to remove all stale shared memory files in @@ -668,11 +496,16 @@ // static void cleanup_sharedmem_resources(const char* dirname) { - int saved_cwd_fd; - // open the directory - DIR* dirp = open_directory_secure_cwd(dirname, &saved_cwd_fd); + // open the user temp directory + DIR* dirp = os::opendir(dirname); + if (dirp == NULL) { - // directory doesn't exist or is insecure, so there is nothing to cleanup + // directory doesn't exist, so there is nothing to cleanup + return; + } + + if (!is_directory_secure(dirname)) { + // the directory is not a secure directory return; } @@ -686,7 +519,6 @@ // struct dirent* entry; char* dbuf = NEW_C_HEAP_ARRAY(char, os::readdir_buf_size(dirname), mtInternal); - errno = 0; while ((entry = os::readdir(dirp, (struct dirent *)dbuf)) != NULL) { @@ -695,8 +527,9 @@ if (pid == 0) { if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) { + // attempt to remove all unexpected files, except "." and ".." - unlink(entry->d_name); + remove_file(dirname, entry->d_name); } errno = 0; @@ -718,14 +551,12 @@ // if ((pid == os::current_process_id()) || (kill(pid, 0) == OS_ERR && (errno == ESRCH || errno == EPERM))) { - unlink(entry->d_name); + + remove_file(dirname, entry->d_name); } errno = 0; } - - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - + os::closedir(dirp); FREE_C_HEAP_ARRAY(char, dbuf, mtInternal); } @@ -782,54 +613,19 @@ return -1; } - int saved_cwd_fd; - // open the directory and set the current working directory to it - DIR* dirp = open_directory_secure_cwd(dirname, &saved_cwd_fd); - if (dirp == NULL) { - // Directory doesn't exist or is insecure, so cannot create shared - // memory file. + int result; + + RESTARTABLE(::open(filename, O_RDWR|O_CREAT|O_TRUNC, S_IREAD|S_IWRITE), result); + if (result == OS_ERR) { + if (PrintMiscellaneous && Verbose) { + warning("could not create file %s: %s\n", filename, strerror(errno)); + } return -1; } - // Open the filename in the current directory. - // Cannot use O_TRUNC here; truncation of an existing file has to happen - // after the is_file_secure() check below. - int result; - RESTARTABLE(::open(filename, O_RDWR|O_CREAT|O_NOFOLLOW, S_IREAD|S_IWRITE), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - if (errno == ELOOP) { - warning("file %s is a symlink and is not secure\n", filename); - } else { - warning("could not create file %s: %s\n", filename, strerror(errno)); - } - } - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - - return -1; - } - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - // save the file descriptor int fd = result; - // check to see if the file is secure - if (!is_file_secure(fd, filename)) { - ::close(fd); - return -1; - } - - // truncate the file to get rid of any existing data - RESTARTABLE(::ftruncate(fd, (off_t)0), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - warning("could not truncate shared memory file: %s\n", strerror(errno)); - } - ::close(fd); - return -1; - } // set the file size RESTARTABLE(::ftruncate(fd, (off_t)size), result); if (result == OS_ERR) { @@ -887,15 +683,8 @@ THROW_MSG_(vmSymbols::java_io_IOException(), strerror(errno), OS_ERR); } } - int fd = result; - // check to see if the file is secure - if (!is_file_secure(fd, filename)) { - ::close(fd); - return -1; - } - - return fd; + return result; } // create a named shared memory region. returns the address of the @@ -926,13 +715,6 @@ char* dirname = get_user_tmp_dir(user_name); char* filename = get_sharedmem_filename(dirname, vmid); - // get the short filename - char* short_filename = strrchr(filename, '/'); - if (short_filename == NULL) { - short_filename = filename; - } else { - short_filename++; - } // cleanup any stale shared memory files cleanup_sharedmem_resources(dirname); @@ -940,7 +722,7 @@ assert(((size > 0) && (size % os::vm_page_size() == 0)), "unexpected PerfMemory region size"); - fd = create_sharedmem_resources(dirname, short_filename, size); + fd = create_sharedmem_resources(dirname, filename, size); FREE_C_HEAP_ARRAY(char, user_name, mtInternal); FREE_C_HEAP_ARRAY(char, dirname, mtInternal); @@ -1055,12 +837,12 @@ // constructs for the file and the shared memory mapping. if (mode == PerfMemory::PERF_MODE_RO) { mmap_prot = PROT_READ; - file_flags = O_RDONLY | O_NOFOLLOW; + file_flags = O_RDONLY; } else if (mode == PerfMemory::PERF_MODE_RW) { #ifdef LATER mmap_prot = PROT_READ | PROT_WRITE; - file_flags = O_RDWR | O_NOFOLLOW; + file_flags = O_RDWR; #else THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Unsupported access mode"); diff -r b1cf34d57e78 -r fb677d6aebea src/os/solaris/vm/perfMemory_solaris.cpp --- a/src/os/solaris/vm/perfMemory_solaris.cpp Thu Nov 06 09:39:49 2014 -0800 +++ b/src/os/solaris/vm/perfMemory_solaris.cpp Mon Nov 10 09:47:41 2014 -0800 @@ -199,38 +199,7 @@ } -// Check if the given statbuf is considered a secure directory for -// the backing store files. Returns true if the directory is considered -// a secure location. Returns false if the statbuf is a symbolic link or -// if an error occurred. -// -static bool is_statbuf_secure(struct stat *statp) { - if (S_ISLNK(statp->st_mode) || !S_ISDIR(statp->st_mode)) { - // The path represents a link or some non-directory file type, - // which is not what we expected. Declare it insecure. - // - return false; - } - // We have an existing directory, check if the permissions are safe. - // - if ((statp->st_mode & (S_IWGRP|S_IWOTH)) != 0) { - // The directory is open for writing and could be subjected - // to a symlink or a hard link attack. Declare it insecure. - // - return false; - } - // See if the uid of the directory matches the effective uid of the process. - // - if (statp->st_uid != geteuid()) { - // The directory was not created by this user, declare it insecure. - // - return false; - } - return true; -} - - -// Check if the given path is considered a secure directory for +// check if the given path is considered a secure directory for // the backing store files. Returns true if the directory exists // and is considered a secure location. Returns false if the path // is a symbolic link or if an error occurred. @@ -244,185 +213,27 @@ return false; } - // The path exists, see if it is secure. - return is_statbuf_secure(&statbuf); -} - - -// Check if the given directory file descriptor is considered a secure -// directory for the backing store files. Returns true if the directory -// exists and is considered a secure location. Returns false if the path -// is a symbolic link or if an error occurred. -// -static bool is_dirfd_secure(int dir_fd) { - struct stat statbuf; - int result = 0; - - RESTARTABLE(::fstat(dir_fd, &statbuf), result); - if (result == OS_ERR) { - return false; - } - - // The path exists, now check its mode. - return is_statbuf_secure(&statbuf); -} - - -// Check to make sure fd1 and fd2 are referencing the same file system object. -// -static bool is_same_fsobject(int fd1, int fd2) { - struct stat statbuf1; - struct stat statbuf2; - int result = 0; - - RESTARTABLE(::fstat(fd1, &statbuf1), result); - if (result == OS_ERR) { - return false; - } - RESTARTABLE(::fstat(fd2, &statbuf2), result); - if (result == OS_ERR) { - return false; - } - - if ((statbuf1.st_ino == statbuf2.st_ino) && - (statbuf1.st_dev == statbuf2.st_dev)) { - return true; - } else { + // the path exists, now check it's mode + if (S_ISLNK(statbuf.st_mode) || !S_ISDIR(statbuf.st_mode)) { + // the path represents a link or some non-directory file type, + // which is not what we expected. declare it insecure. + // return false; } -} - - -// Open the directory of the given path and validate it. -// Return a DIR * of the open directory. -// -static DIR *open_directory_secure(const char* dirname) { - // Open the directory using open() so that it can be verified - // to be secure by calling is_dirfd_secure(), opendir() and then check - // to see if they are the same file system object. This method does not - // introduce a window of opportunity for the directory to be attacked that - // calling opendir() and is_directory_secure() does. - int result; - DIR *dirp = NULL; - RESTARTABLE(::open(dirname, O_RDONLY|O_NOFOLLOW), result); - if (result == OS_ERR) { - // Directory doesn't exist or is a symlink, so there is nothing to cleanup. - if (PrintMiscellaneous && Verbose) { - if (errno == ELOOP) { - warning("directory %s is a symlink and is not secure\n", dirname); - } else { - warning("could not open directory %s: %s\n", dirname, strerror(errno)); - } + else { + // we have an existing directory, check if the permissions are safe. + // + if ((statbuf.st_mode & (S_IWGRP|S_IWOTH)) != 0) { + // the directory is open for writing and could be subjected + // to a symlnk attack. declare it insecure. + // + return false; } - return dirp; - } - int fd = result; - - // Determine if the open directory is secure. - if (!is_dirfd_secure(fd)) { - // The directory is not a secure directory. - os::close(fd); - return dirp; - } - - // Open the directory. - dirp = ::opendir(dirname); - if (dirp == NULL) { - // The directory doesn't exist, close fd and return. - os::close(fd); - return dirp; - } - - // Check to make sure fd and dirp are referencing the same file system object. - if (!is_same_fsobject(fd, dirp->dd_fd)) { - // The directory is not secure. - os::close(fd); - os::closedir(dirp); - dirp = NULL; - return dirp; - } - - // Close initial open now that we know directory is secure - os::close(fd); - - return dirp; -} - -// NOTE: The code below uses fchdir(), open() and unlink() because -// fdopendir(), openat() and unlinkat() are not supported on all -// versions. Once the support for fdopendir(), openat() and unlinkat() -// is available on all supported versions the code can be changed -// to use these functions. - -// Open the directory of the given path, validate it and set the -// current working directory to it. -// Return a DIR * of the open directory and the saved cwd fd. -// -static DIR *open_directory_secure_cwd(const char* dirname, int *saved_cwd_fd) { - - // Open the directory. - DIR* dirp = open_directory_secure(dirname); - if (dirp == NULL) { - // Directory doesn't exist or is insecure, so there is nothing to cleanup. - return dirp; - } - int fd = dirp->dd_fd; - - // Open a fd to the cwd and save it off. - int result; - RESTARTABLE(::open(".", O_RDONLY), result); - if (result == OS_ERR) { - *saved_cwd_fd = -1; - } else { - *saved_cwd_fd = result; - } - - // Set the current directory to dirname by using the fd of the directory. - result = fchdir(fd); - - return dirp; -} - -// Close the directory and restore the current working directory. -// -static void close_directory_secure_cwd(DIR* dirp, int saved_cwd_fd) { - - int result; - // If we have a saved cwd change back to it and close the fd. - if (saved_cwd_fd != -1) { - result = fchdir(saved_cwd_fd); - ::close(saved_cwd_fd); - } - - // Close the directory. - os::closedir(dirp); -} - -// Check if the given file descriptor is considered a secure. -// -static bool is_file_secure(int fd, const char *filename) { - - int result; - struct stat statbuf; - - // Determine if the file is secure. - RESTARTABLE(::fstat(fd, &statbuf), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - warning("fstat failed on %s: %s\n", filename, strerror(errno)); - } - return false; - } - if (statbuf.st_nlink > 1) { - // A file with multiple links is not expected. - if (PrintMiscellaneous && Verbose) { - warning("file %s has multiple links\n", filename); - } - return false; } return true; } + // return the user name for the given user id // // the caller is expected to free the allocated memory. @@ -497,10 +308,9 @@ const char* tmpdirname = os::get_temp_directory(); - // open the temp directory - DIR* tmpdirp = open_directory_secure(tmpdirname); + DIR* tmpdirp = os::opendir(tmpdirname); + if (tmpdirp == NULL) { - // Cannot open the directory to get the user name, return. return NULL; } @@ -525,8 +335,7 @@ strcat(usrdir_name, "/"); strcat(usrdir_name, dentry->d_name); - // open the user directory - DIR* subdirp = open_directory_secure(usrdir_name); + DIR* subdirp = os::opendir(usrdir_name); if (subdirp == NULL) { FREE_C_HEAP_ARRAY(char, usrdir_name, mtInternal); @@ -695,6 +504,26 @@ } +// remove file +// +// this method removes the file with the given file name in the +// named directory. +// +static void remove_file(const char* dirname, const char* filename) { + + size_t nbytes = strlen(dirname) + strlen(filename) + 2; + char* path = NEW_C_HEAP_ARRAY(char, nbytes, mtInternal); + + strcpy(path, dirname); + strcat(path, "/"); + strcat(path, filename); + + remove_file(path); + + FREE_C_HEAP_ARRAY(char, path, mtInternal); +} + + // cleanup stale shared memory resources // // This method attempts to remove all stale shared memory files in @@ -706,11 +535,16 @@ // static void cleanup_sharedmem_resources(const char* dirname) { - int saved_cwd_fd; - // open the directory - DIR* dirp = open_directory_secure_cwd(dirname, &saved_cwd_fd); + // open the user temp directory + DIR* dirp = os::opendir(dirname); + if (dirp == NULL) { - // directory doesn't exist or is insecure, so there is nothing to cleanup + // directory doesn't exist, so there is nothing to cleanup + return; + } + + if (!is_directory_secure(dirname)) { + // the directory is not a secure directory return; } @@ -724,7 +558,6 @@ // struct dirent* entry; char* dbuf = NEW_C_HEAP_ARRAY(char, os::readdir_buf_size(dirname), mtInternal); - errno = 0; while ((entry = os::readdir(dirp, (struct dirent *)dbuf)) != NULL) { @@ -735,7 +568,7 @@ if (strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0) { // attempt to remove all unexpected files, except "." and ".." - unlink(entry->d_name); + remove_file(dirname, entry->d_name); } errno = 0; @@ -758,14 +591,11 @@ if ((pid == os::current_process_id()) || (kill(pid, 0) == OS_ERR && (errno == ESRCH || errno == EPERM))) { - unlink(entry->d_name); + remove_file(dirname, entry->d_name); } errno = 0; } - - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - + os::closedir(dirp); FREE_C_HEAP_ARRAY(char, dbuf, mtInternal); } @@ -822,54 +652,19 @@ return -1; } - int saved_cwd_fd; - // open the directory and set the current working directory to it - DIR* dirp = open_directory_secure_cwd(dirname, &saved_cwd_fd); - if (dirp == NULL) { - // Directory doesn't exist or is insecure, so cannot create shared - // memory file. + int result; + + RESTARTABLE(::open(filename, O_RDWR|O_CREAT|O_TRUNC, S_IREAD|S_IWRITE), result); + if (result == OS_ERR) { + if (PrintMiscellaneous && Verbose) { + warning("could not create file %s: %s\n", filename, strerror(errno)); + } return -1; } - // Open the filename in the current directory. - // Cannot use O_TRUNC here; truncation of an existing file has to happen - // after the is_file_secure() check below. - int result; - RESTARTABLE(::open(filename, O_RDWR|O_CREAT|O_NOFOLLOW, S_IREAD|S_IWRITE), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - if (errno == ELOOP) { - warning("file %s is a symlink and is not secure\n", filename); - } else { - warning("could not create file %s: %s\n", filename, strerror(errno)); - } - } - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - - return -1; - } - // close the directory and reset the current working directory - close_directory_secure_cwd(dirp, saved_cwd_fd); - // save the file descriptor int fd = result; - // check to see if the file is secure - if (!is_file_secure(fd, filename)) { - ::close(fd); - return -1; - } - - // truncate the file to get rid of any existing data - RESTARTABLE(::ftruncate(fd, (off_t)0), result); - if (result == OS_ERR) { - if (PrintMiscellaneous && Verbose) { - warning("could not truncate shared memory file: %s\n", strerror(errno)); - } - ::close(fd); - return -1; - } // set the file size RESTARTABLE(::ftruncate(fd, (off_t)size), result); if (result == OS_ERR) { @@ -905,15 +700,8 @@ THROW_MSG_(vmSymbols::java_io_IOException(), strerror(errno), OS_ERR); } } - int fd = result; - // check to see if the file is secure - if (!is_file_secure(fd, filename)) { - ::close(fd); - return -1; - } - - return fd; + return result; } // create a named shared memory region. returns the address of the @@ -945,21 +733,13 @@ char* dirname = get_user_tmp_dir(user_name); char* filename = get_sharedmem_filename(dirname, vmid); - // get the short filename - char* short_filename = strrchr(filename, '/'); - if (short_filename == NULL) { - short_filename = filename; - } else { - short_filename++; - } - // cleanup any stale shared memory files cleanup_sharedmem_resources(dirname); assert(((size > 0) && (size % os::vm_page_size() == 0)), "unexpected PerfMemory region size"); - fd = create_sharedmem_resources(dirname, short_filename, size); + fd = create_sharedmem_resources(dirname, filename, size); FREE_C_HEAP_ARRAY(char, user_name, mtInternal); FREE_C_HEAP_ARRAY(char, dirname, mtInternal); @@ -1075,12 +855,12 @@ // constructs for the file and the shared memory mapping. if (mode == PerfMemory::PERF_MODE_RO) { mmap_prot = PROT_READ; - file_flags = O_RDONLY | O_NOFOLLOW; + file_flags = O_RDONLY; } else if (mode == PerfMemory::PERF_MODE_RW) { #ifdef LATER mmap_prot = PROT_READ | PROT_WRITE; - file_flags = O_RDWR | O_NOFOLLOW; + file_flags = O_RDWR; #else THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Unsupported access mode"); diff -r b1cf34d57e78 -r fb677d6aebea src/share/vm/utilities/vmError.cpp --- a/src/share/vm/utilities/vmError.cpp Thu Nov 06 09:39:49 2014 -0800 +++ b/src/share/vm/utilities/vmError.cpp Mon Nov 10 09:47:41 2014 -0800 @@ -22,7 +22,6 @@ * */ -#include #include "precompiled.hpp" #include "compiler/compileBroker.hpp" #include "gc_interface/collectedHeap.hpp" @@ -842,8 +841,7 @@ static int expand_and_open(const char* pattern, char* buf, size_t buflen, size_t pos) { int fd = -1; if (Arguments::copy_expand_pid(pattern, strlen(pattern), &buf[pos], buflen - pos)) { - // the O_EXCL flag will cause the open to fail if the file exists - fd = open(buf, O_RDWR | O_CREAT | O_EXCL, 0666); + fd = open(buf, O_RDWR | O_CREAT | O_TRUNC, 0666); } return fd; }