On Mar 14 12:56, Takashi Yano via Cygwin wrote:
> On Fri, 14 Mar 2025 08:12:36 +0900
> Takashi Yano wrote:
> > On Thu, 13 Mar 2025 23:46:49 +0100
> > Corinna Vinschen wrote:
> > > I have a slighty changed version. This one treats anything other
> > > than 0, 1 or 2 new addresses on the stack as bug. I really made
> > > an effort trying to come up with a situation where the signal
> > > stack underflows, but I just couldn't. If I'm missing something,
> > > please explain how this may happen.
> > >
> > > Apart from that, I attached my patch proposal.
> >
> > I think the following is the right thing. This version pulls return
> > addresses completely (not only one) before calling signal handler.
> > I think, stackptr - orig_stackptr can be larger than 2 when
> > user code
> > signal handler 1
> > signal handler 2
> > signal handler 3
> > signal handler 4
> > ret
> > ret
> > ret
> > HERE <= stackptr - orig_stackptr == 3
> > ret
> > Is this right?
>
> No, I was wrong. Every time when call_signal_handler() is
> called, the _cygtls::stack is pulled, so, it always becomes
> empty. Therefore, stackptr - orig_stackptr is never more
> than two.
>
> So, _cygtls::stack needs only two spaces maximum. Please
> look attached v2 patch. Do I overlook something?
I don't think so. I was mulling in circles over this tonight
(don't ask me how I slept!) and came to the same conclusion.
But here's the problem:
I'm simply not 100% sure.
What concerns me is that stackptr points beyond stack if the stack
is full (i.e., sigdelayed + return address).
That was what happened before I applied a942476236b5: stackptr was
incremented until it pointed at _cygtls::initialized, and eventually it
overwrote it. Fortunately, that stopped further incrementing due to the
isinitialized() test.
So, if there *is* a twisted situation which results in pushing another
return address onto the stack, a stack size of 2 would again result in
initialized being overwritten. So I wonder if we should keep kind of
an airbag for an unusual situation. Plus trying to keep stackptr inside
stack even if it's full. So that stackptr never grows into initialized:
#define TLS_STACK_SIZE 5
and
void push (__tlsstack_t addr)
{
if (stackptr < (__tlsstack_t *) &initialized)
*stackptr++ = (__tlsstack_t) addr;
}
What do you think?
Corinna
--
Problem reports: https://cygwin.com/problems.html
FAQ: https://cygwin.com/faq/
Documentation: https://cygwin.com/docs.html
Unsubscribe info: https://cygwin.com/ml/#unsubscribe-simple
