public inbox for cygwin-developers@cygwin.com
 help / color / mirror / Atom feed
From: Ken Brown <kbrown@cornell.edu>
To: cygwin-devel <cygwin-developers@cygwin.com>
Subject: fstat and similar methods in fhandler_socket_local and fhandler_socket_unix
Date: Sat, 20 Feb 2021 18:32:46 -0500	[thread overview]
Message-ID: <e90a3f4f-4aee-061a-f352-bdaa3dbfb9b6@cornell.edu> (raw)

fhandler_socket_local::fstat and many similar methods seem to assume that the 
fhandler is based on a socket *file*.  When this is not the case, they end up 
using handles inappropriately.  Consider the following test case, for example.

$ cat socket_stat.c
#include <time.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/stat.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

void print_stat (struct stat);

int
main ()
{
   struct stat st;
   int fd;

   fd = socket (AF_UNIX, SOCK_STREAM, 0);
   if (fd == -1)
     {
       perror ("socket");
       exit (1);
     }
   if (fstat (fd, &st) < 0)
     {
       perror ("fstat");
       exit (1);
     }
   print_stat (st);
}

/* https://man7.org/linux/man-pages/man2/lstat.2.html */
void
print_stat (struct stat st)
{
   printf("File type:                ");

   switch (st.st_mode & S_IFMT) {
   case S_IFBLK:  printf("block device\n");            break;
   case S_IFCHR:  printf("character device\n");        break;
   case S_IFDIR:  printf("directory\n");               break;
   case S_IFIFO:  printf("FIFO/pipe\n");               break;
   case S_IFLNK:  printf("symlink\n");                 break;
   case S_IFREG:  printf("regular file\n");            break;
   case S_IFSOCK: printf("socket\n");                  break;
   default:       printf("unknown?\n");                break;
   }

   printf("I-node number:            %ld\n", (long) st.st_ino);
   printf("dev:                      %lu\n", st.st_dev);
   printf("rdev:                     %lu\n", st.st_rdev);
   printf("Mode:                     %lo (octal)\n",
          (unsigned long) st.st_mode);

   printf("Link count:               %ld\n", (long) st.st_nlink);
   printf("Ownership:                UID=%ld   GID=%ld\n",
          (long) st.st_uid, (long) st.st_gid);

   printf("Preferred I/O block size: %ld bytes\n",
          (long) st.st_blksize);
   printf("File size:                %lld bytes\n",
          (long long) st.st_size);
   printf("Blocks allocated:         %lld\n",
          (long long) st.st_blocks);

   printf("Last status change:       %s", ctime(&st.st_ctime));
   printf("Last file access:         %s", ctime(&st.st_atime));
   printf("Last file modification:   %s", ctime(&st.st_mtime));
   printf("\n");
}

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

$ ./socket_stat.exe
File type:                socket
I-node number:            4
dev:                      1966200
rdev:                     1966200
Mode:                     140644 (octal)
Link count:               0
Ownership:                UID=197609   GID=197121
Preferred I/O block size: 65536 bytes
File size:                0 bytes
Blocks allocated:         0
Last status change:       Wed Dec 31 19:00:00 1969
Last file access:         Wed Dec 31 19:00:00 1969
Last file modification:   Wed Dec 31 19:00:00 1969

On Linux the output is:

File type:                socket
I-node number:            117
dev:                      0
rdev:                     0
Mode:                     140777 (octal)
Link count:               1
Ownership:                UID=1000   GID=1000
Preferred I/O block size: 512 bytes
File size:                0 bytes
Blocks allocated:         0
Last status change:       Sat Feb 20 18:13:50 2021
Last file access:         Sat Feb 20 18:13:50 2021
Last file modification:   Sat Feb 20 18:13:50 2021

I haven't been able to find any documentation about what fstat(2) should do on a 
socket descriptor, so maybe the difference between Cygwin and Linux is 
unimportant.  But the code path followed on Cygwin definitely seems wrong. 
fhandler_socket_local::fstat calls fstat_fs which ends up using the io_handle as 
though it were a file handle.  It seems to me that fstat_fs should only be 
called if we have a path_conv handle (which is the case if the fstat call 
resulted from a stat call) or if the socket descriptor came from open with 
O_PATH set.

Similar remarks apply to fstatfvs, fchmod,....  In some of these cases, like 
fchmod, POSIX explicitly states that the behavior is undefined, so maybe we 
should just fail when there's no underlying socket file.

Ken

             reply	other threads:[~2021-02-20 23:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-20 23:32 Ken Brown [this message]
2021-02-20 23:51 ` Ken Brown
2021-02-21 18:04   ` Ken Brown
2021-02-22  9:44     ` Corinna Vinschen
2021-02-22 20:01       ` Ken Brown
2021-02-24  9:06         ` Corinna Vinschen
2021-02-24 15:32           ` Ken Brown
2021-02-24 17:06             ` Corinna Vinschen
2021-02-24 17:27               ` Ken Brown
2021-02-25 13:18                 ` Ken Brown
2021-02-25 17:03                   ` Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e90a3f4f-4aee-061a-f352-bdaa3dbfb9b6@cornell.edu \
    --to=kbrown@cornell.edu \
    --cc=cygwin-developers@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).