Re: [PATCH] Cygwin: normalize_win32_path: allow drive without trailing backslash
On Jan 15 17:46, Ken Brown wrote:
> Commit 283cb372, "Cygwin: normalize_win32_path: improve error
> checking", required a prefix '\\?\' or '\??\' in the source path to be
> followed by 'UNC\' or 'X:\', where X is a drive letter. That was too
> restrictive, since it disallowed the paths '\\?\X: and '\??\X:'. This
> caused problems when a user tried to use the root of a drive as the
> Cygwin installation root, as reported here:
>
> https://cygwin.com/ml/cygwin/2020-01/msg00111.html
>
> Modify the requirement so that '\??\X:' and '\\?\X:' are now allowed
> as source paths, without a trailing backslash.
> ---
> winsup/cygwin/path.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
> index c8e73c64c..a00270210 100644
> --- a/winsup/cygwin/path.cc
> +++ b/winsup/cygwin/path.cc
> @@ -1411,7 +1411,7 @@ normalize_win32_path (const char *src, char *dst, char
> *&tail)
>&& src[2] == '?' && isdirsep (src[3]))
> {
>src += 4;
> - if (isdrive (src) && isdirsep (src[2]))
> + if (isdrive (src) && (isdirsep (src[2]) || !src[2]))
> beg_src_slash = false;
>else if (!strncmp (src, "UNC", 3) && isdirsep (src[3]))
> /* native UNC path */
> --
> 2.21.0
Looks good, please push.
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
signature.asc
Description: PGP signature
Re: [PATCH] Cygwin: allow opening an AF_LOCAL/AF_UNIX socket with O_PATH
On Jan 16 18:34, Ken Brown wrote: > If that flag is not set, or if an attempt is made to open a different > type of socket, the errno is now EOPNOTSUPP instead of ENXIO. This is > consistent with POSIX, starting with the 2016 edition. Earlier > editions were silent on this issue. > --- > winsup/cygwin/fhandler.h | 2 ++ > winsup/cygwin/fhandler_socket.cc | 2 +- > winsup/cygwin/fhandler_socket_local.cc | 16 > winsup/cygwin/fhandler_socket_unix.cc | 16 > winsup/cygwin/release/3.1.3| 7 +++ > winsup/doc/new-features.xml| 6 ++ > 6 files changed, 48 insertions(+), 1 deletion(-) I'm a bit concerned here that some function calls might succeed accidentally or even crash, given that the original socket code doesn't cope with the nohandle flag. Did you perform some basic testing? Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer signature.asc Description: PGP signature
Re: [PATCH] Cygwin: allow opening an AF_LOCAL/AF_UNIX socket with O_PATH
On Jan 17 10:48, Corinna Vinschen wrote: > On Jan 16 18:34, Ken Brown wrote: > > If that flag is not set, or if an attempt is made to open a different > > type of socket, the errno is now EOPNOTSUPP instead of ENXIO. This is > > consistent with POSIX, starting with the 2016 edition. Earlier > > editions were silent on this issue. > > --- > > winsup/cygwin/fhandler.h | 2 ++ > > winsup/cygwin/fhandler_socket.cc | 2 +- > > winsup/cygwin/fhandler_socket_local.cc | 16 > > winsup/cygwin/fhandler_socket_unix.cc | 16 > > winsup/cygwin/release/3.1.3| 7 +++ > > winsup/doc/new-features.xml| 6 ++ > > 6 files changed, 48 insertions(+), 1 deletion(-) > > I'm a bit concerned here that some function calls might succeed > accidentally or even crash, given that the original socket code doesn't > cope with the nohandle flag. Did you perform some basic testing? Iow, do the usual socket calls on a fhandler_socket_local return EBADF now? Ignoring fhandler_socket_unix for now. Corinna -- Corinna Vinschen Cygwin Maintainer signature.asc Description: PGP signature
Re: [PATCH v3 2/3] Cygwin: readlinkat, fchownat: allow pathname to be empty
On Jan 16 20:50, Ken Brown wrote:
> Following Linux, allow the pathname argument to be an empty string,
> provided the dirfd argument refers to a symlink opened with O_PATH and
> O_NOFOLLOW. The readlinkat or fchownat call then operates on that
> symlink. In the case of fchownat, the call must specify the
> AT_EMPTY_PATH flag.
> ---
> winsup/cygwin/syscalls.cc | 40 ++-
> 1 file changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 038a316db..3d87fd685 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -4785,14 +4785,29 @@ fchownat (int dirfd, const char *pathname, uid_t uid,
> gid_t gid, int flags)
>tmp_pathbuf tp;
>__try
> {
> - if (flags & ~AT_SYMLINK_NOFOLLOW)
> + if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH))
> {
> set_errno (EINVAL);
> __leave;
> }
>char *path = tp.c_get ();
> - if (gen_full_path_at (path, dirfd, pathname))
> - __leave;
> + int res = gen_full_path_at (path, dirfd, pathname);
> + if (res)
> + {
> + if (!(errno == ENOENT && (flags & AT_EMPTY_PATH)))
> + __leave;
> + /* pathname is an empty string. This is OK if dirfd refers
> + to a symlink that was opened with O_PATH and O_NOFOLLOW.
> + In this case, fchownat operates on the symlink. */
> + cygheap_fdget cfd (dirfd);
> + if (cfd < 0)
> + __leave;
> + if (!(cfd->issymlink ()
> + && cfd->get_flags () & O_PATH
> + && cfd->get_flags () & O_NOFOLLOW))
> + __leave;
I think this is not quite right. Per the Linux man page of fchownat,
if AT_EMPTY_PATH is given, any file type is ok as dirfd:
AT_EMPTY_PATH (since Linux 2.6.39)
If pathname is an empty string, operate on the file referred to
by dirfd (which may have been obtained using the open(2) O_PATH
flag). In this case, dirfd can refer to any type of file, not
just a directory...
Additionally AT_FDCWD is allowed, too:
... If dirfd is AT_FDCWD, the call operates on
the current working directory.
> + return lchown (cfd->get_name (), uid, gid);
Instead of calling lchown, this code could also just tweak the flags
and fall through to the below chown_worker call, I think, just as in
readlinkat below:
strcpy (path, cfd->get_name ());
flags = AT_SYMLINK_NOFOLLOW;
> + }
>return chown_worker (path, (flags & AT_SYMLINK_NOFOLLOW)
>? PC_SYM_NOFOLLOW : PC_SYM_FOLLOW, uid, gid);
> }
> @@ -4979,8 +4994,23 @@ readlinkat (int dirfd, const char *__restrict
> pathname, char *__restrict buf,
>__try
> {
>char *path = tp.c_get ();
> - if (gen_full_path_at (path, dirfd, pathname))
> - __leave;
> + int res = gen_full_path_at (path, dirfd, pathname);
> + if (res)
> + {
> + if (errno != ENOENT)
> + __leave;
> + /* pathname is an empty string. This is OK if dirfd refers
> + to a symlink that was opened with O_PATH and O_NOFOLLOW.
> + In this case, readlinkat operates on the symlink. */
> + cygheap_fdget cfd (dirfd);
> + if (cfd < 0)
> + __leave;
> + if (!(cfd->issymlink ()
> + && cfd->get_flags () & O_PATH
> + && cfd->get_flags () & O_NOFOLLOW))
> + __leave;
> + strcpy (path, cfd->get_name ());
> + }
>return readlink (path, buf, bufsize);
> }
>__except (EFAULT) {}
> --
> 2.21.0
Thanks,
Corinna
--
Corinna Vinschen
Cygwin Maintainer
signature.asc
Description: PGP signature
[PATCH v4 1/4] Cygwin: allow opening a symlink with O_PATH | O_NOFOLLOW
Up to now, opening a symlink with O_NOFOLLOW fails with ELOOP.
Following Linux, allow this to succeed if O_PATH is also specified.
---
winsup/cygwin/syscalls.cc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 20126ce10..038a316db 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -1470,7 +1470,7 @@ open (const char *unix_path, int flags, ...)
if (!(fh = build_fh_name (unix_path, opt, stat_suffixes)))
__leave;/* errno already set */
- if ((flags & O_NOFOLLOW) && fh->issymlink ())
+ if ((flags & O_NOFOLLOW) && fh->issymlink () && !(flags & O_PATH))
{
set_errno (ELOOP);
__leave;
--
2.21.0
[PATCH v4 0/4] Support opening a symlink with O_PATH | O_NOFOLLOW
Currently, opening a symlink with O_NOFOLLOW fails with ELOOP. Following Linux, the first patch in this series allows the call to succeed if O_PATH is also specified. According to the Linux man page for open(2), "the call returns a file descriptor referring to the symbolic link. This file descriptor can be used as the dirfd argument in calls to fchownat(2), fstatat(2), linkat(2), and readlinkat(2) with an empty pathname to have the calls operate on the symbolic link." The second patch achieves this for readlinkat. The third patch does this for fstatat and fchownat by adding support for the AT_EMPTY_PATH flag. Nothing needs to be done for linkat, which already supports the AT_EMPTY_PATH flag. Ken Brown (4): Cygwin: allow opening a symlink with O_PATH | O_NOFOLLOW Cygwin: readlinkat: allow pathname to be empty Cygwin: fstatat, fchownat: support the AT_EMPTY_PATH flag Cygwin: document recent changes winsup/cygwin/release/3.1.3 | 19 +-- winsup/cygwin/syscalls.cc | 68 - winsup/doc/new-features.xml | 19 +++ 3 files changed, 94 insertions(+), 12 deletions(-) -- 2.21.0
[PATCH v4 2/4] Cygwin: readlinkat: allow pathname to be empty
Following Linux, allow the pathname argument to be an empty string,
provided the dirfd argument refers to a symlink opened with
O_PATH | O_NOFOLLOW. The readlinkat call then operates on that
symlink.
---
winsup/cygwin/syscalls.cc | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 038a316db..282d9e0ee 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4979,8 +4979,23 @@ readlinkat (int dirfd, const char *__restrict pathname,
char *__restrict buf,
__try
{
char *path = tp.c_get ();
- if (gen_full_path_at (path, dirfd, pathname))
- __leave;
+ int res = gen_full_path_at (path, dirfd, pathname);
+ if (res)
+ {
+ if (errno != ENOENT)
+ __leave;
+ /* pathname is an empty string. This is OK if dirfd refers
+to a symlink that was opened with O_PATH | O_NOFOLLOW.
+In this case, readlinkat operates on the symlink. */
+ cygheap_fdget cfd (dirfd);
+ if (cfd < 0)
+ __leave;
+ if (!(cfd->issymlink ()
+ && cfd->get_flags () & O_PATH
+ && cfd->get_flags () & O_NOFOLLOW))
+ __leave;
+ strcpy (path, cfd->get_name ());
+ }
return readlink (path, buf, bufsize);
}
__except (EFAULT) {}
--
2.21.0
[PATCH v4 4/4] Cygwin: document recent changes
--- winsup/cygwin/release/3.1.3 | 19 --- winsup/doc/new-features.xml | 15 +++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/winsup/cygwin/release/3.1.3 b/winsup/cygwin/release/3.1.3 index 489741136..425d8bb2d 100644 --- a/winsup/cygwin/release/3.1.3 +++ b/winsup/cygwin/release/3.1.3 @@ -1,5 +1,18 @@ -Bug Fixes -- +What changed: +- + +- Allow symlinks to be opened with O_PATH | O_NOFOLLOW. + +- Allow the pathname argument to readlinkat(2) to be an empty string, + provided the dirfd argument refers to a symlink opened with + O_PATH | O_NOFOLLOW. The readlinkat call then operates on that + symlink. + +- Support the Linux-specific AT_EMPTY_PATH flag for fchownat(2) and + fstatat(2). + +Bug Fixes: +-- - Define CPU_SETSIZE, as on Linux. Addresses: https://cygwin.com/ml/cygwin/2019-12/msg00248.html @@ -7,6 +20,6 @@ Bug Fixes - Fix the problem which overrides the code page setting. Addresses: https://www.cygwin.com/ml/cygwin/2019-12/msg00292.html -- Fix a regression that prevents the root of a drive from being the +- Fix a regression that prevented the root of a drive from being the Cygwin installation root. Addresses: https://cygwin.com/ml/cygwin/2020-01/msg00111.html diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml index 65bdc17ab..967c64ac5 100644 --- a/winsup/doc/new-features.xml +++ b/winsup/doc/new-features.xml @@ -54,6 +54,21 @@ Allow times(2) to have a NULL argument, as on Linux. Improve /proc/cpuinfo output and align more closely with Linux. + +Allow symlinks to be opened with O_PATH | O_NOFOLLOW. + + + +Allow the pathname argument to readlinkat(2) to be an empty string, +provided the dirfd argument refers to a symlink opened with O_PATH | +O_NOFOLLOW. The readlinkat call then operates on that symlink. + + + +Support the Linux-specific AT_EMPTY_PATH flag for fchownat(2) and +fstatat(2). + + -- 2.21.0
[PATCH v4 3/4] Cygwin: fstatat, fchownat: support the AT_EMPTY_PATH flag
Following Linux, allow the pathname argument to be an empty string if
the AT_EMPTY_PATH flag is specified. In this case the dirfd argument
can refer to any type of file, not just a directory, and the call
operates on that file. In particular, dirfd can refer to a symlink
that was opened with O_PATH and O_NOFOLLOW.
---
winsup/cygwin/syscalls.cc | 47 ++-
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 282d9e0ee..4956b6ff5 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4785,14 +4785,36 @@ fchownat (int dirfd, const char *pathname, uid_t uid,
gid_t gid, int flags)
tmp_pathbuf tp;
__try
{
- if (flags & ~AT_SYMLINK_NOFOLLOW)
+ if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH))
{
set_errno (EINVAL);
__leave;
}
char *path = tp.c_get ();
- if (gen_full_path_at (path, dirfd, pathname))
- __leave;
+ int res = gen_full_path_at (path, dirfd, pathname);
+ if (res)
+ {
+ if (!(errno == ENOENT && (flags & AT_EMPTY_PATH)))
+ __leave;
+ /* pathname is an empty string. Operate on dirfd. */
+ if (dirfd == AT_FDCWD)
+ {
+ cwdstuff::cwd_lock.acquire ();
+ strcpy (path, cygheap->cwd.get_posix ());
+ cwdstuff::cwd_lock.release ();
+ }
+ else
+ {
+ cygheap_fdget cfd (dirfd);
+ if (cfd < 0)
+ __leave;
+ strcpy (path, cfd->get_name ());
+ /* If dirfd refers to a symlink (which was necessarily
+opened with O_PATH | O_NOFOLLOW), we must operate
+directly on that symlink.. */
+ flags = AT_SYMLINK_NOFOLLOW;
+ }
+ }
return chown_worker (path, (flags & AT_SYMLINK_NOFOLLOW)
? PC_SYM_NOFOLLOW : PC_SYM_FOLLOW, uid, gid);
}
@@ -4808,14 +4830,27 @@ fstatat (int dirfd, const char *__restrict pathname,
struct stat *__restrict st,
tmp_pathbuf tp;
__try
{
- if (flags & ~AT_SYMLINK_NOFOLLOW)
+ if (flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH))
{
set_errno (EINVAL);
__leave;
}
char *path = tp.c_get ();
- if (gen_full_path_at (path, dirfd, pathname))
- __leave;
+ int res = gen_full_path_at (path, dirfd, pathname);
+ if (res)
+ {
+ if (!(errno == ENOENT && (flags & AT_EMPTY_PATH)))
+ __leave;
+ /* pathname is an empty string. Operate on dirfd. */
+ if (dirfd == AT_FDCWD)
+ {
+ cwdstuff::cwd_lock.acquire ();
+ strcpy (path, cygheap->cwd.get_posix ());
+ cwdstuff::cwd_lock.release ();
+ }
+ else
+ return fstat (dirfd, st);
+ }
path_conv pc (path, ((flags & AT_SYMLINK_NOFOLLOW)
? PC_SYM_NOFOLLOW : PC_SYM_FOLLOW)
| PC_POSIX | PC_KEEP_HANDLE, stat_suffixes);
--
2.21.0
