MIME-Version: 1.0
Date: Wed, 15 Aug 2012 16:54:42 +0400
Message-ID: <CACgPhYpaRft6EfP6uDR34487TJ5-932cHDNyYssLg1nUSdr4Fw@mail.gmail.com>
Subject: Cygwin crashes in kill_pgrp, _pinfo truncation issue.
From: Andrey Khalyavin <halyavin AT google DOT com>
To: cygwin AT cygwin DOT com
Content-Type: text/plain; charset=ISO-8859-1
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com

I finally got a cygwin crash dump from our build bots. It shows, that
cygwin1.dll crashes in kill_pgrp function on line:
	  (pid > 1 && p->pgid != pid) ||
where p is a pointer to _pinfo. This function enumerates all _pinfo's
and executes this line for all of them which pass p->exists() check.
In crash dump p points to _pinfo that has process_state equal to
PID_IN_USE | PID_EXECED. As far as I understand, such _pinfo's
have smaller size and so accessing pgid field leads to crash.
I found a past thread that describes very similar problem:
http://cygwin.com/ml/cygwin/2010-09/msg00390.html

We obviously need to add a check for PID_EXECED in this function. But
I'm worried that we can still have a race condition where
external process can truncate the structure just after our check.

Some ideas how to fix this race condition (if it exists):
1. Use ReadProcessMemory for memory access.
2. Move command line to the end of the structure and extend
PINFO_REDIR_SIZE to include all important fields.
3. Make proper synchronization. Not sure how it should be done in this
case. Since this is a memory mapped file, one may need
to use functions like LockFile.

Andrey Khalyavin

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