From 4bfa93f1a00a09e4fb3bccea334ba22e4c05c3d6 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Tue, 28 Jan 2020 17:57:50 +0100 Subject: [PATCH] Cygwin: symlink/mknod: fix ACL handling mknod32 actually creates a path_conv, just to call mknod_worker with a win32 path. This doesn't only require to create path_conv twice, it also breaks permissions on filesystems supporting ACLs. Fix this by passing the path_conv created in the caller down to symlink_worker. Also, while at it, simplify the handling of trailing slashes and move it out of symlink_worker. Especially use the new PC_SYM_NOFOLLOW_DIR flag to avoid fiddeling with creating a new path copy without the trailing slash. Signed-off-by: Corinna Vinschen --- winsup/cygwin/path.cc | 69 +++++++++++++++++++-------------------- winsup/cygwin/path.h | 2 +- winsup/cygwin/syscalls.cc | 41 ++++++++++++----------- 3 files changed, 57 insertions(+), 55 deletions(-) diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index 5ebbddf3a..142a73979 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -1674,7 +1674,34 @@ conv_path_list (const char *src, char *dst, size_t size, extern "C" int symlink (const char *oldpath, const char *newpath) { - return symlink_worker (oldpath, newpath, false); + path_conv win32_newpath; + + __try + { + if (!*oldpath || !*newpath) + { + set_errno (ENOENT); + __leave; + } + + /* Trailing dirsep is a no-no, only errno differs. */ + bool has_trailing_dirsep = isdirsep (newpath[strlen (newpath) - 1]); + win32_newpath.check (newpath, + PC_SYM_NOFOLLOW | PC_SYM_NOFOLLOW_DIR | PC_POSIX, + stat_suffixes); + + if (win32_newpath.error || has_trailing_dirsep) + { + set_errno (win32_newpath.error ?: + win32_newpath.exists () ? EEXIST : ENOENT); + __leave; + } + + return symlink_worker (oldpath, win32_newpath, false); + } + __except (EFAULT) {} + __endtry + return -1; } static int @@ -1846,15 +1873,12 @@ symlink_native (const char *oldpath, path_conv &win32_newpath) } int -symlink_worker (const char *oldpath, const char *newpath, bool isdevice) +symlink_worker (const char *oldpath, path_conv &win32_newpath, bool isdevice) { int res = -1; size_t len; - path_conv win32_newpath; char *buf, *cp; tmp_pathbuf tp; - unsigned check_opt; - bool has_trailing_dirsep = false; winsym_t wsym_type; /* POSIX says that empty 'newpath' is invalid input while empty @@ -1862,31 +1886,12 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice) symlink contents point to existing filesystem object */ __try { - if (!*oldpath || !*newpath) - { - set_errno (ENOENT); - __leave; - } - if (strlen (oldpath) > SYMLINK_MAX) { set_errno (ENAMETOOLONG); __leave; } - /* Trailing dirsep is a no-no. */ - len = strlen (newpath); - has_trailing_dirsep = isdirsep (newpath[len - 1]); - if (has_trailing_dirsep) - { - newpath = strdup (newpath); - ((char *) newpath)[len - 1] = '\0'; - } - - check_opt = PC_SYM_NOFOLLOW | PC_POSIX | (isdevice ? PC_NOWARN : 0); - /* We need the normalized full path below. */ - win32_newpath.check (newpath, check_opt, stat_suffixes); - /* Default symlink type is determined by global allow_winsymlinks variable. Device files are always shortcuts. */ wsym_type = isdevice ? WSYM_lnk : allow_winsymlinks; @@ -1910,8 +1915,9 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice) && (isdevice || !win32_newpath.fs_is_nfs ())) { char *newplnk = tp.c_get (); - stpcpy (stpcpy (newplnk, newpath), ".lnk"); - win32_newpath.check (newplnk, check_opt); + stpcpy (stpcpy (newplnk, win32_newpath.get_posix ()), ".lnk"); + win32_newpath.check (newplnk, PC_SYM_NOFOLLOW | PC_POSIX + | (isdevice ? PC_NOWARN : 0)); } if (win32_newpath.error) @@ -1929,11 +1935,6 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice) set_errno (EEXIST); __leave; } - if (has_trailing_dirsep && !win32_newpath.exists ()) - { - set_errno (ENOENT); - __leave; - } /* Handle NFS and native symlinks in their own functions. */ switch (wsym_type) @@ -2189,9 +2190,7 @@ symlink_worker (const char *oldpath, const char *newpath, bool isdevice) __except (EFAULT) {} __endtry syscall_printf ("%d = symlink_worker(%s, %s, %d)", - res, oldpath, newpath, isdevice); - if (has_trailing_dirsep) - free ((void *) newpath); + res, oldpath, win32_newpath.get_posix (), isdevice); return res; } @@ -3389,7 +3388,7 @@ chdir (const char *in_dir) syscall_printf ("dir '%s'", in_dir); /* Convert path. PC_NONULLEMPTY ensures that we don't check for - NULL/empty/invalid again. */ + NULL/empty/invalid again. */ path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY); if (path.error) { diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h index 7b89b03a7..a7debc108 100644 --- a/winsup/cygwin/path.h +++ b/winsup/cygwin/path.h @@ -447,4 +447,4 @@ int normalize_win32_path (const char *, char *, char *&); int normalize_posix_path (const char *, char *, char *&); PUNICODE_STRING __reg3 get_nt_native_path (const char *, UNICODE_STRING&, bool); -int __reg3 symlink_worker (const char *, const char *, bool); +int __reg3 symlink_worker (const char *, path_conv &, bool); diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index a6386dd9c..885ca375a 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -73,8 +73,7 @@ details. */ #undef _lseek64 #undef _fstat64 -static int __stdcall mknod_worker (const char *, mode_t, mode_t, _major_t, - _minor_t); +static int mknod_worker (path_conv &, mode_t, _major_t, _minor_t); /* Close all files and process any queued deletions. Lots of unix style applications will open a tmp file, unlink it, @@ -1773,15 +1772,16 @@ umask (mode_t mask) return oldmask; } +#define FILTERED_MODE(m) ((m) & (S_ISUID | S_ISGID | S_ISVTX \ + | S_IRWXU | S_IRWXG | S_IRWXO)) + int chmod_device (path_conv& pc, mode_t mode) { - return mknod_worker (pc.get_win32 (), pc.dev.mode () & S_IFMT, mode, pc.dev.get_major (), pc.dev.get_minor ()); + return mknod_worker (pc, (pc.dev.mode () & S_IFMT) | FILTERED_MODE (mode), + pc.dev.get_major (), pc.dev.get_minor ()); } -#define FILTERED_MODE(m) ((m) & (S_ISUID | S_ISGID | S_ISVTX \ - | S_IRWXU | S_IRWXG | S_IRWXO)) - /* chmod: POSIX 5.6.4.1 */ extern "C" int chmod (const char *path, mode_t mode) @@ -3364,14 +3364,12 @@ ptsname_r (int fd, char *buf, size_t buflen) return cfd->ptsname_r (buf, buflen); } -static int __stdcall -mknod_worker (const char *path, mode_t type, mode_t mode, _major_t major, - _minor_t minor) +static int +mknod_worker (path_conv &pc, mode_t mode, _major_t major, _minor_t minor) { char buf[sizeof (":\\00000000:00000000:00000000") + PATH_MAX]; - sprintf (buf, ":\\%x:%x:%x", major, minor, - type | (mode & (S_IRWXU | S_IRWXG | S_IRWXO))); - return symlink_worker (buf, path, true); + sprintf (buf, ":\\%x:%x:%x", major, minor, mode); + return symlink_worker (buf, pc, true); } extern "C" int @@ -3388,12 +3386,17 @@ mknod32 (const char *path, mode_t mode, dev_t dev) if (strlen (path) >= PATH_MAX) __leave; - path_conv w32path (path, PC_SYM_NOFOLLOW); - if (w32path.exists ()) - { - set_errno (EEXIST); - __leave; - } + /* Trailing dirsep is a no-no, only errno differs. */ + bool has_trailing_dirsep = isdirsep (path[strlen (path) - 1]); + + path_conv w32path (path, PC_SYM_NOFOLLOW | PC_SYM_NOFOLLOW_DIR + | PC_POSIX, stat_suffixes); + + if (w32path.exists () || has_trailing_dirsep) + { + set_errno (w32path.exists () ? EEXIST : ENOENT); + __leave; + } mode_t type = mode & S_IFMT; _major_t major = _major (dev); @@ -3424,7 +3427,7 @@ mknod32 (const char *path, mode_t mode, dev_t dev) __leave; } - return mknod_worker (w32path.get_win32 (), type, mode, major, minor); + return mknod_worker (w32path, mode, major, minor); } __except (EFAULT) __endtry