[PATCH v3 0/3] Support opening a symlink with O_PATH | O_NOFOLLOW

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 0/3] Support opening a symlink with O_PATH | O_NOFOLLOW

Ken Brown-6
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 and fchownat.  The third
patch does this for fstatat 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 (3):
  Cygwin: allow opening a symlink with O_PATH | O_NOFOLLOW
  Cygwin: readlinkat, fchownat: allow pathname to be empty
  Cygwin: fstatat: support the AT_EMPTY_PATH flag

 winsup/cygwin/syscalls.cc | 61 +++++++++++++++++++++++++++++++++------
 1 file changed, 52 insertions(+), 9 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 1/3] Cygwin: allow opening a symlink with O_PATH | O_NOFOLLOW

Ken Brown-6
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

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 2/3] Cygwin: readlinkat, fchownat: allow pathname to be empty

Ken Brown-6
In reply to this post by Ken Brown-6
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;
+  return lchown (cfd->get_name (), uid, gid);
+ }
       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

Reply | Threaded
Open this post in threaded view
|

[PATCH v3 3/3] Cygwin: fstatat: support the AT_EMPTY_PATH flag

Ken Brown-6
In reply to this post by Ken Brown-6
Following Linux, allow the pathname argument to be empty if the
AT_EMPTY_PATH 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 | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 3d87fd685..e7298fd43 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4823,14 +4823,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

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 2/3] Cygwin: readlinkat, fchownat: allow pathname to be empty

Corinna Vinschen-2
In reply to this post by Ken Brown-6
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 (849 bytes) Download Attachment