X-Recipient: archive-cygwin AT delorie DOT com
X-Spam-Check-By: sourceware.org
Date: Tue, 14 Sep 2010 20:59:32 +0200
From: Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: 1.7.5: Occasional failure of CreatePipe or signal handing due to 	thread-unsafe code in cwdstuff::set
Message-ID: <20100914185932.GJ15121@calimero.vinschen.de>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
References: <20100812081151 DOT GT14202 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA702A AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100903073740 DOT GA1749 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA702D AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100904092626 DOT GE16534 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A158EDA704C AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100911103010 DOT GM16534 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD78 AT MBX8 DOT EXCHPROD DOT USA DOT NET> <20100914081225 DOT GE16534 AT calimero DOT vinschen DOT de> <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD80 AT MBX8 DOT EXCHPROD DOT USA DOT NET>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD80@MBX8.EXCHPROD.USA.NET>
User-Agent: Mutt/1.5.20 (2009-06-14)
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Precedence: bulk
List-Id: <cygwin.cygwin.com>
List-Unsubscribe: <mailto:cygwin-unsubscribe-archive-cygwin=delorie DOT com AT cygwin DOT com>
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com

On Sep 14 17:39, John Carey wrote:
> On Sep 14 01:12, Corinna Vinschen wrote:
> > On Sep 13 19:47, John Carey wrote:
> > > On Sep 11 03:30, Corinna Vinschen:
> > > Anyway, it looks to me as if overwriting the path in an existing
> > > VistaCwd instance would confuse RtlGetCurrentDirectory_U() and
> > > probably other Win32 API functions.  (It may be that the same is
> > > true for the handle member; I have not tried to find that out.)
> > > Cygwin should probably allocate a new VistaCwd instance,
> > > as does the Win32 SetCurrentDirectory() implementation.
> > 
> > Hmm, I'm still wondering.  In theory we don't have to replace the
> > directory name.  We just have to InterlockedExchange the handle, since
> > we only need to replace the original handle which does not allow
> > sharing-for-delete with our own handle which does.
> > 
> > All other border cases (virtual dir, directory with restricted rights)
> > can be rightfully handled by setting the Win32 CWD to \\?\PIPE\ again.
> > 
> > That's at least worth a try, isn't it?
> 
> Oh, I had thought that you wanted it for long paths
> and directories with restricted rights, too.

That would be nice, but to me it looks like the Win32 API is just too
restricted for that, so that it won't make much sense.  The native API
can't make any assumption that it would work in such a scenario.

Most Win32 calls will still not work in a restricted rights CWD, because
they neglect to set the FILE_OPEN_FOR_BACKUP_INTENT flag.  And most Apps
don't set the FILE_FLAG_BACKUP_SEMANTICS flag incalls to CreateFile.

As for long paths, there's some chance that the internal calls creating
an absolute path from a relative path fail or crash if the CWD is longer
than MAX_PATH.  What MSDN has to say about the maximum path length of
relative paths(*) is not very promising, at least.  Also, even if
Vista++ allows to create VistaCwd entries with a longer path, this would
probably break starting native child processes due to the MAX_PATH
restriction for the CWD in calls to CreateProcess.

> There's one other issue: the new directory will limit deletion
> very briefly, until replaced.  I don't know if that will bother
> people as much as the current situation.

True.  Implementing a full replacement for SetCurrentDirectory as in
your PoC is still an option.  However, I can't do that anymore since
I'm tainted by reading your code.  If you would contemplate to sign
a copyright assignment(**), we could create a more thorough solution
with code scanning and all that in the long run.

In the meantime, I could apply my patch and we can try how well it
works, hacked as it is.


Corinna


(*) http://msdn.microsoft.com/en-us/library/aa365247%28VS.85%29.aspx

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple