Mail Archives: djgpp-workers/2003/08/30/20:24:33
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 <errno.h>
#include <fcntl.h>
@@ -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, "!<symlink>");
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, "!<symlink>");
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 @@
+!<symlink>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
- Raw text -