Consider exposing mmap_is_attached_or_noreserve

Consider exposing mmap_is_attached_or_noreserve

[E. Madison Bray]

Hello,

A very technical request regarding Cygwin internals: In mmap.c there
is a function mmap_is_attached_or_noreserve(void *addr, size_t len)
which is called from Cygwin's exception handler in the case of a
STATUS_ACCESS_VIOLATION.

This is called in case an access violation occurs in memory that was
allocated with Cygwin's mmap() with the MAP_NORESERVE flag, and allows
us to commit the relevant pages when they are accessed.

After a successful call of mmap_is_attached_or_noreserve(), the Cygwin
exception handler returns with ExceptionContinueExecution.
Unfortunately, if the application happens to have a Vectored Continue
Handler registered which happens to do something in the case of
STATUS_ACCESS_VIOLATION (see [1]) there is no obvious way to tell if
we're handling this sort of case.

Normally this isn't too much of a problem: E.g. we could just check
the address that caused the access violation and see if its status is
now MEM_COMMIT (i.e. Cygwin ran its exception handler and all is
good).  However, due to the bug described in [1], if an exception
occurs in code running on a sigaltstack, the Cygwin exception handler
isn't run.

This makes for a tricky to handle use case:  What if some code in a
signal handler function tries to access uncommitted memory in a
MAP_NORESERVE mmap?  It's probably an unusual, undesirable case, and I
haven't personally encountered it *yet*, but I could imagine some
cases where it might happen.

In order to handle such a case it might be nice if
mmap_is_attached_or_noreserve were able to be called by user code,
perhaps as a new cygwin_internal(...) call.  I'd happily provide a
patch, but I fear this might be an X/Y problem that I'm not seeing.

Thanks,
Madison



[1] https://cygwin.com/ml/cygwin/2018-05/msg00333.html

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

Re: Consider exposing mmap_is_attached_or_noreserve

[Corinna Vinschen]

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

On Feb 27 16:38, E. Madison Bray wrote:
> Hello,
> 
> A very technical request regarding Cygwin internals: In mmap.c there
> is a function mmap_is_attached_or_noreserve(void *addr, size_t len)
> which is called from Cygwin's exception handler in the case of a
> STATUS_ACCESS_VIOLATION.
> 
> This is called in case an access violation occurs in memory that was
> allocated with Cygwin's mmap() with the MAP_NORESERVE flag, and allows
> us to commit the relevant pages when they are accessed.
> 
> After a successful call of mmap_is_attached_or_noreserve(), the Cygwin
> exception handler returns with ExceptionContinueExecution.
> Unfortunately, if the application happens to have a Vectored Continue
> Handler registered which happens to do something in the case of
> STATUS_ACCESS_VIOLATION (see [1]) there is no obvious way to tell if
> we're handling this sort of case.
> 
> Normally this isn't too much of a problem: E.g. we could just check
> the address that caused the access violation and see if its status is
> now MEM_COMMIT (i.e. Cygwin ran its exception handler and all is
> good).  However, due to the bug described in [1], if an exception
> occurs in code running on a sigaltstack, the Cygwin exception handler
> isn't run.
> 
> This makes for a tricky to handle use case:  What if some code in a
> signal handler function tries to access uncommitted memory in a
> MAP_NORESERVE mmap?  It's probably an unusual, undesirable case, and I
> haven't personally encountered it *yet*, but I could imagine some
> cases where it might happen.
> 
> In order to handle such a case it might be nice if
> mmap_is_attached_or_noreserve were able to be called by user code,
> perhaps as a new cygwin_internal(...) call.  I'd happily provide a
> patch, but I fear this might be an X/Y problem that I'm not seeing.

Honestly, I'm not overly keen to expose this stuff.  Wouldn't it
make more sense to fix Cygwin's sigaltstack implementation to handle
these cases gracefully?  You're apparently not shy working with
Windows exception handling.  Patches more than welcome!  I'm not
happy not having found a solution to this problem :}


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

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

Re: Consider exposing mmap_is_attached_or_noreserve

[Corinna Vinschen]

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

