Date: Sun, 31 Aug 2003 01:33:31 +0100 From: "Richard Dawe" Sender: rich AT phekda DOT freeserve DOT co DOT uk To: djgpp-workers AT delorie DOT com X-Mailer: Emacs 21.3.50 (via feedmail 8.3.emacs20_6 I) and Blat ver 1.8.6 Subject: open, O_CREAT|O_EXCL and symlinks [PATCH] Message-Id: Reply-To: djgpp-workers AT delorie DOT com Hello. open(symlink, O_CREAT|O_EXCL, ...) should fail for a symlink according to SUSv3. Currently it doesn't. Below is a patch to fix that, including an update to the open() tests. Doing both __solve_dir_symlinks and __solve_symlinks at the start allows the O_NOFOLLOW code to be simplifed. The cost is that there are now two FILENAME_MAX + 1 buffers on the stack and the code will be a little slower. Perhaps the __solve_symlinks calls should be replaced with the readlink code that I removed from the O_NOFOLLOW path. Something for revision 2, I guess. Any comments? Thanks, bye, Rich =] Index: src/libc/posix/fcntl/open.c =================================================================== RCS file: /cvs/djgpp/djgpp/src/libc/posix/fcntl/open.c,v retrieving revision 1.12 diff -p -u -3 -r1.12 open.c --- src/libc/posix/fcntl/open.c 26 Mar 2003 19:42:00 -0000 1.12 +++ src/libc/posix/fcntl/open.c 31 Aug 2003 00:18:48 -0000 @@ -111,20 +111,27 @@ open(const char* filename, int oflag, .. { const int original_oflag = oflag; int fd, dmode, bintext, dont_have_share; - char real_name[FILENAME_MAX + 1]; + char dir_resolved_name[FILENAME_MAX + 1]; + char all_resolved_name[FILENAME_MAX + 1]; + const char *real_name; int should_create = (oflag & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL); + int last_is_symlink; - /* Solve symlinks and honor O_NOLINK flag */ + /* Solve symlinks */ + if (!__solve_dir_symlinks(filename, dir_resolved_name)) + return -1; /* errno from __solve_dir_symlinks() */ + + if (!__solve_symlinks(filename, all_resolved_name)) + return -1; /* errno from __solve_symlinks() */ + + /* Is the last component a symlink? */ + last_is_symlink = strcmp(dir_resolved_name, all_resolved_name) != 0; + + /* Honor O_NOLINK flag */ if (oflag & O_NOLINK) - { - if (!__solve_dir_symlinks(filename, real_name)) - return -1; /* errno from from __solve_dir_symlinks() */ - } + real_name = dir_resolved_name; else - { - if (!__solve_symlinks(filename, real_name)) - return -1; - } + real_name = all_resolved_name; /* Honor O_NOFOLLOW flag. */ if (oflag & O_NOFOLLOW) @@ -132,43 +139,39 @@ open(const char* filename, int oflag, .. /* O_NOFOLLOW, as defined in glibc, requires open() to fail if the * last path component is a symlink. However, it still requires to * resolve all other path components. - * We check if there were any symlinks by comparing __solve_symlinks() - * input and output. That function does not perform any path - * canonicalization so it should be safe. */ + * We check if there were any symlinks by comparing + * __solve_dir_symlinks() and __solve_symlinks() output. + * Those functions do not perform any path canonicalization, + * so it should be safe. */ if (strcmp(filename, real_name)) { - /* Yes, there were symlinks in the path. Now take all but the last - * path component from `real_name', add last path component from - * `filename', and try to resolve that mess. - */ - char temp[FILENAME_MAX + 1]; - char resolved[2]; - char * last_separator; - int old_errno = errno; - strcpy(temp, real_name); - last_separator = basename(temp); - *last_separator = '\0'; - last_separator = basename(filename); - strcat(temp, "/"); - strcat(temp, last_separator); - if ((readlink(temp, resolved, 1) != -1) || (errno != EINVAL)) - { - /* Yes, the last path component was a symlink. */ - errno = ELOOP; - return -1; - } - errno = old_errno; + if (last_is_symlink) + { + /* Yes, the last path component was a symlink. */ + errno = ELOOP; + return -1; + } } } /* Check this up front, to reduce cost and minimize effect */ if (should_create) + { if (__file_exists(real_name)) { /* file exists and we didn't want it to */ errno = EEXIST; return -1; } + + /* Check that we're not trying to "create" a symlink. */ + if (last_is_symlink && __file_exists(dir_resolved_name)) + { + /* Symlink => fail. */ + errno = EEXIST; + return -1; + } + } /* figure out what mode we're opening the file in */ bintext = oflag & (O_TEXT | O_BINARY); Index: src/libc/posix/fcntl/open.txh =================================================================== RCS file: /cvs/djgpp/djgpp/src/libc/posix/fcntl/open.txh,v retrieving revision 1.9 diff -p -u -3 -r1.9 open.txh --- src/libc/posix/fcntl/open.txh 26 Mar 2003 19:42:00 -0000 1.9 +++ src/libc/posix/fcntl/open.txh 31 Aug 2003 00:18:49 -0000 @@ -39,7 +39,9 @@ If the file does exist, it is truncated @item O_EXCL If the file exists, and @code{O_CREAT} is also specified, the -@code{open} call will fail. +@code{open} call will fail. If the file is a symlink and @code{O_CREAT} +is also specified, then the contents of the symlink are ignored - +the @code{open} call will fail. @item O_APPEND Index: tests/libc/posix/fcntl/open.c =================================================================== RCS file: /cvs/djgpp/djgpp/tests/libc/posix/fcntl/open.c,v retrieving revision 1.4 diff -p -u -3 -r1.4 open.c --- tests/libc/posix/fcntl/open.c 11 Mar 2001 05:22:18 -0000 1.4 +++ tests/libc/posix/fcntl/open.c 31 Aug 2003 00:18:56 -0000 @@ -9,24 +9,26 @@ * we really have opened a referred file. * 4. Open symlink in a symlink subdir with O_NOFOLLOW flag. Should fail with * ELOOP. - * 5. Open a symlink with O_NOLINK but with symlinks in leading dirs. - * 6. Open a symlink file in a simple subdir. Check if we really have opened + * 5. Open an existing symlink with O_CREAT|O_EXCL. Should fail + * with EEXIST. + * 6. Open a symlink with O_NOLINK but with symlinks in leading dirs. + * 7. Open a symlink file in a simple subdir. Check if we really have opened * the referred file. - * 7. Open and close a file with O_TEMPORARY. Verify the file is deleted. - * 8. Open a file with O_TEMPORARY. Duplicate the file handle. Close both - handles. Verify the file is deleted at the correct time. - 9. Open a file with O_TEMPORARY. Duplicate the file handle to an - unused file handle. Close both handles. Verify the file is deleted - at the correct time. - 10. Open a file with O_TEMPORARY. Open the same file without O_TEMPORARY. - Close both handles. Verify the file is deleted at the correct time. - 11. Open a file without O_TEMPORARY. Open the same file with O_TEMPORARY. - Duplicate the handle opened without O_TEMPORARY on to the handle - opened with O_TEMPORARY. Close both handles. Verify the file is - deleted at the correct time. - 12. Open a file with O_TEMPORARY. Open it again with O_TEMPORARY. - Duplicate one handle on to the other. Close both handles. Verify the - file is deleted at the correct time. + * 8. Open and close a file with O_TEMPORARY. Verify the file is deleted. + * 9. Open a file with O_TEMPORARY. Duplicate the file handle. Close both + * handles. Verify the file is deleted at the correct time. + * 10. Open a file with O_TEMPORARY. Duplicate the file handle to an + * unused file handle. Close both handles. Verify the file is deleted + * at the correct time. + * 11. Open a file with O_TEMPORARY. Open the same file without O_TEMPORARY. + * Close both handles. Verify the file is deleted at the correct time. + * 12. Open a file without O_TEMPORARY. Open the same file with O_TEMPORARY. + * Duplicate the handle opened without O_TEMPORARY on to the handle + * opened with O_TEMPORARY. Close both handles. Verify the file is + * deleted at the correct time. + * 13. Open a file with O_TEMPORARY. Open it again with O_TEMPORARY. + * Duplicate one handle on to the other. Close both handles. Verify the + * file is deleted at the correct time. */ #include #include @@ -45,28 +47,58 @@ static int testnum; int main(void) { int fd; + if (!__file_exists("file1") || !__file_exists("test1") || !__file_exists("test2") || !__file_exists("dir/file1")) { fprintf(stderr, "Required data file is missing\n"); exit(1); } + + if (__file_exists("doesnotexist")) + { + fprintf(stderr, + "File 'doesnotexist' is in the way - " + "please remove it!\n"); + exit(1); + } + test_success("test1", O_RDONLY, "file1"); test_success("test1", O_RDONLY | O_NOLINK, "!"); test_success("test2/file1", O_RDONLY | O_NOFOLLOW, "file1"); + ++testnum; fd = open("test2/test1", O_RDONLY | O_NOFOLLOW); if (fd != -1) { - fprintf(stderr, "Test 4 failed - unexpected open() success.\n"); + fprintf(stderr, "Test %d failed - unexpected open() success.\n", + testnum); exit(1); } if (errno != ELOOP) { - perror("Test 4 failed - wrong errno returned "); + fprintf(stderr, "Test %d failed - wrong errno returned: %s\n", + testnum, strerror(errno)); exit(1); } - printf("Test 4 passed\n"); + printf("Test %d passed\n", testnum); + + ++testnum; + fd = open("test3", O_RDONLY | O_CREAT | O_EXCL); + if (fd != -1) + { + fprintf(stderr, "Test %d failed - unexpected open() success.\n", + testnum); + exit(1); + } + if (errno != EEXIST) + { + fprintf(stderr, "Test %d failed - wrong errno returned: %s\n", + testnum, strerror(errno)); + exit(1); + } + printf("Test %d passed\n", testnum); + test_success("test2/test1", O_RDONLY | O_NOLINK, "!"); test_success("dir/test2", O_RDONLY, "tstlink2"); test_o_temporary(); --- /dev/null 2003-08-31 01:28:28.000000000 +0000 +++ tests/libc/posix/fcntl/test3 2003-03-24 17:25:02.000000000 +0000 @@ -0,0 +1,2 @@ +!doesnotexist +This is just a text to force symlink file to be 510 bytes long. Do not delete it nor spaces following it. \ No newline at end of file