[PATCH v2 0/5] Support opening an AF_LOCAL socket with O_PATH

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

[PATCH v2 0/5] Support opening an AF_LOCAL socket with O_PATH

Ken Brown-6
I'll follow up with the program I used to test this patch series.

Ken Brown (5):
  Cygwin: AF_LOCAL: allow opening with the O_PATH flag
  Cygwin: AF_LOCAL: set appropriate errno on system calls
  Cygwin: AF_LOCAL::fstatvfs: use our handle if O_PATH is set
  Cygwin: AF_LOCAL: fix fcntl and dup if O_PATH is set
  Cygwin: document recent changes

 winsup/cygwin/fhandler.h               |  3 ++
 winsup/cygwin/fhandler_socket_local.cc | 39 ++++++++++++++++++++++++++
 winsup/cygwin/net.cc                   | 19 ++++++++++---
 winsup/cygwin/release/3.1.3            |  2 ++
 winsup/doc/new-features.xml            |  6 ++++
 5 files changed, 65 insertions(+), 4 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/5] Cygwin: AF_LOCAL: allow opening with the O_PATH flag

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

Opening is done in a (new) fhandler_socket_local::open method by
calling fhandler_base::open_fs.

Also add a corresponding fhandler_socket_local::close method.
---
 winsup/cygwin/fhandler.h               |  2 ++
 winsup/cygwin/fhandler_socket_local.cc | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 80a78d14c..c54780ef6 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -834,6 +834,8 @@ class fhandler_socket_local: public fhandler_socket_wsock
   int getsockopt (int level, int optname, const void *optval,
   __socklen_t *optlen);
 
+  int open (int flags, mode_t mode = 0);
+  int close ();
   int __reg2 fstat (struct stat *buf);
   int __reg2 fstatvfs (struct statvfs *buf);
   int __reg1 fchmod (mode_t newmode);
diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc
index f88ced22d..e7f4fe603 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -634,6 +634,26 @@ fhandler_socket_local::dup (fhandler_base *child, int flags)
   return fhandler_socket_wsock::dup (child, flags);
 }
 
+int
+fhandler_socket_local::open (int flags, mode_t mode)
+{
+  /* We don't support opening sockets unless O_PATH is specified. */
+  if (flags & O_PATH)
+    return open_fs (flags, mode);
+
+  set_errno (EOPNOTSUPP);
+  return 0;
+}
+
+int
+fhandler_socket_local::close ()
+{
+  if (get_flags () & O_PATH)
+    return fhandler_base::close ();
+  else
+    return fhandler_socket_wsock::close ();
+}
+
 int __reg2
 fhandler_socket_local::fstat (struct stat *buf)
 {
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 2/5] Cygwin: AF_LOCAL: set appropriate errno on system calls

Ken Brown-6
In reply to this post by Ken Brown-6
If an AF_LOCAL socket is opened with O_PATH, all socket system calls
that take a file descriptor argument fail on the resulting descriptor.
Make sure that errno is set as on Linux for those calls that are
implemented on Linux.  In almost all cases it is ENOTSOCK.  There are
two exceptions:

- sockatatmark(3); errno is EBADF.

- bindresvport(3); errno is EAFNOSUPPORT if the second argument sin
  (of type struct sockaddr_in *) is non-NULL and satisfies
  sin->sin_family == AF_INET.

Finally, there are two BSD socket system calls implemented on Cygwin
but not Linux: getpeereid(3) and bindresvport_sa(3).  Set errno to
ENOTSOCK for these for consistency with the majority of the other
calls.
---
 winsup/cygwin/net.cc | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/winsup/cygwin/net.cc b/winsup/cygwin/net.cc
index 437712c63..d9f51bf68 100644
--- a/winsup/cygwin/net.cc
+++ b/winsup/cygwin/net.cc
@@ -65,8 +65,11 @@ get (const int fd)
 
   fhandler_socket *const fh = cfd->is_socket ();
 
-  if (!fh)
-    set_errno (ENOTSOCK);
+  if (!fh || (fh->get_flags () & O_PATH))
+    {
+      set_errno (ENOTSOCK);
+      return NULL;
+    }
 
   return fh;
 }
