Re: fstat and similar methods in fhandler_socket_local and fhandler_socket_unix

[Ken Brown <kbrown@cornell.edu>]

On 2/24/2021 12:27 PM, Ken Brown via Cygwin-developers wrote:
> On 2/24/2021 12:06 PM, Corinna Vinschen via Cygwin-developers wrote:
>> On Feb 24 10:32, Ken Brown via Cygwin-developers wrote:
>>> On 2/24/2021 4:06 AM, Corinna Vinschen via Cygwin-developers wrote:
>>>> On Feb 22 15:01, Ken Brown via Cygwin-developers wrote:
>>>>> OK, I've got patches ready to go for fstat, fstatvfs, fchmod, fchown, facl,
>>>>> and link.  I have to test them, and then I'll send them to cygwin-patches.
>>>>> In the case of link, I think the only way an fhandler_socket's link method
>>>>> can be called is through link(2) being called on a socket file.  So the
>>>>> following should do the job:
>>>>>
>>>>> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
>>>>> index 21e1df172..f4aa12956 100644
>>>>> --- a/winsup/cygwin/fhandler.h
>>>>> +++ b/winsup/cygwin/fhandler.h
>>>>> @@ -611,7 +611,6 @@ class fhandler_socket: public fhandler_base
>>>>>      int __reg1 fchmod (mode_t newmode);
>>>>>      int __reg2 fchown (uid_t newuid, gid_t newgid);
>>>>>      int __reg3 facl (int, int, struct acl *);
>>>>> -  int __reg2 link (const char *);
>>>>>      off_t lseek (off_t, int)
>>>>>      {
>>>>>        set_errno (ESPIPE);
>>>>> diff --git a/winsup/cygwin/fhandler_socket.cc 
>>>>> b/winsup/cygwin/fhandler_socket.cc
>>>>> index f22412650..08870cc6b 100644
>>>>> --- a/winsup/cygwin/fhandler_socket.cc
>>>>> +++ b/winsup/cygwin/fhandler_socket.cc
>>>>> @@ -357,9 +357,3 @@ fhandler_socket::facl (int cmd, int nentries, aclent_t 
>>>>> *aclbufp)
>>>>>      set_errno (EOPNOTSUPP);
>>>>>      return -1;
>>>>>    }
>>>>> -
>>>>> -int
>>>>> -fhandler_socket::link (const char *newpath)
>>>>> -{
>>>>> -  return fhandler_base::link (newpath);
>>>>> -}
>>>>
>>>> Hmm.  What about linkat (AT_EMPTY_PATH, socket, ...)?

While working on this, I discovered that linkat(AT_EMPTY_PATH) doesn't currently 
work on socket files opened with O_PATH.  I started debugging this (not finished 
yet), and it occurred to me that the implementation of linkat(AT_EMPTY_PATH) 
could be simplified like this:

--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -4962,6 +4962,8 @@ linkat (int olddirfd, const char *oldpathname,
         int flags)
  {
    tmp_pathbuf tp;
+  fhandler_base *fh = NULL;
+
    __try
      {
        if (flags & ~(AT_SYMLINK_FOLLOW | AT_EMPTY_PATH))
@@ -4970,21 +4972,19 @@ linkat (int olddirfd, const char *oldpathname,
           __leave;
         }
        char *oldpath = tp.c_get ();
-      /* AT_EMPTY_PATH with an empty oldpathname is equivalent to
-
-          linkat(AT_FDCWD, "/proc/self/fd/<olddirfd>", newdirfd,
-                 newname, AT_SYMLINK_FOLLOW);
-
-        Convert the request accordingly. */
        if ((flags & AT_EMPTY_PATH) && oldpathname && oldpathname[0] == '\0')
         {
+         /* Operate directly on olddirfd. */
           if (olddirfd == AT_FDCWD)
             {
               set_errno (EPERM);
               __leave;
             }
-         __small_sprintf (oldpath, "/proc/%d/fd/%d", myself->pid, olddirfd);
-         flags = AT_SYMLINK_FOLLOW;
+         cygheap_fdget cfd (olddirfd);
+         if (cfd < 0)
+           __leave;
+         fh = cfd;
+         flags = 0;
         }
        else if (gen_full_path_at (oldpath, olddirfd, oldpathname))
         __leave;
@@ -5003,6 +5003,8 @@ linkat (int olddirfd, const char *oldpathname,
             }
           strcpy (oldpath, old_name.get_posix ());
         }
+      if (fh)
+       return fh->link (newpath);
        return link (oldpath, newpath);
      }
    __except (EFAULT) {}

