fstat and similar methods in fhandler_socket_local and fhandler_socket_unix
Corinna Vinschen
corinna-cygwin@cygwin.com
Mon Feb 22 09:44:25 GMT 2021
On Feb 21 13:04, Ken Brown via Cygwin-developers wrote:
> On 2/20/2021 6:51 PM, Ken Brown via Cygwin-developers wrote:
> > On 2/20/2021 6:32 PM, Ken Brown via Cygwin-developers wrote:
> > > 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
> > > [...]
> > > 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.
I see what you mean. This was clearly fat-fingered, probably after
trying to add abstract sockets.
> 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.
> >
> > Maybe the answer in all these cases is just to revert to the
> > fhandler_socket methods when our fhandler_socket_local is not based on a
> > socket file.
>
> I think the following should do the job for fstat:
>
> --- a/winsup/cygwin/fhandler_socket_local.cc
> +++ b/winsup/cygwin/fhandler_socket_local.cc
> @@ -673,11 +673,11 @@ fhandler_socket_local::fcntl (int cmd, intptr_t arg)
> int __reg2
> fhandler_socket_local::fstat (struct stat *buf)
> {
> - int res;
> + if (!pc.handle () && !(get_flags () & O_PATH))
> + /* fstat is not being called on a socket file. */
The comment should probably read "fstat called on a socket"...
> + return fhandler_socket::fstat (buf);
...plus adding a comment right here:
/* stat/lstat on a socket file or fstat on a socket opened w/ O_PATH */
> - if (get_sun_path () && get_sun_path ()[0] == '\0')
> - return fhandler_socket_wsock::fstat (buf);
> - res = fhandler_base::fstat_fs (buf);
> + int res = fhandler_base::fstat_fs (buf);
> if (!res)
> {
> buf->st_mode = (buf->st_mode & ~S_IFMT) | S_IFSOCK;
>
> With this patch, the output of my testcase looks like this:
>
> File type: socket
> I-node number: 6
> dev: 1966080
> rdev: 1966200
> Mode: 140777 (octal)
> Link count: 1
> Ownership: UID=197609 GID=197121
> Preferred I/O block size: 65536 bytes
> File size: 0 bytes
> Blocks allocated: 0
> Last status change: Thu Nov 30 19:00:00 2006
> Last file access: Sun Feb 21 13:03:03 2021
> Last file modification: Sun Feb 21 13:03:03 2021
Might be a good idea to check the output twice in your testcase, adding
a second fstat call after calling bind, to see if it's still the same
after binding the socket to a file.
> If this patch looks right, I'll look at the other system calls too.
Yeah, I think you're right. Thanks!
Corinna
More information about the Cygwin-developers
mailing list