@@ -641,9 +644,17 @@ extern "C" int
 sockatmark (int fd)
 {
   int ret;
+  cygheap_fdget cfd (fd);
 
-  fhandler_socket *fh = get (fd);
-  if (fh && fh->ioctl (SIOCATMARK, &ret) != -1)
+  if (cfd < 0)
+    return -1;
+
+  fhandler_socket *const fh = cfd->is_socket ();
+  if (!fh)
+    set_errno (ENOTSOCK);
+  else if (fh->get_flags () & O_PATH)
+    set_errno (EBADF);
+  else if (fh->ioctl (SIOCATMARK, &ret) != -1)
     return ret;
   return -1;
 }
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 3/5] Cygwin: AF_LOCAL::fstatvfs: use our handle if O_PATH is set

Ken Brown-6
In reply to this post by Ken Brown-6
If O_PATH is set, then the fhandler_socket_local object has a handle
that can be used for getting the statvfs information.  Use it by
calling fhandler_base::fstatvfs_by_handle.  Without this change,
fhandler_disk_file::fstatfvs would called on a new fhandler_disk
object, which would then have to be opened.
---
 winsup/cygwin/fhandler_socket_local.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc
index e7f4fe603..76815a611 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -675,6 +675,13 @@ fhandler_socket_local::fstatvfs (struct statvfs *sfs)
 {
   if (get_sun_path () && get_sun_path ()[0] == '\0')
     return fhandler_socket_wsock::fstatvfs (sfs);
+  if (get_flags () & O_PATH)
+    /* We already have a handle. */
+    {
+      HANDLE h = get_handle ();
+      if (h)
+ return fstatvfs_by_handle (h, sfs);
+    }
   fhandler_disk_file fh (pc);
   fh.get_device () = FH_FS;
   return fh.fstatvfs (sfs);
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 4/5] Cygwin: AF_LOCAL: fix fcntl and dup if O_PATH is set

Ken Brown-6
In reply to this post by Ken Brown-6
For fcntl this requires a new method, fhandler_socket_local::fcntl,
which calls fhandler_base::fcntl if O_PATH is set and
fhandler_socket_wsock::fcntl otherwise.
---
 winsup/cygwin/fhandler.h               |  1 +
 winsup/cygwin/fhandler_socket_local.cc | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index c54780ef6..1b477f633 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -836,6 +836,7 @@ class fhandler_socket_local: public fhandler_socket_wsock
 
   int open (int flags, mode_t mode = 0);
   int close ();
+  int fcntl (int cmd, intptr_t);
   int __reg2 fstat (struct stat *buf);
   int __reg2 fstatvfs (struct statvfs *buf);
   int __reg1 fchmod (mode_t newmode);
diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc
index 76815a611..531f574b0 100644
--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -628,6 +628,9 @@ fhandler_socket_local::af_local_set_secret (char *buf)
 int
 fhandler_socket_local::dup (fhandler_base *child, int flags)
 {
+  if (get_flags () & O_PATH)
+    return fhandler_base::dup (child, flags);
+
   fhandler_socket_local *fhs = (fhandler_socket_local *) child;
   fhs->set_sun_path (get_sun_path ());
   fhs->set_peer_sun_path (get_peer_sun_path ());
@@ -654,6 +657,15 @@ fhandler_socket_local::close ()
     return fhandler_socket_wsock::close ();
 }
 
+int
+fhandler_socket_local::fcntl (int cmd, intptr_t arg)
+{
+  if (get_flags () & O_PATH)
+    return fhandler_base::fcntl (cmd, arg);
+  else
+    return fhandler_socket_wsock::fcntl (cmd, arg);
+}
+
 int __reg2
 fhandler_socket_local::fstat (struct stat *buf)
 {
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 5/5] Cygwin: document recent changes

Ken Brown-6
In reply to this post by Ken Brown-6
---
 winsup/cygwin/release/3.1.3 | 2 ++
 winsup/doc/new-features.xml | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/winsup/cygwin/release/3.1.3 b/winsup/cygwin/release/3.1.3
index f8752ad56..06ed1eb57 100644
--- a/winsup/cygwin/release/3.1.3
+++ b/winsup/cygwin/release/3.1.3
@@ -11,6 +11,8 @@ What changed:
 - Support the Linux-specific AT_EMPTY_PATH flag for fchownat(2) and
   fstatat(2).
 
+- Allow AF_LOCAL sockets to be opened with O_PATH.
+
 Bug Fixes:
 ----------
 
diff --git a/winsup/doc/new-features.xml b/winsup/doc/new-features.xml
index 967c64ac5..78c7760cf 100644
--- a/winsup/doc/new-features.xml
+++ b/winsup/doc/new-features.xml
@@ -69,6 +69,12 @@ Support the Linux-specific AT_EMPTY_PATH flag for fchownat(2) and
 fstatat(2).
 </para></listitem>
 
+<listitem><para>
+Allow AF_LOCAL sockets to be opened with O_PATH.  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.
+</para></listitem>
+
 </itemizedlist>
 
 </sect2>
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 0/5] Support opening an AF_LOCAL socket with O_PATH

Ken Brown-6
In reply to this post by Ken Brown-6
On 1/29/2020 12:22 PM, Ken Brown wrote:
> I'll follow up with the program I used to test this patch series.

Attached.

$ gcc -Wall -o o_path_socket_test o_path_socket_test.c

$ ./o_path_socket_test.exe
The following calls should fail with EBADF:
read: OK
write: OK
fchmod: OK
fchown: OK
ioctl: OK
fgetxattr: OK
mmap: OK
sockatmark: OK

The following calls should fail with ENOTSOCK:
sendto: OK
recvfrom: OK
setsockopt: OK
getsockopt: OK
getpeereid: OK
connect: OK
accept: OK
accept4: OK
bind: OK
getsockname: OK
listen: OK
shutdown: OK
getpeername: OK
recv: OK
send: OK
recvmsg: OK
sendmsg: OK
bindresvport_sa: OK
bindresvport with NULL arg: OK
bindresvport with valid sin arg: OK

The following call should fail with EAFNOSUPPORT:
bindresvport with invalid sin arg: OK

The following calls should succeed:
fstat: OK
fstatvfs: OK
fcntl_dup: OK
fcntl_getfl: OK
fcntl_setfl: OK
fcntl_getfd: OK
fcntl_setfd: OK
close: OK

o_path_socket_test.c (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 4/5] Cygwin: AF_LOCAL: fix fcntl and dup if O_PATH is set

Corinna Vinschen-2
In reply to this post by Ken Brown-6
On Jan 29 17:22, Ken Brown wrote:
> For fcntl this requires a new method, fhandler_socket_local::fcntl,
> which calls fhandler_base::fcntl if O_PATH is set and
> fhandler_socket_wsock::fcntl otherwise.

The patchset looks great.  Please apply with just a minor change:

Can you please add a hint why using fhandler_base::dup and
fhandler_base::fcntl is sufficient, despite fhandler_disk_file has its
own methods?  It's clear from looking at those functions, but a quick
description in the commit message and a one-line comment each in the
source might be helpful when debugging at one point.


Thanks,
Corinna


> ---
>  winsup/cygwin/fhandler.h               |  1 +
>  winsup/cygwin/fhandler_socket_local.cc | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> index c54780ef6..1b477f633 100644
> --- a/winsup/cygwin/fhandler.h
> +++ b/winsup/cygwin/fhandler.h
> @@ -836,6 +836,7 @@ class fhandler_socket_local: public fhandler_socket_wsock
>  
>    int open (int flags, mode_t mode = 0);
>    int close ();
> +  int fcntl (int cmd, intptr_t);
>    int __reg2 fstat (struct stat *buf);
>    int __reg2 fstatvfs (struct statvfs *buf);
>    int __reg1 fchmod (mode_t newmode);
> diff --git a/winsup/cygwin/fhandler_socket_local.cc b/winsup/cygwin/fhandler_socket_local.cc
> index 76815a611..531f574b0 100644
> --- a/winsup/cygwin/fhandler_socket_local.cc
> +++ b/winsup/cygwin/fhandler_socket_local.cc
> @@ -628,6 +628,9 @@ fhandler_socket_local::af_local_set_secret (char *buf)
>  int
>  fhandler_socket_local::dup (fhandler_base *child, int flags)
>  {
> +  if (get_flags () & O_PATH)
> +    return fhandler_base::dup (child, flags);
> +
>    fhandler_socket_local *fhs = (fhandler_socket_local *) child;
>    fhs->set_sun_path (get_sun_path ());
>    fhs->set_peer_sun_path (get_peer_sun_path ());
> @@ -654,6 +657,15 @@ fhandler_socket_local::close ()
>      return fhandler_socket_wsock::close ();
>  }
>  
> +int
> +fhandler_socket_local::fcntl (int cmd, intptr_t arg)
> +{
> +  if (get_flags () & O_PATH)
> +    return fhandler_base::fcntl (cmd, arg);
> +  else
> +    return fhandler_socket_wsock::fcntl (cmd, arg);
> +}
> +
>  int __reg2
>  fhandler_socket_local::fstat (struct stat *buf)
>  {
> --
> 2.21.0
--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment