From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.126.131]) by sourceware.org (Postfix) with ESMTPS id B8FDE3854802 for ; Mon, 22 Feb 2021 09:44:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B8FDE3854802 Received: from calimero.vinschen.de ([24.134.7.25]) by mrelayeu.kundenserver.de (mreue012 [212.227.15.167]) with ESMTPSA (Nemesis) id 1N94FT-1ls0nF0pfz-0165qz for ; Mon, 22 Feb 2021 10:44:26 +0100 Received: by calimero.vinschen.de (Postfix, from userid 500) id A5E49A80CFE; Mon, 22 Feb 2021 10:44:25 +0100 (CET) Date: Mon, 22 Feb 2021 10:44:25 +0100 From: Corinna Vinschen To: cygwin-developers@cygwin.com Subject: Re: fstat and similar methods in fhandler_socket_local and fhandler_socket_unix Message-ID: Reply-To: cygwin-developers@cygwin.com Mail-Followup-To: cygwin-developers@cygwin.com References: <508b12a5-9797-9236-a5b5-d8ff5968ad5d@cornell.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <508b12a5-9797-9236-a5b5-d8ff5968ad5d@cornell.edu> X-Provags-ID: V03:K1:klVMQWRR2B2XcTnrB8wInT9VvG5LfySfIe0OWnauSN+kVxVkjds 0Jtza7hsLxuhDUgU6eX7zfMU+yzOBZgt0FbEyTCQcDq27Cza4wYEGYZOn/Xj2nt2K7OxYUp iCsD1VJAk5+QMnxuhKhtTx8elBbeC0uG7q/iblcS9f3EcADFMb2pMTCuac12hJrHuWb21ZD 7jx2A3oTEfaaROdVw80Zg== X-UI-Out-Filterresults: notjunk:1;V03:K0:91xeQTwo7WE=:GZv9KaUcBJJ+PGVr+AYMvW ifb/iQzc2QYnzs9qU3C0KDoMudLDVl1OCS5RdCFvtT4Gskaf4j71DNlHpj3B9Skld/3U60e62 jzqBDgFFVHfC0/OMTjHwd9k7bfv/3zBjVoyutzVl5QDUqORykdEkVEFOw78QbTxbm0ftr6Qve f8flCF8WlgGrfD9130CPbAKD7vDs+7HHCSaB7AuN8+goRQYGY5zSG/QSXtH2Cz4YeXlpDPHsu TyK6fmmdv0tbbS7joH81uRN24l83oHAd7JebNyqotOaINe/CssvOUscBTxAun08RFWyXuvy1a bWEa7hmEF3LR2m7XPFCT/niNATzP/lToxgrHg8t1N+CvgVPnHot7CyRHmxYlH/tR00z+ZHJjx jkdqpNpnLwgVzcShvT9VkR5ref/5uj0nkc0y80OG3uqF3bktDbcXMKwIKQngDHa8Vkj/tNor5 AgL+0gF7pQ== X-Spam-Status: No, score=-101.1 required=5.0 tests=BAYES_00, GOOD_FROM_CORINNA_CYGWIN, JMQ_SPF_NEUTRAL, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NEUTRAL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: cygwin-developers@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin core component developers mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Feb 2021 09:44:33 -0000 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