[PATCH v4 0/4] Support opening a symlink with O_PATH | O_NOFOLLOW

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

[PATCH v4 0/4] 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.  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

Reply | Threaded
Open this post in threaded view
|

[PATCH v4 1/4] 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 v4 2/4] 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 | 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v4 3/4] Cygwin: fstatat, fchownat: 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 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

Reply | Threaded
Open this post in threaded view
|

[PATCH v4 4/4] Cygwin: document recent changes

Ken Brown-6
In reply to this post by Ken Brown-6
---
 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.
 </para></listitem>
 
+<listitem><para>
+Allow symlinks to be opened with O_PATH | O_NOFOLLOW.
+</para></listitem>
+
+<listitem><para>
+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.
+</para></listitem>
+
+<listitem><para>
+Support the Linux-specific AT_EMPTY_PATH flag for fchownat(2) and
+fstatat(2).
+</para></listitem>
+
 </itemizedlist>
 
 </sect2>
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v4 0/4] Support opening a symlink with O_PATH | O_NOFOLLOW

Corinna Vinschen-2
In reply to this post by Ken Brown-6
On Jan 17 16:10, Ken Brown wrote:

> 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(-)
This looks good to me.  Please push.  I just wonder if this isn't
new feature enough to bump the Cygwin version to 3.2...


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v4 0/4] Support opening a symlink with O_PATH | O_NOFOLLOW

Ken Brown-6
On 1/20/2020 4:56 AM, Corinna Vinschen wrote:

> On Jan 17 16:10, Ken Brown wrote:
>> 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(-)
>
> This looks good to me.  Please push.  I just wonder if this isn't
> new feature enough to bump the Cygwin version to 3.2...

Maybe.  You're in a better position to judge this than I am.  If you decide to
do it, I'll tweak the documentation accordingly.

Ken
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v4 0/4] Support opening a symlink with O_PATH | O_NOFOLLOW

Corinna Vinschen-2
On Jan 20 14:57, Ken Brown wrote:

> On 1/20/2020 4:56 AM, Corinna Vinschen wrote:
> > On Jan 17 16:10, Ken Brown wrote:
> >> 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(-)
> >
> > This looks good to me.  Please push.  I just wonder if this isn't
> > new feature enough to bump the Cygwin version to 3.2...
>
> Maybe.  You're in a better position to judge this than I am.  If you decide to
> do it, I'll tweak the documentation accordingly.
I'm not sure yet.  Just push as is for now.  Tweaking the docs for
3.2 is trivial.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment