cygwin-1.5.4-1 breaks fetchmail on Win9x (Pierre can you comment?)
Christopher Faylor
cgf-rcm@cygwin.com
Tue Oct 7 16:12:00 GMT 2003
On Tue, Oct 07, 2003 at 09:49:14AM -0400, Christopher Faylor wrote:
>On Tue, Oct 07, 2003 at 09:19:05AM -0400, Jason Tishler wrote:
>>On Mon, Oct 06, 2003 at 10:58:37PM -0400, Christopher Faylor wrote:
>>>On Mon, Oct 06, 2003 at 10:13:00PM -0400, Jason Tishler wrote:
>>>>BTW, there seemed to be some gyration regarding this section of
>>>>unlink() during that time period:
>>>
>>>...which might be illuminated by reading the archives, I suspect...
>>
>>I tried searching the archives via Google and Cygwin's mailing list
>>search engine, but came up empty. Would you be willing to enlighten
>>me?
>
>Actually, I am trying not to have to do the search myself. I recall
>that there was a discussion about this with Pierre which caused the
>change to take place. It might have been in the cygwin-developers
>list.
Here is the thread from my personal archives.
cgf
-------------- next part --------------
>From cygwin-developers-return-6502-cgf=redhat.com@nowhere.com Mon Mar 10 10:34:22 2003
Date: Mon, 10 Mar 2003 10:36:26 -0500
From: "Pierre A. Humblet"
MIME-Version: 1.0
To: cygwin-developers
Subject: unlink
Status: RO
Content-Length: 2611
Lines: 59
Chris,
It's a great idea to use FILE_FLAG_DELETE_ON_CLOSE for unlink,
but a few bells rang when I read the code.
1) ntsec
/> mkdir testdir
/> chown 1002:1005 testdir
/> chmod +rwxt testdir
/> touch testdir/testfile
/> chmod 577 testdir/testfile
/> rm -f testdir/testfile
rm: cannot unlink `testdir/testfile': Permission denied
The problem is the DELETE flag in alloc_sd (and sec_acl). Corinna
has put effort into that and when I first saw that code I thought
it was an excellent idea. With the benefit of hindsight I now think it's
counterproductive. POSIX semantics do not support a delete permission
in the Windows sense. Not allowing DELETE in one spot forces us
to *automatically* add it in unlink, so it doesn't help at all from
a Cygwin user point of view.
On the other hand, if a Windows user goes out of Cygwin to remove
DELETE permission on a file, then one could argue that unlink should
fail. Some other action should be required from the user, unlink
shouldn't go out of its way to delete the file.
This is symmetric to refusing to delete a file in a non-writable
directory even though Windows allows it (err on the side of caution).
So I would propose to remove the DELETE code from alloc_sd and sec_acl
and to leave unlink as it is, in that respect.
2) Why is wincap.has_delete_on_close needed?
All Windows versions have delete_on_close capability. This is a snippet
on WinME with a file on a shared directory mounted on Win98.
152 325366 [main] rm 36050599 unlink: _unlink (e:\xx)
6422 331788 [main] rm 36050599 unlink: 1 = CloseHandle (0xCC)
1237 333025 [main] rm 36050599 unlink: CreateFile (FILE_FLAG_DELETE_ON_CLOSE) succeeded
158 333183 [main] rm 36050599 unlink: 0 = unlink (/e/xx)
However Win95 does not have FILE_SHARE_DELETE, so if the file is already opened
the CreateFile fails (and wincap.has_delete_on_close won't be looked up).
Is there reason to believe that a file won't be deleted if the CreateFile succeeds?
It would violate MS semantics. I couldn't create such a case, but I don't have
access to many types of shared drive. Perhaps the test after the CloseHandle could be
if (win32_name.isremote () && GetFileAttributes (win32_name) != INVALID_FILE_ATTRIBUTES)
then handle CreateFile (FILE_FLAG_DELETE_ON_CLOSE) failed
3) I don't understand why there are so many SetFileAttributes after CreateFile
succeeds. If the file will be deleted, its attributes are don't care. If it won't,
then we undo twice what we just did before the CreateFile.
Also it's an easy test to decide if the SetFileAttribute before CreateFile is needed.
Pierre
>From cygwin-developers-return-6505-cgf=redhat.com@nowhere.com Mon Mar 10 10:59:24 2003
Date: Mon, 10 Mar 2003 16:58:55 +0100
From: "Corinna Vinschen"
To: cygwin-developers
Subject: Re: unlink
Message-ID: <20030310155855.GB1193@cygbert.vinschen.de>
Reply-To: cygwin-developers
Mail-Followup-To: "cygwin-developers" <cygwin-developers>
References: <3E6CB0FA.2312CCD0@ieee.org>
In-Reply-To: <3E6CB0FA.2312CCD0@ieee.org>
User-Agent: Mutt/1.4i
Status: RO
Content-Length: 3034
Lines: 79
On Mon, Mar 10, 2003 at 10:36:26AM -0500, Pierre A. Humblet wrote:
> So I would propose to remove the DELETE code from alloc_sd and sec_acl
> and to leave unlink as it is, in that respect.
Like this?
Index: sec_acl.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/sec_acl.cc,v
retrieving revision 1.29
diff -p -u -r1.29 sec_acl.cc
--- sec_acl.cc 9 Mar 2003 20:31:07 -0000 1.29
+++ sec_acl.cc 10 Mar 2003 15:52:35 -0000
@@ -119,19 +119,13 @@ setacl (const char *file, int nentries,
DWORD allow;
/* Owner has more standard rights set. */
if ((aclbufp[i].a_type & ~ACL_DEFAULT) == USER_OBJ)
- allow = (STANDARD_RIGHTS_ALL & ~DELETE)
- | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA;
+ allow = STANDARD_RIGHTS_ALL | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA;
else
allow = STANDARD_RIGHTS_READ | FILE_READ_ATTRIBUTES | FILE_READ_EA;
if (aclbufp[i].a_perm & S_IROTH)
allow |= FILE_GENERIC_READ;
if (aclbufp[i].a_perm & S_IWOTH)
- {
- allow |= STANDARD_RIGHTS_WRITE | FILE_GENERIC_WRITE;
- /* Owner gets DELETE right, too. */
- if ((aclbufp[i].a_type & ~ACL_DEFAULT) == USER_OBJ)
- allow |= DELETE;
- }
+ allow |= STANDARD_RIGHTS_WRITE | FILE_GENERIC_WRITE;
if (aclbufp[i].a_perm & S_IXOTH)
allow |= FILE_GENERIC_EXECUTE;
if ((aclbufp[i].a_perm & (S_IWOTH | S_IXOTH)) == (S_IWOTH | S_IXOTH))
Index: security.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/security.cc,v
retrieving revision 1.139
diff -p -u -r1.139 security.cc
--- security.cc 9 Mar 2003 20:31:07 -0000 1.139
+++ security.cc 10 Mar 2003 15:52:35 -0000
@@ -1644,12 +1644,12 @@ alloc_sd (__uid32_t uid, __gid32_t gid,
int ace_off = 0;
/* Construct allow attribute for owner. */
- DWORD owner_allow = (STANDARD_RIGHTS_ALL & ~DELETE)
+ DWORD owner_allow = STANDARD_RIGHTS_ALL
| FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA;
if (attribute & S_IRUSR)
owner_allow |= FILE_GENERIC_READ;
if (attribute & S_IWUSR)
- owner_allow |= FILE_GENERIC_WRITE | DELETE;
+ owner_allow |= FILE_GENERIC_WRITE;
if (attribute & S_IXUSR)
owner_allow |= FILE_GENERIC_EXECUTE;
if ((attribute & (S_IFDIR | S_IWUSR | S_IXUSR))
> 2) Why is wincap.has_delete_on_close needed?
Hmm, I can't answer this one. Historical reasons? Bad experience?
> 3) I don't understand why there are so many SetFileAttributes after CreateFile
> succeeds. If the file will be deleted, its attributes are don't care. If it won't,
> then we undo twice what we just did before the CreateFile.
> Also it's an easy test to decide if the SetFileAttribute before CreateFile is needed.
Chris and I are through this attribute hell this weekend. To make it
short: If the file is hardlinked more than once, the other links would
miss an attribute after removing this one link.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Developer
Red Hat, Inc.
>From cygwin-developers-return-6508-cgf=redhat.com@nowhere.com Mon Mar 10 11:18:16 2003
Message-ID: <3E6CBB60.FAC2C62D@ieee.org>
Date: Mon, 10 Mar 2003 11:20:48 -0500
From: "Pierre A. Humblet"
To: cygwin-developers
Subject: Re: unlink
References: <3E6CB0FA.2312CCD0@ieee.org> <20030310155855.GB1193@cygbert.vinschen.de>
Status: RO
Content-Length: 1060
Lines: 29
Corinna Vinschen wrote:
>
> On Mon, Mar 10, 2003 at 10:36:26AM -0500, Pierre A. Humblet wrote:
> > So I would propose to remove the DELETE code from alloc_sd and sec_acl
> > and to leave unlink as it is, in that respect.
>
> Like this?
Wow. Yes.
> > 2) Why is wincap.has_delete_on_close needed?
>
> Hmm, I can't answer this one. Historical reasons? Bad experience?
>
> > 3) I don't understand why there are so many SetFileAttributes after CreateFile
> > succeeds. If the file will be deleted, its attributes are don't care. If it won't,
> > then we undo twice what we just did before the CreateFile.
> > Also it's an easy test to decide if the SetFileAttribute before CreateFile is needed.
>
> Chris and I are through this attribute hell this weekend. To make it
> short: If the file is hardlinked more than once, the other links would
> miss an attribute after removing this one link.
I think I see. When a file is hard linked, SetFileAttributes affects *all* links.
Correct?
Still, it's easy to decide if the calls are needed.
Pierre
>From cygwin-developers-return-6507-cgf=redhat.com@nowhere.com Mon Mar 10 11:17:25 2003
Date: Mon, 10 Mar 2003 11:16:28 -0500
From: "Christopher Faylor"
To: cygwin-developers
Subject: Re: unlink
Message-ID: <20030310161628.GG23549@redhat.com>
Reply-To: cygwin-developers
Mail-Followup-To: cygwin-developers
References: <3E6CB0FA.2312CCD0@ieee.org>
Content-Length: 876
Lines: 25
On Mon, Mar 10, 2003 at 10:36:26AM -0500, Pierre A. Humblet wrote:
>Chris,
>
>It's a great idea to use FILE_FLAG_DELETE_ON_CLOSE for unlink,
>but a few bells rang when I read the code.
AFAIK, I didn't add anything here. I just changed the order. unlink
has always used FILE_FLAG_DELETE_ON_CLOSE.
>2) Why is wincap.has_delete_on_close needed?
>
>All Windows versions have delete_on_close capability. This is a snippet
>on WinME with a file on a shared directory mounted on Win98.
It doesn't work quite right on Windows 98. I tested it extensively a
while ago and it seemed unreliable. Maybe it is ok in the current incarnation.
I guess I could remove that test and see if anyone hollers.
>3) I don't understand why there are so many SetFileAttributes after CreateFile
> succeeds. If the file will be deleted, its attributes are don't care.
Think 'hard links'.
cgf
-------------- next part --------------
--
Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple
Problem reports: http://cygwin.com/problems.html
Documentation: http://cygwin.com/docs.html
FAQ: http://cygwin.com/faq/
More information about the Cygwin
mailing list