public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: Stanislav Kascak <stanislav.kascak@gmail.com>
Cc: cygwin@cygwin.com
Subject: Re: possible problem with memory allocation using calloc/mmap/munmap
Date: Wed, 05 Jun 2019 18:23:00 -0000	[thread overview]
Message-ID: <20190605182315.GZ3437@calimero.vinschen.de> (raw)
In-Reply-To: <CALLhcm5rKTVCD3Hb69CzC-94-86-PRpUofA9OEAy_mci1GnHQA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3995 bytes --]

On Jun  4 18:01, Stanislav Kascak wrote:
> > > > > > > It seems that when mmap() is called with length argument exceeding
> > > > > > > size of file, only memory to fit that file is allocated. munmap()
> > > > > > > however frees the full specified length. [...]
> > > > > > [...]
> > > > > > I know this situation is unsatisfying, but I have no easy workaround
> > > > > > to allow this.  Cygwin could add the anonymous mapping on the next
> > > > > > 64K boundary on 64 bit, but that would result in a hole in the mapping
> > > > > > which seemed like a rather bad idea when porting mmap to 64 bit.
> > > > > >
> > > > > > Ken's also right that munmap is doing the right thing here.  If
> > > > > > anything's wrong, it's mmap's workaround for mappings beyond the file
> > > > > > length.  If only 64 bit would allow 4K-aligned mappings :(
> > > > >
> > > > > Thanks for the answer. It is appreciated.
> > > > > I understand the problem and difficulty to resolve it. Maybe returning
> > > > > an error from mmap (and putting a comment to code for its reason)
> > > > > would be sufficient. mmap caller could just adjust requested
> > > > > allocation size to file size. Without error, caller has no way of
> > > > > knowing memory was not allocated and segfault is then thrown in an
> > > > > unrelated memory segment which makes the root cause hard to track
> > > > > down. But, I do not know all the implication that could result from
> > > > > that, so evaluation of this approach is up to you.
> > > > [...]
> > > > Eventually Cygwin adds another mapping to fullfill the entire mapping
> > > > request:
> > > >
> > > >  |-- file 4K --|-- filler 60K --|-- filler 192K --|
> > > >
> > > > The problem on WOW64 and real 64 bit is that it's impossible to map
> > > > the first filler.  However, this area in the VM will *never* be
> > > > allocated by other application functions due to the allocation
> > > > granularity of 64K!
> > > >
> > > > So my workaround for 64 bit and WOW64 is to just skip allocating the
> > > > first filler:
> > > >
> > > >  |-- file 4K --|-- THE VOID 60K --|-- filler 192K --|
> > > >
> > > > The advantage is now that the following munmap of 256K will only
> > > > unmap the map for the file and the filler, but not the region you
> > > > calloced before, which formerly was accidentally mapped to the
> > > > filler region.  This just can't happen anymore now.
> > > >
> > > > Would that be feasible?  If so I can push my patch and create a
> > > > developer snapshot for testing.
> > >
> > > Two questions arise when I'm thinking about workaround solution:
> > > - what happens if caller tries to write to |-- THE VOID 60K --|. Since
> > > this is unallocated, would there be a segfault?
> >
> > Accessing the VOID would raise SIGSEGV, while accessing the filler
> > raises SIGBUS.  The latter is also used to implement MAP_NORESERVE,
> > which the VOID can't support.
> 
> I played around a bit and I can confirm it would be consistent with
> current behavior:
> memwrite <0 - filesize) - no error, written to file
> memwrite <filesize - 4k) - no error, no file change
> memwrite <4k, 64k) - SIGSEGV
> memwrite <64k, mmap alloc size) - SIGSEGV or another mem alloc
> overwrite (depending on whether there is another allocation)
> With workaround last line would be fixed to SIGBUS (along with proper
> allocation length). I believe this is completely OK.
> 
> >
> > > - is it possible that some subsequent mem alloc request would return
> > > region from |-- THE VOID 60K --| which could again cause segfault
> > > after munmap?
> >
> > No, as stated above.  Allocations are restricted to Windows' 64K
> > allocation granularity.
> 
> I apologize. I missed that sentence. So, your workaround seems fine.

Please try the latest snapshot from https://cygwin.com/snapshots/
Just replacing the Cygwin DLL is sufficient.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2019-06-05 18:23 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-03 11:33 Stanislav Kascak
2019-05-20 22:26 ` Ken Brown
2019-06-03 11:55 ` Corinna Vinschen
2019-06-04  9:38   ` Stanislav Kascak
2019-06-04 13:18     ` Corinna Vinschen
2019-06-04 13:49       ` Stanislav Kascak
2019-06-04 14:49         ` Corinna Vinschen
2019-06-04 16:02           ` Stanislav Kascak
2019-06-05 18:23             ` Corinna Vinschen [this message]
2019-06-06 13:14               ` Stanislav Kascak
2019-06-07  7:27                 ` Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190605182315.GZ3437@calimero.vinschen.de \
    --to=corinna-cygwin@cygwin.com \
    --cc=cygwin@cygwin.com \
    --cc=stanislav.kascak@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).