[PATCH 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 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', the file descriptor
returned by the call should be usable as the dirfd argument in calls
to fchownat, fstatat, and readlinkat with an empty pathname, to have
the calls operate on the symbolic link.  The second and third patches
achieve this.  For fchownat and fstatat, we do this by adding support
for the AT_EMPTY_PATH flag.  [The man page mentions linkat also, but
it already supports the AT_EMPTY_PATH flag.]

Ken Brown (3):
  Cygwin: allow opening a symlink with O_PATH | O_NOFOLLOW
  Cygwin: readlinkat: allow pathname to be empty
  Cygwin: fchownat and fstatat: support the AT_EMPTY_PATH flag

 winsup/cygwin/syscalls.cc | 51 +++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 13 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 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 2/3] Cygwin: readlinkat: 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 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..2be8693c9 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 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 3/3] Cygwin: fchownat and 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.

Add a new optional argument to gen_full_path_at to help implement
this.
---
 winsup/cygwin/syscalls.cc | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 2be8693c9..1bc856268 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4635,7 +4635,8 @@ pclose (FILE *fp)
 
 static int
 gen_full_path_at (char *path_ret, int dirfd, const char *pathname,
-  bool null_pathname_allowed = false)
+  bool null_pathname_allowed = false,
+  bool at_empty_path_flag = false)
 {
   /* Set null_pathname_allowed to true to allow GLIBC compatible behaviour
      for NULL pathname.  Only used by futimesat. */
@@ -4644,20 +4645,25 @@ gen_full_path_at (char *path_ret, int dirfd, const char *pathname,
       set_errno (EFAULT);
       return -1;
     }
+  bool empty_path = false;
   if (pathname)
     {
       if (!*pathname)
  {
-  set_errno (ENOENT);
-  return -1;
+  empty_path = true;
+  if (!at_empty_path_flag)
+    {
+      set_errno (ENOENT);
+      return -1;
+    }
  }
-      if (strlen (pathname) >= PATH_MAX)
+      else if (strlen (pathname) >= PATH_MAX)
  {
   set_errno (ENAMETOOLONG);
   return -1;
  }
     }
-  if (pathname && isabspath (pathname))
+  if (pathname && !empty_path && isabspath (pathname))
     stpcpy (path_ret, pathname);
   else
     {
@@ -4674,12 +4680,14 @@ gen_full_path_at (char *path_ret, int dirfd, const char *pathname,
   cygheap_fdget cfd (dirfd);
   if (cfd < 0)
     return -1;
-  if (!cfd->pc.isdir ())
+  if (!empty_path && !cfd->pc.isdir ())
     {
       set_errno (ENOTDIR);
       return -1;
     }
   p = stpcpy (path_ret, cfd->get_name ());
+  if (empty_path)
+    return 0;
  }
       if (!p)
  {
@@ -4785,13 +4793,14 @@ 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))
+      if (gen_full_path_at (path, dirfd, pathname, false,
+    flags & AT_EMPTY_PATH))
  __leave;
       return chown_worker (path, (flags & AT_SYMLINK_NOFOLLOW)
  ? PC_SYM_NOFOLLOW : PC_SYM_FOLLOW, uid, gid);
@@ -4808,13 +4817,14 @@ 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))
+      if (gen_full_path_at (path, dirfd, pathname, false,
+    flags & AT_EMPTY_PATH))
  __leave;
       path_conv pc (path, ((flags & AT_SYMLINK_NOFOLLOW)
    ? PC_SYM_NOFOLLOW : PC_SYM_FOLLOW)
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/3] Cygwin: fchownat and fstatat: support the AT_EMPTY_PATH flag

Ken Brown-6
On 12/28/2019 2:52 PM, Ken Brown wrote:
> 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.
>
> Add a new optional argument to gen_full_path_at to help implement
> this.

I don't like the way I did this, at least for fstatat.  If dirfd was opened with
O_PATH, I've ignored the purpose of that flag.  I'll rethink this and possibly
submit a v2.

Ken