On Feb 27 17:17, Corinna Vinschen wrote:
> On Feb 27 16:38, E. Madison Bray wrote:
> > Hello,
> > 
> > A very technical request regarding Cygwin internals: In mmap.c there
> > is a function mmap_is_attached_or_noreserve(void *addr, size_t len)
> > which is called from Cygwin's exception handler in the case of a
> > STATUS_ACCESS_VIOLATION.
> > 
> > This is called in case an access violation occurs in memory that was
> > allocated with Cygwin's mmap() with the MAP_NORESERVE flag, and allows
> > us to commit the relevant pages when they are accessed.
> > 
> > After a successful call of mmap_is_attached_or_noreserve(), the Cygwin
> > exception handler returns with ExceptionContinueExecution.
> > Unfortunately, if the application happens to have a Vectored Continue
> > Handler registered which happens to do something in the case of
> > STATUS_ACCESS_VIOLATION (see [1]) there is no obvious way to tell if
> > we're handling this sort of case.
> > 
> > Normally this isn't too much of a problem: E.g. we could just check
> > the address that caused the access violation and see if its status is
> > now MEM_COMMIT (i.e. Cygwin ran its exception handler and all is
> > good).  However, due to the bug described in [1], if an exception
> > occurs in code running on a sigaltstack, the Cygwin exception handler
> > isn't run.
> > 
> > This makes for a tricky to handle use case:  What if some code in a
> > signal handler function tries to access uncommitted memory in a
> > MAP_NORESERVE mmap?  It's probably an unusual, undesirable case, and I
> > haven't personally encountered it *yet*, but I could imagine some
> > cases where it might happen.
> > 
> > In order to handle such a case it might be nice if
> > mmap_is_attached_or_noreserve were able to be called by user code,
> > perhaps as a new cygwin_internal(...) call.  I'd happily provide a
> > patch, but I fear this might be an X/Y problem that I'm not seeing.
> 
> Honestly, I'm not overly keen to expose this stuff.  Wouldn't it
> make more sense to fix Cygwin's sigaltstack implementation to handle
> these cases gracefully?  You're apparently not shy working with
> Windows exception handling.  Patches more than welcome!  I'm not
> happy not having found a solution to this problem :}

Oh, wait!  Maybe there is a simple solution.  Patch 9a5abcc896bd
added a single line

  exception protect;

to the pthread::thread_init_wrapper method.

What if adding the same line to the altstack_wrapper function
would help for altstack as well?

Can you test this?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

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

Re: Consider exposing mmap_is_attached_or_noreserve

[E. Madison Bray]

On Wed, Feb 27, 2019 at 5:17 PM Corinna Vinschen wrote:
>
> On Feb 27 16:38, E. Madison Bray wrote:
> > Hello,
> >
> > A very technical request regarding Cygwin internals: In mmap.c there
> > is a function mmap_is_attached_or_noreserve(void *addr, size_t len)
> > which is called from Cygwin's exception handler in the case of a
> > STATUS_ACCESS_VIOLATION.
> >
> > This is called in case an access violation occurs in memory that was
> > allocated with Cygwin's mmap() with the MAP_NORESERVE flag, and allows
> > us to commit the relevant pages when they are accessed.
> >
> > After a successful call of mmap_is_attached_or_noreserve(), the Cygwin
> > exception handler returns with ExceptionContinueExecution.
> > Unfortunately, if the application happens to have a Vectored Continue
> > Handler registered which happens to do something in the case of
> > STATUS_ACCESS_VIOLATION (see [1]) there is no obvious way to tell if
> > we're handling this sort of case.
> >
> > Normally this isn't too much of a problem: E.g. we could just check
> > the address that caused the access violation and see if its status is
> > now MEM_COMMIT (i.e. Cygwin ran its exception handler and all is
> > good).  However, due to the bug described in [1], if an exception
> > occurs in code running on a sigaltstack, the Cygwin exception handler
> > isn't run.
> >
> > This makes for a tricky to handle use case:  What if some code in a
> > signal handler function tries to access uncommitted memory in a
> > MAP_NORESERVE mmap?  It's probably an unusual, undesirable case, and I
> > haven't personally encountered it *yet*, but I could imagine some
> > cases where it might happen.
> >
> > In order to handle such a case it might be nice if
> > mmap_is_attached_or_noreserve were able to be called by user code,
> > perhaps as a new cygwin_internal(...) call.  I'd happily provide a
> > patch, but I fear this might be an X/Y problem that I'm not seeing.
>
> Honestly, I'm not overly keen to expose this stuff.  Wouldn't it
> make more sense to fix Cygwin's sigaltstack implementation to handle
> these cases gracefully?  You're apparently not shy working with
> Windows exception handling.  Patches more than welcome!  I'm not
> happy not having found a solution to this problem :}

I can theoretically imagine a case where might be a problem totally
outside the context of the altstack issue: e.g. maybe a cygwin
application that has to link with a native Windows DLL that happens to
register some vectored continue handler that does something with
STATUS_ACCESS_VIOLATION exceptions.  Of course, absent a real example
I wouldn't push for it.

I completely agree it would be better to have a solution to the actual  problem.

> Oh, wait!  Maybe there is a simple solution.  Patch 9a5abcc896bd
> added a single line
>
>   exception protect;
>
> to the pthread::thread_init_wrapper method.
>
> What if adding the same line to the altstack_wrapper function
> would help for altstack as well?

You know, I actually noticed this just recently, because I noticed
that pthreads also run on a stack allocated by Cygwin, and I wondered
how exception handling would work in that case.  I think I was looking
at it in the context of the thread last month that resulted in that
fix, but I forgot to ask you if this could work for the altstack issue
as well.

> Can you test this?

I'll give it a try and report back with a patch if it works.

The biggest risk is if a stack overflow happens while running on the
altstack--in this case even the Cygwin exception handling could fail
and the application will just crash.  But for other, less extreme
cases having this would be better than nothing.

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

Re: Consider exposing mmap_is_attached_or_noreserve

[Corinna Vinschen]

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

On Feb 28 12:50, E. Madison Bray wrote:
> On Wed, Feb 27, 2019 at 5:17 PM Corinna Vinschen wrote:
> >
> > On Feb 27 16:38, E. Madison Bray wrote:
> > > In order to handle such a case it might be nice if
> > > mmap_is_attached_or_noreserve were able to be called by user code,
> > > perhaps as a new cygwin_internal(...) call.  I'd happily provide a
> > > patch, but I fear this might be an X/Y problem that I'm not seeing.
> >
> > Honestly, I'm not overly keen to expose this stuff.  Wouldn't it
> > make more sense to fix Cygwin's sigaltstack implementation to handle
> > these cases gracefully?  You're apparently not shy working with
> > Windows exception handling.  Patches more than welcome!  I'm not
> > happy not having found a solution to this problem :}
> 
> I can theoretically imagine a case where might be a problem totally
> outside the context of the altstack issue: e.g. maybe a cygwin
> application that has to link with a native Windows DLL that happens to
> register some vectored continue handler that does something with
> STATUS_ACCESS_VIOLATION exceptions.  Of course, absent a real example
> I wouldn't push for it.

A Cygwin application creating a vectored exception handler is not really
a Cygwin application since that circumvents Cygwin's exeception and
signal handling.  Even if Cygwin's handling isn't entirely foolproof,
that's a slippery slope.

> I completely agree it would be better to have a solution to the actual  problem.
> 
> > Oh, wait!  Maybe there is a simple solution.  Patch 9a5abcc896bd
> > added a single line
> >
> >   exception protect;
> >
> > to the pthread::thread_init_wrapper method.
> >
> > What if adding the same line to the altstack_wrapper function
> > would help for altstack as well?
> 
> You know, I actually noticed this just recently, because I noticed
> that pthreads also run on a stack allocated by Cygwin, and I wondered
> how exception handling would work in that case.  I think I was looking
> at it in the context of the thread last month that resulted in that
> fix, but I forgot to ask you if this could work for the altstack issue
> as well.

No worries, we can't fix all problems at one :}

> > Can you test this?
> 
> I'll give it a try and report back with a patch if it works.

That would be way cool.  I'm looking forward to it.

> The biggest risk is if a stack overflow happens while running on the
> altstack--in this case even the Cygwin exception handling could fail
> and the application will just crash.  But for other, less extreme
> cases having this would be better than nothing.

Stack overflow is nasty.  You're aware of the special concession for
this case in altstack_wrapper(), right?  It's not exacly what happens
on Linux, but it's at least some clean way out.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

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