delorie.com/archives/browse.cgi   search  
Mail Archives: cygwin/2010/09/14/04:13:06

X-Recipient: archive-cygwin AT delorie DOT com
X-Spam-Check-By: sourceware.org
Date: Tue, 14 Sep 2010 10:12:25 +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: <20100914081225.GE16534@calimero.vinschen.de>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
References: <3C031C390CBF1E4A8CE1F74DE7ECAF3A140684F0B0 AT MBX8 DOT EXCHPROD DOT USA DOT NET> <AANLkTimpBCKzUioEarGiKMQbTjN+6s9iDYm_zzZ2Lxr9 AT mail DOT gmail DOT com> <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>
MIME-Version: 1.0
In-Reply-To: <3C031C390CBF1E4A8CE1F74DE7ECAF3A15945FDD78@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
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 13 19:47, John Carey wrote:
> On Sep 11 03:30, Corinna Vinschen:
> > Given that, wouldn't it be potentially possible to override the content
> > of curCwd?  The problem is probably (just as in my old Cygwin code up to
> > 1.7.5) to make sure that this is thread safe.  Probably we would still
> > need CwdCS.
> 
> Unfortunately, it looks as if Win32 API functions guarantee to each
> other that they will not modify the path in a VistaCwd whose reference
> count is >= 2.  (Though it looks like they may update OldDismountCount
> under a CwdCS lock.)
> 
> Consider RtlGetCurrentDirectory_U().  It shares a (non-exported)
> subroutine with other Win32 API functions that acquires a counted
> reference to the current VistaCwd instance under the protection of
> a lock on CwdCS.  (This subroutine also does some kind of freshness
> check against DismountCount.)  But that subroutine releases its
> lock before returning, and RtlGetCurrentDirectory_U() then uses the
> pathname in the returned VistaCwd instance *without* holding a lock.
> Specifically, it copies that pathname to a memory buffer passed in by
> its own caller.  Pathname copies are not atomic.  Thus, unless there
> is a bug in RtlGetCurrentDirectory_U(), it has some guarantee that
> other threads will not modify the pathname in its VistaCwd instance,
> though they are free to allocate a new VistaCwd instance and assign
> its address to the global Cwd pointer (so long as they lock CwdCS).
> 
> Presumably the point of the Vista++ CWD mechanism is to reduce
> contention on the CWD lock.  Threads acquire that lock for very
> short periods of time.  They appear to avoid making system calls
> or even allocating memory while holding it.  Also, the CWD lock
> is no longer the PEB lock, so CWD usage does not serialize with
> other PEB-related activities.
> 
> 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?

> > >        << "  <== critical section" << endl;
> > >   cout << showbase << hex << (size_t)Cwd
> > >        << "  <== Vista++ CWD struct pointer" << endl;
> > 
> > Is there any connection between these two addresses, like, say, they
> > are side by side?
> 
> Not that I have been able to find.  The gap between Cwd and CwdCS
> is always positive, but is large and varies among OS versions:

Bummer.  I was hoping that these global variables could be identified
as part of some other global structure for which there is an easier way
to find it's address.

> > Can't we find Cwd by scanning the .data segment of ntdll.h for the
> > address we inferred from the Params.CurrentDirectoryName.Buffer value?
> 
> Nice; that might work.  But there would need to be some kind of rule
> to pick a winner among multiple matching words, in case by coincidence
> some other non-pointer word just happens to have the same bit pattern.
> I can see two alternatives for the multiple-match case:
> 
>   1. Code scanning, as suggested earlier.  There would need to be a
>   unit test of the code scanner itself so that incompatible changes to
>   ntdll.dll could be detected deterministically, instead of only after
>   a multiple-match coincidence.
> 
>   2. Call SetCurrentDirectory() and recheck the previously-matching
>   addresses to see which one matches the new value.  Place some limit
>   (like 4) on the number of such retries, in case some new version of
>   ntdll.dll intentionally duplicates the pointer every time.
>   (Not sure what to do in that case; fall back to code scanning?)

Sounds like code scanning is the better choice then.  Your code scanner
isn't overly complicated anyway, and it has the advantage of being
rather fast.


Corinna

-- 
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

- Raw text -


  webmaster     delorie software   privacy  
  Copyright © 2019   by DJ Delorie     Updated Jul 2019