This seems to work fine on regular files and on socket files opened with O_PATH, 
after I tweak fhandler_socket_local::link (see below).  I haven't tested other 
file types yet.

$ cat linkat_test.c
#define _GNU_SOURCE
#include <sys/types.h>
#include <time.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>

#define FILE "/tmp/myfile"
#define NEW_FILE "/tmp/mynewfile"
#define SOCK_PATH "/tmp/mysocket"
#define NEW_SOCK_PATH "/tmp/mynewsocket"

int
main ()
{
   struct sockaddr_un addr;
   int fd;

   /* Regular file. */
   if (unlink (FILE) < 0  && errno != ENOENT)
     {
       perror ("unlink");
       exit (1);
     }
   if (unlink (NEW_FILE) < 0  && errno != ENOENT)
     {
       perror ("unlink");
       exit (1);
     }
   fd = open (FILE, O_RDONLY | O_CREAT, S_IRUSR | S_IWUSR);
   if (fd < 0)
     {
       perror ("open");
       exit (1);
     }
   printf ("Testing linkat (AT_EMPTY_PATH) on a regular file...\n");
   if (linkat (fd, "", AT_FDCWD, NEW_FILE, AT_EMPTY_PATH) < 0)
     perror ("linkat");
   else
     printf ("success\n\n");
   if (close (fd) < 0)
     {
       perror ("close");
       exit (1);
     }

   /* Socket file. */
   if (unlink (SOCK_PATH) < 0  && errno != ENOENT)
     {
       perror ("unlink");
       exit (1);
     }
   if (unlink (NEW_SOCK_PATH) < 0  && errno != ENOENT)
     {
       perror ("unlink");
       exit (1);
     }
   fd = socket (AF_UNIX, SOCK_STREAM, 0);
   if (fd < 0)
     {
       perror ("socket");
       exit (1);
     }
   memset (&addr, 0, sizeof (struct sockaddr_un));
   addr.sun_family = AF_UNIX;
   strcpy (addr.sun_path, SOCK_PATH);
   if (bind (fd, (struct sockaddr *) &addr, sizeof(struct sockaddr_un)) < 0)
     {
       perror ("bind");
       exit (1);
     }
   if (close (fd) < 0)
     {
       perror ("close");
       exit (1);
     }

   fd = open (SOCK_PATH, O_PATH);
   if (fd < 0)
     {
       perror ("open");
       exit (1);
     }

   printf ("Testing linkat (AT_EMPTY_PATH) on a socket file...\n");
   if (linkat (fd, "", AT_FDCWD, NEW_SOCK_PATH, AT_EMPTY_PATH) < 0)
     perror ("linkat");
   else
     printf ("success\n");
   if (close (fd) < 0)
     {
       perror ("close");
       exit (1);
     }
}

$ gcc -g -O0 -o linkat_test linkat_test.c

$ ./linkat_test.exe
Testing linkat (AT_EMPTY_PATH) on a regular file...
success

Testing linkat (AT_EMPTY_PATH) on a socket file...
success

$ ls -l /tmp/my*
-rw------- 2 kbrown None 0 2021-02-25 07:59 /tmp/myfile
-rw------- 2 kbrown None 0 2021-02-25 07:59 /tmp/mynewfile
srw-r--r-- 2 kbrown None 0 2021-02-25 07:59 /tmp/mynewsocket=
srw-r--r-- 2 kbrown None 0 2021-02-25 07:59 /tmp/mysocket=

And here's the tweak to fhandler_socket_local::link:

--- a/winsup/cygwin/fhandler_socket_local.cc
+++ b/winsup/cygwin/fhandler_socket_local.cc
@@ -749,9 +749,14 @@ fhandler_socket_local::facl (int cmd, int nentries, 
aclent_t *aclbufp)
  int
  fhandler_socket_local::link (const char *newpath)
  {
-  if (get_sun_path () && get_sun_path ()[0] == '\0')
+  if (!dev ().isfs ())
+    /* linkat w/ AT_EMPTY_PATH called on a socket not opened w/ O_PATH. */
      return fhandler_socket_wsock::link (newpath);
+  /* link on a socket file or linkat w/ AT_EMPTY_PATH called on a
+     socket opened w/ O_PATH. */
    fhandler_disk_file fh (pc);
+  if (get_flags () & O_PATH)
+    fh.set_handle (get_handle ());
    return fh.link (newpath);
  }

I'll put all this together in a patchset, along with the tweaks to fstat, etc., 
but first I wanted to get your reaction to the proposed simplification of 
linkat(AT_EMPTY_PATH).

Thanks.

Ken