From: corinna Date: Tue, 8 Mar 2011 23:26:14 +0000 (+0000) Subject: * fhandler.cc (fhandler_base::open): When creating a file on a X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=e0c0cfd9ca0eb73982fa68e7bb138169f096c06e;p=pf3gnuchains%2Fsourceware.git * fhandler.cc (fhandler_base::open): When creating a file on a filesystem supporting ACLs, create the file with WRITE_DAC access. Explain why. * fhandler_disk_file.cc (fhandler_disk_file::mkdir): Ditto for directories. * fhandler_socket.cc (fhandler_socket::bind): Ditto for sockets. * path.cc (symlink_worker): Ditto for symlinks. * security.cc (get_file_sd): Always call GetSecurityInfo for directories on XP and Server 2003. Improve comment to explain why. (set_file_attribute): Explicitely cast mode_t value to bool in call to get_file_sd. * wincap.h (wincaps::use_get_sec_info_on_dirs): New element. * wincap.cc: Implement above element throughout. --- diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 0902b27398..0cb0217615 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,19 @@ +2011-03-07 Corinna Vinschen + + * fhandler.cc (fhandler_base::open): When creating a file on a + filesystem supporting ACLs, create the file with WRITE_DAC access. + Explain why. + * fhandler_disk_file.cc (fhandler_disk_file::mkdir): Ditto for + directories. + * fhandler_socket.cc (fhandler_socket::bind): Ditto for sockets. + * path.cc (symlink_worker): Ditto for symlinks. + * security.cc (get_file_sd): Always call GetSecurityInfo for directories + on XP and Server 2003. Improve comment to explain why. + (set_file_attribute): Explicitely cast mode_t value to bool in call to + get_file_sd. + * wincap.h (wincaps::use_get_sec_info_on_dirs): New element. + * wincap.cc: Implement above element throughout. + 2011-03-04 Corinna Vinschen * fhandler_procsys.cc (fhandler_procsys::exists): Rewrite. diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc index cea9c1bb0a..963635c70f 100644 --- a/winsup/cygwin/fhandler.cc +++ b/winsup/cygwin/fhandler.cc @@ -577,6 +577,13 @@ fhandler_base::open (int flags, mode_t mode) /* If mode has no write bits set, and ACLs are not used, we set the DOS R/O attribute. */ file_attributes |= FILE_ATTRIBUTE_READONLY; + else if (!exists () && has_acls ()) + /* If we are about to create the file and the filesystem supports + ACLs, we will overwrite the DACL after the call to NtCreateFile. + This requires a handle with additional WRITE_DAC access, + otherwise set_file_sd has to open the file again. */ + access |= WRITE_DAC; + /* The file attributes are needed for later use in, e.g. fchmod. */ pc.file_attributes (file_attributes); } diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc index 115b746c7c..cb85fee0dd 100644 --- a/winsup/cygwin/fhandler_disk_file.cc +++ b/winsup/cygwin/fhandler_disk_file.cc @@ -1460,6 +1460,7 @@ fhandler_disk_file::mkdir (mode_t mode) IO_STATUS_BLOCK io; PFILE_FULL_EA_INFORMATION p = NULL; ULONG plen = 0; + ULONG access = FILE_LIST_DIRECTORY | SYNCHRONIZE; if (pc.fs_is_nfs ()) { @@ -1479,8 +1480,13 @@ fhandler_disk_file::mkdir (mode_t mode) nfs_attr->type = NF3DIR; nfs_attr->mode = (mode & 07777) & ~cygheap->umask; } - status = NtCreateFile (&dir, FILE_LIST_DIRECTORY | SYNCHRONIZE, - pc.get_object_attr (attr, sa), &io, NULL, + else if (has_acls ()) + /* If the filesystem supports ACLs, we will overwrite the DACL after the + call to NtCreateFile. This requires a handle with READ_CONTROL and + WRITE_DAC access, otherwise get_file_sd and set_file_sd both have to + open the file again. */ + access |= READ_CONTROL | WRITE_DAC; + status = NtCreateFile (&dir, access, pc.get_object_attr (attr, sa), &io, NULL, FILE_ATTRIBUTE_DIRECTORY, FILE_SHARE_VALID_FLAGS, FILE_CREATE, FILE_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT diff --git a/winsup/cygwin/fhandler_socket.cc b/winsup/cygwin/fhandler_socket.cc index 1b4bf29253..22ae05f88f 100644 --- a/winsup/cygwin/fhandler_socket.cc +++ b/winsup/cygwin/fhandler_socket.cc @@ -996,10 +996,17 @@ fhandler_socket::bind (const struct sockaddr *name, int namelen) HANDLE fh; OBJECT_ATTRIBUTES attr; IO_STATUS_BLOCK io; + ULONG access = DELETE | FILE_GENERIC_WRITE; - status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE, - pc.get_object_attr (attr, sa), &io, NULL, fattr, - 0, FILE_CREATE, + /* If the filesystem supports ACLs, we will overwrite the DACL after the + call to NtCreateFile. This requires a handle with READ_CONTROL and + WRITE_DAC access, otherwise get_file_sd and set_file_sd both have to + open the file again. */ + if (pc.has_acls ()) + access |= READ_CONTROL | WRITE_DAC; + + status = NtCreateFile (&fh, access, pc.get_object_attr (attr, sa), &io, + NULL, fattr, 0, FILE_CREATE, FILE_NON_DIRECTORY_FILE | FILE_SYNCHRONOUS_IO_NONALERT | FILE_OPEN_FOR_BACKUP_INTENT, diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index f6704fb8a1..4a7c116926 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -1415,6 +1415,7 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym, IO_STATUS_BLOCK io; NTSTATUS status; HANDLE fh; + ULONG access = DELETE | FILE_GENERIC_WRITE; tmp_pathbuf tp; unsigned check_opt; bool mk_winsym = use_winsym; @@ -1671,8 +1672,14 @@ symlink_worker (const char *oldpath, const char *newpath, bool use_winsym, goto done; } } - status = NtCreateFile (&fh, DELETE | FILE_GENERIC_WRITE, - win32_newpath.get_object_attr (attr, sa), + else if (!isdevice && win32_newpath.has_acls ()) + /* If the filesystem supports ACLs, we will overwrite the DACL after the + call to NtCreateFile. This requires a handle with READ_CONTROL and + WRITE_DAC access, otherwise get_file_sd and set_file_sd both have to + open the file again. */ + access |= READ_CONTROL | WRITE_DAC; + + status = NtCreateFile (&fh, access, win32_newpath.get_object_attr (attr, sa), &io, NULL, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_VALID_FLAGS, isdevice ? FILE_OVERWRITE_IF : FILE_CREATE, diff --git a/winsup/cygwin/security.cc b/winsup/cygwin/security.cc index 73b740c1d5..6d8d6dba51 100644 --- a/winsup/cygwin/security.cc +++ b/winsup/cygwin/security.cc @@ -43,21 +43,31 @@ get_file_sd (HANDLE fh, path_conv &pc, security_descriptor &sd, { if (fh) { - if (justcreated) + /* Amazing but true. If you want to know if an ACE is inherited + from the parent object, you can't use the NtQuerySecurityObject + function. In the DACL returned by this functions, the + INHERITED_ACE flag is never set. Only by calling GetSecurityInfo + you get this information. + + However, this functionality is slow, and the extra information is + only required when the file has been created and the permissions + are about to be set to POSIX permissions. Therefore we only use + it in case the file just got created. In all other cases we + rather call NtQuerySecurityObject directly... + + ...except that there's a problem on 5.1 and 5.2 kernels. The + GetSecurityInfo call on a file sometimes returns with + ERROR_INVALID_ADDRESS if a former request for the SD of the + parent directory (or one of the parent directories?) used the + NtQuerySecurityObject call, rather than GetSecurityInfo as well. + As soon as all directory SDs are fetched using GetSecurityInfo, + the problem disappears. */ + if (justcreated + || (pc.isdir () && wincap.use_get_sec_info_on_dirs ())) { - /* Amazing but true. If you want to know if an ACE is inherited - from the parent object, you can't use the NtQuerySecurityObject - function. In the DACL returned by this functions, the - INHERITED_ACE flag is never set. Only by calling - GetSecurityInfo you get this information. - - This functionality is slow, and the extra information is only - required when the file has been created and the permissions - are about to be set to POSIX permissions. Therefore we only - use it in case the file just got created. In all other cases - we rather call NtQuerySecurityObject directly. */ PSECURITY_DESCRIPTOR psd; - error = GetSecurityInfo (fh, SE_FILE_OBJECT, ALL_SECURITY_INFORMATION, + error = GetSecurityInfo (fh, SE_FILE_OBJECT, + ALL_SECURITY_INFORMATION, NULL, NULL, NULL, NULL, &psd); if (error == ERROR_SUCCESS) { @@ -876,7 +886,7 @@ set_file_attribute (HANDLE handle, path_conv &pc, { security_descriptor sd; - if (!get_file_sd (handle, pc, sd, attribute & S_JUSTCREATED) + if (!get_file_sd (handle, pc, sd, (bool)(attribute & S_JUSTCREATED)) && alloc_sd (pc, uid, gid, attribute, sd)) ret = set_file_sd (handle, pc, sd, uid != ILLEGAL_UID || gid != ILLEGAL_GID); diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc index 89d032367e..c153721138 100644 --- a/winsup/cygwin/wincap.cc +++ b/winsup/cygwin/wincap.cc @@ -62,6 +62,7 @@ wincaps wincap_nt4sp4 __attribute__((section (".cygwin_dll_common"), shared)) = has_fast_cwd:false, has_restricted_raw_disk_access:false, use_dont_resolve_hack:false, + use_get_sec_info_on_dirs:false, }; wincaps wincap_2000 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -103,6 +104,7 @@ wincaps wincap_2000 __attribute__((section (".cygwin_dll_common"), shared)) = { has_fast_cwd:false, has_restricted_raw_disk_access:false, use_dont_resolve_hack:false, + use_get_sec_info_on_dirs:false, }; wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -144,6 +146,7 @@ wincaps wincap_2000sp4 __attribute__((section (".cygwin_dll_common"), shared)) = has_fast_cwd:false, has_restricted_raw_disk_access:false, use_dont_resolve_hack:false, + use_get_sec_info_on_dirs:false, }; wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -185,6 +188,7 @@ wincaps wincap_xp __attribute__((section (".cygwin_dll_common"), shared)) = { has_fast_cwd:false, has_restricted_raw_disk_access:false, use_dont_resolve_hack:true, + use_get_sec_info_on_dirs:true, }; wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -226,6 +230,7 @@ wincaps wincap_xpsp1 __attribute__((section (".cygwin_dll_common"), shared)) = { has_fast_cwd:false, has_restricted_raw_disk_access:false, use_dont_resolve_hack:true, + use_get_sec_info_on_dirs:true, }; wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -267,6 +272,7 @@ wincaps wincap_xpsp2 __attribute__((section (".cygwin_dll_common"), shared)) = { has_fast_cwd:false, has_restricted_raw_disk_access:false, use_dont_resolve_hack:true, + use_get_sec_info_on_dirs:true, }; wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -308,6 +314,7 @@ wincaps wincap_2003 __attribute__((section (".cygwin_dll_common"), shared)) = { has_fast_cwd:false, has_restricted_raw_disk_access:false, use_dont_resolve_hack:false, + use_get_sec_info_on_dirs:true, }; wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -349,6 +356,7 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = { has_fast_cwd:true, has_restricted_raw_disk_access:true, use_dont_resolve_hack:false, + use_get_sec_info_on_dirs:false, }; wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = { @@ -390,6 +398,7 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = { has_fast_cwd:true, has_restricted_raw_disk_access:true, use_dont_resolve_hack:false, + use_get_sec_info_on_dirs:false, }; wincapc wincap __attribute__((section (".cygwin_dll_common"), shared)); diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h index f1eb7a6ec1..60d0512680 100644 --- a/winsup/cygwin/wincap.h +++ b/winsup/cygwin/wincap.h @@ -52,6 +52,7 @@ struct wincaps unsigned has_fast_cwd : 1; unsigned has_restricted_raw_disk_access : 1; unsigned use_dont_resolve_hack : 1; + unsigned use_get_sec_info_on_dirs : 1; }; class wincapc @@ -109,6 +110,7 @@ public: bool IMPLEMENT (has_fast_cwd) bool IMPLEMENT (has_restricted_raw_disk_access) bool IMPLEMENT (use_dont_resolve_hack) + bool IMPLEMENT (use_get_sec_info_on_dirs) #undef IMPLEMENT };