From 8eca5362726ea2c0a83ca1e9d64086f2e58c22f5 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Thu, 30 Nov 2006 10:17:24 +0000 Subject: [PATCH] * dir.cc (mkdir): Check last path component for "..". (rmdir): Don't check last path component for "..". * fhandler_disk_file.cc (fhandler_disk_file::rmdir): Drop kludge which tries to allow deleting the current working directory. * path.cc (has_dot_last_component): Add parameter to indicate testing for "..". Take trailing slash into account. (symlink_info::posixify): Rely on cygheap->cwd.win32 having a useful value. (cwdstuff::init): Initialize cygheap->cwd with current working directory. Change to windows_system_directory afterwards. (cwdstuff::set): Never call SetCurrentDirectory here. Just check if changing into target directory would be allowed. Add comment to explain why. * path.h (has_dot_last_component): Declare with second parameter. * pinfo.cc (pinfo::zap_cwd): Remove. (pinfo::exit): Drop call to zap_cwd. * pinfo.h (class pinfo): Remove declaration of zap_cwd. * spawn.cc (spawn_guts): Set current working directory for non-Cygwin child applications. Drop call to zap_cwd. --- winsup/cygwin/ChangeLog | 22 +++++++ winsup/cygwin/dir.cc | 4 +- winsup/cygwin/fhandler_disk_file.cc | 94 ++++++++++------------------- winsup/cygwin/path.cc | 76 +++++++++++++++++------ winsup/cygwin/path.h | 2 +- winsup/cygwin/pinfo.cc | 11 ---- winsup/cygwin/pinfo.h | 1 - winsup/cygwin/spawn.cc | 5 +- 8 files changed, 118 insertions(+), 97 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index b8aeca83c..8e7781469 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,25 @@ +2006-11-29 Corinna Vinschen + + * dir.cc (mkdir): Check last path component for "..". + (rmdir): Don't check last path component for "..". + * fhandler_disk_file.cc (fhandler_disk_file::rmdir): Drop kludge + which tries to allow deleting the current working directory. + * path.cc (has_dot_last_component): Add parameter to indicate testing + for "..". Take trailing slash into account. + (symlink_info::posixify): Rely on cygheap->cwd.win32 having a + useful value. + (cwdstuff::init): Initialize cygheap->cwd with current working + directory. Change to windows_system_directory afterwards. + (cwdstuff::set): Never call SetCurrentDirectory here. Just check + if changing into target directory would be allowed. Add comment to + explain why. + * path.h (has_dot_last_component): Declare with second parameter. + * pinfo.cc (pinfo::zap_cwd): Remove. + (pinfo::exit): Drop call to zap_cwd. + * pinfo.h (class pinfo): Remove declaration of zap_cwd. + * spawn.cc (spawn_guts): Set current working directory for non-Cygwin + child applications. Drop call to zap_cwd. + 2006-11-28 Corinna Vinschen * security.cc (create_token): Revert erroneous change to test diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc index 3c94b8c88..c3b22c66d 100644 --- a/winsup/cygwin/dir.cc +++ b/winsup/cygwin/dir.cc @@ -277,7 +277,7 @@ mkdir (const char *dir, mode_t mode) debug_printf ("got %d error from build_fh_name", fh->error ()); set_errno (fh->error ()); } - else if (has_dot_last_component (dir)) + else if (has_dot_last_component (dir, true)) set_errno (fh->exists () ? EEXIST : ENOENT); else if (!fh->mkdir (mode)) res = 0; @@ -307,7 +307,7 @@ rmdir (const char *dir) debug_printf ("got %d error from build_fh_name", fh->error ()); set_errno (fh->error ()); } - else if (has_dot_last_component (dir)) + else if (has_dot_last_component (dir, false)) set_errno (fh->exists () ? EINVAL : ENOENT); else if (!fh->rmdir ()) res = 0; diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc index f3b5ec03e..2d6ae3578 100644 --- a/winsup/cygwin/fhandler_disk_file.cc +++ b/winsup/cygwin/fhandler_disk_file.cc @@ -1449,70 +1449,42 @@ fhandler_disk_file::rmdir () SetFileAttributes (get_win32_name (), (DWORD) pc & ~FILE_ATTRIBUTE_READONLY); - for (bool is_cwd = false; ; is_cwd = true) + DWORD err, att = 0; + int rc = RemoveDirectory (get_win32_name ()); + + if (isremote () && exists ()) + att = GetFileAttributes (get_win32_name ()); + + /* Sometimes smb indicates failure when it really succeeds, so check for + this case specifically. */ + if (rc || att == INVALID_FILE_ATTRIBUTES) { - DWORD err, att = 0; - int rc = RemoveDirectory (get_win32_name ()); + /* RemoveDirectory on a samba drive doesn't return an error if the + directory can't be removed because it's not empty. Checking for + existence afterwards keeps us informed about success. */ + if (!isremote () || att == INVALID_FILE_ATTRIBUTES) + return res; - if (isremote () && exists ()) - att = GetFileAttributes (get_win32_name ()); - - /* Sometimes smb indicates failure when it really succeeds, so check for - this case specifically. */ - if (rc || att == INVALID_FILE_ATTRIBUTES) - { - /* RemoveDirectory on a samba drive doesn't return an error if the - directory can't be removed because it's not empty. Checking for - existence afterwards keeps us informed about success. */ - if (!isremote () || att == INVALID_FILE_ATTRIBUTES) - { - res = 0; - break; - } - err = ERROR_DIR_NOT_EMPTY; - } - else - err = GetLastError (); - - /* This kludge detects if we are attempting to remove the current working - directory. If so, we will move elsewhere to potentially allow the - rmdir to succeed. This means that cygwin's concept of the current - working directory != Windows concept but, hey, whaddaregonnado? - Note that this will not cause something like the following to work: - $ cd foo - $ rmdir . - since the shell will have foo "open" in the above case and so Windows - will not allow the deletion. (Actually it does on 9X.) - FIXME: A potential workaround for this is for cygwin apps to *never* - call SetCurrentDirectory. */ - - extern char windows_system_directory[]; - if (strcasematch (get_win32_name (), cygheap->cwd.win32) - && !strcasematch (windows_system_directory, cygheap->cwd.win32) - && !is_cwd - && SetCurrentDirectory (windows_system_directory)) - continue; - - /* On 9X ERROR_ACCESS_DENIED is returned if you try to remove a - non-empty directory. */ - if (err == ERROR_ACCESS_DENIED - && wincap.access_denied_on_delete ()) - err = ERROR_DIR_NOT_EMPTY; - /* ...and, that's *not* funny, when trying to remove a non-existing - directory on a share, which is hosted by a 9x machine, the error - code ERROR_INVALID_FUNCTION is returned. */ - else if (err == ERROR_INVALID_FUNCTION) - err = ERROR_FILE_NOT_FOUND; - - __seterrno_from_win_error (err); - - /* Directory still exists, restore its characteristics. */ - if (pc.has_attribute (FILE_ATTRIBUTE_READONLY)) - SetFileAttributes (get_win32_name (), (DWORD) pc); - if (is_cwd) - SetCurrentDirectory (get_win32_name ()); - break; + err = ERROR_DIR_NOT_EMPTY; } + else + err = GetLastError (); + /* On 9X ERROR_ACCESS_DENIED is returned if you try to remove a + non-empty directory. */ + if (err == ERROR_ACCESS_DENIED + && wincap.access_denied_on_delete ()) + err = ERROR_DIR_NOT_EMPTY; + /* ...and, that's *not* funny, when trying to remove a non-existing + directory on a share, which is hosted by a 9x machine, the error + code ERROR_INVALID_FUNCTION is returned. */ + else if (err == ERROR_INVALID_FUNCTION) + err = ERROR_FILE_NOT_FOUND; + + __seterrno_from_win_error (err); + + /* Directory still exists, restore its characteristics. */ + if (pc.has_attribute (FILE_ATTRIBUTE_READONLY)) + SetFileAttributes (get_win32_name (), (DWORD) pc); return res; } diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index fbd15e667..3012e2564 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -215,16 +215,28 @@ pathmatch (const char *path1, const char *path2) Right now, normalize_posix_path will just normalize those components away, which changes the semantics. */ bool -has_dot_last_component (const char *dir) +has_dot_last_component (const char *dir, bool test_dot_dot) { /* SUSv3: . and .. are not allowed as last components in various system calls. Don't test for backslash path separator since that's a Win32 path following Win32 rules. */ const char *last_comp = strrchr (dir, '/'); - return last_comp - && last_comp[1] == '.' - && (last_comp[2] == '\0' - || (last_comp[2] == '.' && last_comp[3] == '\0')); + if (!last_comp) + last_comp = dir; + else { + /* Check for trailing slash. If so, hop back to the previous slash. */ + if (!last_comp[1]) + while (last_comp > dir) + if (*--last_comp == '/') + break; + if (*last_comp == '/') + ++last_comp; + } + return last_comp[0] == '.' + && ((last_comp[1] == '\0' || last_comp[1] == '/') + || (test_dot_dot + && last_comp[1] == '.' + && (last_comp[2] == '\0' || last_comp[2] == '/'))); } #define isslash(c) ((c) == '/') @@ -3199,10 +3211,7 @@ symlink_info::posixify (char *srcbuf) { char cvtbuf[CYG_MAX_PATH + 6]; - if (cygheap->cwd.win32) - strncpy (cvtbuf, cygheap->cwd.win32, 2); - else - GetCurrentDirectory (CYG_MAX_PATH, cvtbuf); + strncpy (cvtbuf, cygheap->cwd.win32, 2); strcpy (cvtbuf + 2, srcbuf); mount_table->conv_to_posix_path (cvtbuf, contents, 0); } @@ -4146,6 +4155,12 @@ void cwdstuff::init () { cwd_lock.init ("cwd_lock"); + get_initial (); + /* Actually chdir into the syste dir to avoid cwd problems. See comment + in cwdstuff::set below. */ + extern char windows_system_directory[]; + SetCurrentDirectory (windows_system_directory); + cwd_lock.release (); } /* Get initial cwd. Should only be called once in a @@ -4173,17 +4188,42 @@ cwdstuff::set (const char *win32_cwd, const char *posix_cwd, bool doit) if (win32_cwd) { - cwd_lock.acquire (); - if (doit && !SetCurrentDirectory (win32_cwd)) - { - /* When calling SetCurrentDirectory for a non-existant dir on a - Win9x share, it returns ERROR_INVALID_FUNCTION. */ - if (GetLastError () == ERROR_INVALID_FUNCTION) + cwd_lock.acquire (); + if (doit) + { + /* Check if we *could* chdir, if we actually would. + + Why don't we actually chdir? For two reasons: + - A process has always an open handle to the current working + directory which disallows manipulating this directory. + POSIX allows to remove a directory if the permissions are + ok. The fact that its the cwd of some process doesn't matter. + - SetCurrentDirectory fails for directories with strict + permissions even for processes with the SE_BACKUP_NAME + privilege enabled. The reason is apparently that + SetCurrentDirectory calls NtOpenFile without the + FILE_OPEN_FOR_BACKUP_INTENT flag set. */ + DWORD attr = GetFileAttributes (win32_cwd); + if (attr == INVALID_FILE_ATTRIBUTES) + { set_errno (ENOENT); - else + goto out; + } + if (!(attr & FILE_ATTRIBUTE_DIRECTORY)) + { + set_errno (ENOTDIR); + goto out; + } + HANDLE h = CreateFile (win32_cwd, GENERIC_READ, wincap.shared (), + NULL, OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (h == INVALID_HANDLE_VALUE) + { __seterrno (); - goto out; - } + goto out; + } + CloseHandle (h); + } } /* If there is no win32 path or it has the form c:xxx, get the value */ if (!win32_cwd || (isdrive (win32_cwd) && win32_cwd[2] != '\\')) diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h index 4097053c7..e011575f2 100644 --- a/winsup/cygwin/path.h +++ b/winsup/cygwin/path.h @@ -305,7 +305,7 @@ has_exec_chars (const char *buf, int len) int pathmatch (const char *path1, const char *path2) __attribute__ ((regparm (2))); int pathnmatch (const char *path1, const char *path2, int len) __attribute__ ((regparm (2))); -bool has_dot_last_component (const char *dir) __attribute__ ((regparm (1))); +bool has_dot_last_component (const char *dir, bool test_dot_dot) __attribute__ ((regparm (2))); bool fnunmunge (char *, const char *) __attribute__ ((regparm (2))); diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc index d73ad20aa..404591b26 100644 --- a/winsup/cygwin/pinfo.cc +++ b/winsup/cygwin/pinfo.cc @@ -122,16 +122,6 @@ pinfo::maybe_set_exit_code_from_windows () self->pid, oexitcode, x, self->exitcode); } -void -pinfo::zap_cwd () -{ - extern char windows_system_directory[]; - /* Move to an innocuous location to avoid a race with other processes - that may want to manipulate the current directory before this - process has completely exited. */ - SetCurrentDirectory (windows_system_directory); -} - void pinfo::exit (DWORD n) { @@ -148,7 +138,6 @@ pinfo::exit (DWORD n) } sigproc_terminate (ES_FINAL); - zap_cwd (); /* FIXME: There is a potential race between an execed process and its parent here. I hated to add a mutex just for that, though. */ diff --git a/winsup/cygwin/pinfo.h b/winsup/cygwin/pinfo.h index 0367f38e1..7d32f0bd5 100644 --- a/winsup/cygwin/pinfo.h +++ b/winsup/cygwin/pinfo.h @@ -193,7 +193,6 @@ public: #endif HANDLE shared_handle () {return h;} void set_acl (); - void zap_cwd (); friend class _pinfo; friend class winpids; }; diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index 2771d87e0..d1750f744 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -501,7 +501,7 @@ loop: TRUE, /* inherit handles from parent */ c_flags, envblock, /* environment */ - 0, /* use current drive/directory */ + real_path.iscygexec () ? NULL : cygheap->cwd.win32, &si, &pi); } @@ -536,7 +536,7 @@ loop: TRUE, /* inherit handles from parent */ c_flags, envblock, /* environment */ - 0, /* use current drive/directory */ + real_path.iscygexec () ? NULL : cygheap->cwd.win32, &si, &pi); } @@ -613,7 +613,6 @@ loop: myself->sync_proc_pipe (); /* Make sure that we own wr_proc_pipe just in case we've been previously execed. */ - myself.zap_cwd (); } orig_wr_proc_pipe = myself->dup_proc_pipe (pi.hProcess); }