llvm-project/compiler-rt/test/asan/TestCases/Linux/interception_readdir_r_test.cc

63 lines
2.6 KiB
C++
Raw Normal View History

// FIXME: https://code.google.com/p/address-sanitizer/issues/detail?id=316
// XFAIL: android
//
// RUN: %clangxx_asan -O0 %s -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_asan -O1 %s -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_asan -O2 %s -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_asan -O3 %s -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
//
// RUN: %clangxx_asan -O0 %s -D_FILE_OFFSET_BITS=64 -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_asan -O1 %s -D_FILE_OFFSET_BITS=64 -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_asan -O2 %s -D_FILE_OFFSET_BITS=64 -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_asan -O3 %s -D_FILE_OFFSET_BITS=64 -DTEMP_DIR='"'"%T"'"' -o %t && %run %t 2>&1 | FileCheck %s
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
#include <dirent.h>
#include <memory.h>
#include <stdio.h>
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
#include <stdlib.h>
#include <unistd.h>
int main() {
// Ensure the readdir_r interceptor doesn't erroneously mark the entire dirent
// as written when the end of the directory pointer is reached.
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
fputs("test1: reading the " TEMP_DIR " directory...\n", stderr);
DIR *d = opendir(TEMP_DIR);
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
struct dirent *result = (struct dirent *)(0xfeedbeef);
// We assume the temp dir for this test doesn't have crazy long file names.
char entry_buffer[4096];
memset(entry_buffer, 0xab, sizeof(entry_buffer));
unsigned count = 0;
do {
// Stamp the entry struct to try to trick the interceptor.
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
((struct dirent *)entry_buffer)->d_reclen = 9999;
if (readdir_r(d, (struct dirent *)entry_buffer, &result) != 0)
abort();
++count;
} while (result != NULL);
fprintf(stderr, "read %d entries\n", count);
closedir(d);
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
// CHECK: test1: reading the {{.*}} directory...
// CHECK-NOT: stack-buffer-overflow
// CHECK: read {{.*}} entries
// Ensure the readdir64_r interceptor doesn't have the bug either.
fputs("test2: reading the " TEMP_DIR " directory...\n", stderr);
d = opendir(TEMP_DIR);
struct dirent64 *result64;
memset(entry_buffer, 0xab, sizeof(entry_buffer));
count = 0;
do {
// Stamp the entry struct to try to trick the interceptor.
((struct dirent64 *)entry_buffer)->d_reclen = 9999;
if (readdir64_r(d, (struct dirent64 *)entry_buffer, &result64) != 0)
abort();
++count;
} while (result64 != NULL);
fprintf(stderr, "read %d entries\n", count);
closedir(d);
Fix a veritable conucopia of bugs in the readdir_r interceptors. First, the reason I came here: I forgot to look at readdir64_r which had the exact same bug as readdir_r. However, upon applying the same quick-fix and testing it I discovered that it still didn't work at all. As a consequence, I spent some time studying the code and thinking about it and fixed several other problems. Second, the code was checking for a null entry and result pointer, but there is no indication that null pointers are viable here. Certainly, the spec makes it extremely clear that there is no non-error case where the implementation of readdir_r fails to dereference the 'result' pointer and store NULL to it. Thus, our checking for a non-null 'result' pointer before reflecting that write in the instrumentation was trivially dead. Remove it. Third, the interceptor was marking the write to the actual dirent struct by looking at the entry pointer, but nothing in the spec requires that the dirent struct written is actually written into the entry structure provided. A threadlocal buffer would be just as conforming, and the spec goes out of its way to say the pointer to the *actual* result dirent struct is stored into *result, so *that* is where the interceptor should reflect a write occuring. This also obviates the need to even consider whether the 'entry' parameter is null. Fourth, I got to the bottom of why nothing at all worked in readdir64_r -- the interceptor structure for dirent64 was completely wrong in that it was the same as dirent. I fixed this struct to be correct (64-bit inode and 64-bit offset! just a 64-bit offset isn't enough!) and added several missing tests for the size and layout of this struct. llvm-svn: 186109
2013-07-12 02:51:40 +08:00
// CHECK: test2: reading the {{.*}} directory...
// CHECK-NOT: stack-buffer-overflow
// CHECK: read {{.*}} entries
}