Subject: Bug with Fix: mmap(MAP_PRIVATE) region handling in fork()
From: [email-masked] (Gary Fuehrer)

24 Aug 1997 20:59:12 -0700
Approved: [email-masked]
Distribution: cygnus
Message-ID: <6CC63E2E4FC1D011A2A700609716117A266F11.cygnus.gnu-win32@seawolf>
Mime-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Original-To: "[email-masked]'" <[email-masked]>
X-Priority: 3
X-Mailer: Internet Mail Service (5.0.1458.49)
Original-Sender: [email-masked]

After a fork, a child does not see the same contents in its mmap regions
as the parent does when the region was created with
mmap(,,,MAP_PRIVATE,,) and the parent has written to that regjon prior
to the fork.  This bug is a result of the fork code failing to make the
parent copy into the child process its version of mmap regions that have
the FILE_MAP_COPY bit set.

The following changes to three files (winsup.h, fork.cc, mmap.cc) will
fix this bug.

================
winsup.h
================

Eliminate these declarations:
--------------------------------
/* For mmaps across fork() */

int recreate_mmaps_after_fork(void
*);

void set_child_mmap_ptr(pinfo *);

--------------------------------

================
fork.cc
================

Rename the static "copy" function to the non-static
"copy_memory_to_forkee" function:
--------------------------------
int /* NOTE: not "static" anymore */
copy_memory_to_forkee (HANDLE child, void *low, void *high, int)

:
      rc = copy_memory_to_forkee (pi.hProcess, (char *)proc_data_start,
(char *)proc_data_end, 0);

	:
      rc = copy_memory_to_forkee (pi.hProcess, (char *)proc_bss_start,
(char *)proc_bss_end, 1);
	:
      rc = copy_memory_to_forkee (child->hProcess, u->base, u->ptr, 2);

:
      rc = copy_memory_to_forkee (child->hProcess, &x, u->initial_sp,
3);

--------------------------------

Add these function declarations just before the "cygwin_fork_helper1"
function:
--------------------------------
int copy_mmap_records_to_forkee(pinfo *child);

int
recreate_mmaps_after_fork(void *);

int
copy_copyonwrite_mmaps_to_forkee(HANDLE
hChild);

--------------------------------

Replace the line "set_child_mmap_ptr (child);" with:
--------------------------------
      if (copy_mmap_records_to_forkee(child))

	{

	  small_printf
("fork_helper: copy of mmap_records failed\n");

	  set_errno
(EAGAIN);

	  syscall_printf ("-1 = fork(), copy of mmap_records
failed\n");

	  TerminateProcess (child->hProcess, 1);

u->forkee = 0;

          goto cleanup;

}

--------------------------------

Insert this block of code before the comment "* Now we have started...":
--------------------------------
      if(copy_copyonwrite_mmaps_to_forkee(child->hProcess))

        {

small_printf("fork_helper: copy_copyonwrite_mmaps_to_forkee failed\n");

set_errno (EAGAIN);

	      syscall_printf ("-1 = fork(), copy of
copyonwrite memory failed\n");

	      TerminateProcess (child->hProcess,
1);

              goto cleanup;

}

--------------------------------

Move this block of code so that it comes before the line "/* Tell our
parent we've started."
--------------------------------
      if(recreate_mmaps_after_fork(u->self->mmap_ptr))

        {

small_printf("fork child: recreate_mmaps_after_fork_failed\n");

ExitProcess (1);

        }

--------------------------------

================
mmap.cc
================

Replace the functions "recreate_mmaps_after_fork", and
"set_child_mmap_ptr" with these three functions that follow:
--------------------------------
/*

 * Called by the parent process durring a fork.  At this point, the
child has

 * not yet run.  When it does run and initialize, we want it
to re-establish

 * the same mmap regions that we have.  Because our
heap, where our

 * mmap_records are stored, will not have been copied to
the child at the time

 * it initializes, we can't simply pass it the
"mmapped_areas" pointer.

 * Instead, we must copy the contents of our
"mmapped_areas" into an array of

 * mmap_records where the child process
can read them.  The "child->mmap_ptr"

 * is set to a handle for the
memory where the the child will find the array.

 * After we return, we
will suspend and the child will initialize and call

 *
"recreate_mmaps_after_fork" below.

 */

int
copy_mmap_records_to_forkee(pinfo *child)

{

  /*

   * Count the number
of mmap_record entries are in the "mmapped_areas" map.

   */

  unsigned
count = 0;

  map< int, list<mmap_record> * >::iterator it;

  if
(mmapped_areas != NULL)

  {

    for( it = mmapped_areas->begin(); it !=
mmapped_areas->end(); ++it)

    {

      list<mmap_record> *l =
(*it).second;

      if(l != 0)

      {

list<mmap_record>::iterator li;

        for( li = l->begin(); li !=
l->end(); ++li)

            count++;

      }

    }

  }

  /*

   * Return
with "child->mmap_ptr == NULL"; there are no mmap regions.

   */

  if
(count == 0)

  {

    child->mmap_ptr = NULL;

debug_printf("copy_mmap_records_to_forkee: succeeded\n");

    return 0;

}

  /*

   * Create a file mapping that will be made available to the
child process

   * and will hold an array of all the mmap_records in
"mmapped_areas".

   */

  HANDLE h = CreateFileMapping((HANDLE)-1, NULL,
PAGE_READWRITE, 0,

(count+1)*sizeof(mmap_record), NULL);

  if (h == NULL)

  {

syscall_printf("copy_mmap_records_to_forkee: CreateFileMapping
failed\n");

    return -1;

  }

  mmap_record* pRec =
(mmap_record*)MapViewOfFile(h, FILE_MAP_WRITE, 0, 0, 0);

  if (pRec ==
NULL)

  {

    CloseHandle(h);

syscall_printf("copy_mmap_records_to_forkee: MapViewOfFile failed\n");

return -1;

  }

  /*

   * Now copy all the mmap_records into the mapped
memory array.

   * The count of elements in this array are in the first
record

   * along with some other useful info.

   */

  *pRec =
mmap_record(0, 0, (DWORD)mmapped_areas, count, pRec);

  if
(mmapped_areas != NULL)

  {

    for( it = mmapped_areas->begin(); it !=
mmapped_areas->end(); ++it)

    {

      list<mmap_record> *l =
(*it).second;

      if(l != 0)

      {

list<mmap_record>::iterator li;

        for( li = l->begin(); li !=
l->end(); ++li)

          *(++pRec) = *li;

      }

    }

  }

  /*

   *
Duplicate the handle for use by the child process. The child process

* will use the duplicate handle in recreate_mmaps_after_fork.

   * We
also unmap our view and close our handle, since we're done with it.

*/

  UnmapViewOfFile(pRec - count);

DuplicateHandle(GetCurrentProcess(), h, child->hProcess,

(LPHANDLE)&child->mmap_ptr, FILE_MAP_READ,

                  FALSE,
DUPLICATE_CLOSE_SOURCE);

  debug_printf("copy_mmap_records_to_forkee:
succeeded\n");

  return 0;

}

/*

 * Call to re-create all the file
mappings in a fork()'ed child.  Called from

 * "cygwin_fork_helper1" by
the child durring initialization.  At this point,

 * the parent has
duplicated its .data and .bss, but not its stack or heap.

 * We are
passed a handle to memory via "u->self->mmap_ptr" where an array of

 *
mmap_records can be found.  This was put there by our parent in its
call

 * to "copy_mmap_records_to_forkee".  The first record in the array
contains

 * information like the count of records.  All the HANDLE's in
those records

 * are valid for us (we inherited them), but none of the
mapped areas

 * are in our address space.  We need to iterate through
the array and do the

 * MapViewOfFileEx calls.  Initially, the
"mmapped_areas" pointer is assumed to

 * be NULL, since this process was
just created.  Before returning, both

 * "mmapped_areas" and
"u->self->mmap_ptr" are set to point to the mmap data

 * structure in
the heap.  This pointer is also passed to us in the first

 *
mmap_record.  Remember, that pointer refers to heap memory which the
parent

 * process hasn't yet copied to us.  After we return, we will
suspend while we

 * wait for our parent to copy our heap (so that
"mmaped_areas" is valid).

 * All of our mmap regions, except the ones
that the parent created with

 * FILE_MAP_COPY, will refer to the same
physical memory that the cooresponding

 * region in our parent refers
to.  In the case of FILE_MAP_COPY regions, we

 * will see the contents
of the original file at the time the parent mmapped it.

 * Any changes
made by the parent to these regions will not be seen by us.  So,

 * the
parent will also need to call "copy_copyonwrite_mmaps_to_forkee" below
so

 * that we are in sync with the parent immediately after the fork.

*/

int recreate_mmaps_after_fork(void *param)

{

  if (param != NULL)

{

  HANDLE h = (HANDLE)param;

  mmap_record* pRec =
(mmap_record*)MapViewOfFile(h, FILE_MAP_READ, 0, 0,
                       sizeof(mmap_record));

  if (pRec == NULL)

  {

small_printf("-1 = recreate_mmaps_after_fork(): MapViewOfFile failed "

"with GetLastError = %x\n", GetLastError());

    return -1;

  }

  /*

* Copy the information that was put in the first record

   * (see
set_child_mmap_ptr).

   */

  void* pRecDesired = pRec->get_address();

unsigned count = pRec->get_size();

  mmapped_areas = (map< int,
list<mmap_record> * > *)pRec->get_offset();

  /*

   * We need to have
the array of mmap_records at the address "pRecDesired".

   * Otherwise,
this array may be overlapping one of the mapped regions

   * in the
parent that we are trying to duplicate.  It is unlikely that

   * we are
so lucky as to have "pRec" mapped correctly the first time, so

   *
unmap this view and re-map it using MapViewOfFileEx.

   */

UnmapViewOfFile(pRec);

  if (count > 0)

  {

    pRec =
(mmap_record*)MapViewOfFileEx(h, FILE_MAP_READ, 0, 0, 0, pRecDesired);

if (pRec == NULL)

    {

      small_printf("-1 =
recreate_mmaps_after_fork(): MapViewOfFileEx failed "

"with GetLastError = %x\n", GetLastError());

      return -1;

    }

if (pRec != pRecDesired)

    {

      small_printf("-1 =
recreate_mmaps_after_fork(): MapViewOfFileEx wrong address\n");

return -1;

    }

    /*

     * Copy the "pRec" mmap_record array into
the

     * "mmaped_areas" map of mmap_record lists.

     */

    while
(count-- > 0)

    {

      ++pRec;

      /* Now re-create the
MapViewOfFileEx call */

      void* base =
MapViewOfFileEx(pRec->get_handle(), pRec->get_access(),

0, pRec->get_offset(),

pRec->get_size(), pRec->get_address());

      if(base !=
pRec->get_address())

      {

small_printf("recreate_mmaps_after_fork: base address %x\

fails to match requested address %x\n", base, pRec->get_address());

return -1;

      }

      debug_printf("recreate_mmaps_after_fork: h =
%x, access = %x, offset = %d, \

          size = %d, address = %x\n",
pRec->get_handle(), pRec->get_access(),

          pRec->get_offset(),
pRec->get_size(), pRec->get_address());

    }

UnmapViewOfFile(pRecDesired);

  }

  CloseHandle(h);

 }

  /* Now set
our mmap record in case the child forks. */

  u->self->mmap_ptr =
mmapped_areas;

  debug_printf("recreate_mmaps_after_fork:
succeeded\n");

  return 0;

}

/*

 * Called by the parent process
durring a fork.  At this point,

 * the address space of the child
process has been established;

 * that is, the child has already called
"recreate_mmaps_after_fork".

 * Any mmap region created with
"FILE_MAP_COPY" will need to be

 * copied to the cooresponding memory
addresses in the child process,

 * or else the parent and child may not
see the same contents at those

 * addresses.  They will be different for
those FILE_MAP_COPY regions

 * that the parent has written to (unless
the parent wrote what was

 * there originally).

 */

extern int
copy_memory_to_forkee (HANDLE child, void *low, void *high, int);

int
copy_copyonwrite_mmaps_to_forkee(HANDLE hChild)

{

  /*

   * Iterate
through the array if mmap_records and copy those entries

   * that have
FILE_MAP_COPY set.

   */

  map< int, list<mmap_record> * >::iterator
it;

  if (mmapped_areas != NULL)

  {

    for( it =
mmapped_areas->begin(); it != mmapped_areas->end(); ++it)

    {

list<mmap_record> *l = (*it).second;

      if(l != 0)

      {

list<mmap_record>::iterator li;

          for( li = l->begin(); li !=
l->end(); ++li)

          {

            mmap_record& rec = *li;

char* start = (char*)rec.get_address() + rec.get_offset();

if ((rec.get_access() & FILE_MAP_COPY) &&

!copy_memory_to_forkee(hChild, start, start + rec.get_size(), 0))

{

              syscall_printf("copy_copyonwrite_mmaps_to_forkee:
copy_memory_to_forkee failed\n");

              return -1;

}

	        debug_printf("copy_copyonwrite_mmaps_to_forkee: h = %x,
"

                "access = %x, offset = %d, size = %d, address =
%x\n",

                rec.get_handle(), rec.get_access(),
rec.get_offset(),
                rec.get_size(), rec.get_address());

          }

      }

}

  }

  debug_printf("copy_copyonwrite_mmaps_to_forkee: succeeded\n");

return 0;

}

--------------------------------

-
For help on using this list (especially unsubscribing), send a message to
"[email-masked]" with one line of text: "help".