public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Florian Weimer <fweimer@redhat.com>
To: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Cc: Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>,
	 Carlos O'Donell <carlos@redhat.com>
Subject: Re: [PATCH 1/2] setjmp: Use BSD sematic as default for setjmp
Date: Wed, 02 Aug 2023 15:17:33 +0200	[thread overview]
Message-ID: <87jzudshw2.fsf@oldenburg.str.redhat.com> (raw)
In-Reply-To: <a0aef4dc-e888-e5f5-717e-f9d1a03794b4@linaro.org> (Adhemerval Zanella Netto's message of "Wed, 2 Aug 2023 09:48:11 -0300")

* Adhemerval Zanella Netto:

> On 02/08/23 09:42, Florian Weimer wrote:
>> * Adhemerval Zanella Netto:
>> 
>>> On 02/08/23 04:59, Florian Weimer wrote:
>>>> * Adhemerval Zanella Netto:
>>>>
>>>>> On 01/08/23 05:35, Florian Weimer wrote:
>>>>>> * Adhemerval Zanella via Libc-alpha:
>>>>>>
>>>>>>> POSIX relaxed the relation of setjmp/longjmp and the signal mask
>>>>>>> save/restore, meaning that setjmp does not require to be routed to
>>>>>>> _setjmp to be standard compliant.
>>>>>>>
>>>>>>> This is done to avoid breakage of SIGABRT handlers, since to fully
>>>>>>> make abort AS-safe, it is required to remove the recurisve lock
>>>>>>> used to unblock SIGABRT prior raised the signal.
>>>>>>>
>>>>>>> Also, it allows caller to actually use setjmp, since from
>>>>>>> 7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally
>>>>>>> routed to _setjmp.
>>>>>>
>>>>>> Doesn't this have non-trivial performance impact?
>>>>>
>>>>> Yes, it is two extra sigprocmask to get/set the signal mask.  This is
>>>>> not *strictly* required, but the SIGABRT on abort generates racy
>>>>> conditions on process creation and.  This patch can be dropped, but it
>>>>> would mean that to get expected semantic for abort handlers will need
>>>>> to use sigsetjmp (..., 1) instead of setjmp.
>>>>
>>>> Sorry, I don't understand?  With the current locking, this change should
>>>> really not be required because the user SIGABRT handler does not run
>>>> with the signal mask changed.
>>>>
>>>
>>> This change is only required to keep the same semantic of setjmp/longjmp
>>> regarding SIGABRT handler, where current code keeps subsequent SIGABRT 
>>> installed with default flags to not keep the signal masked.  Otherwise,
>>> users that callers that handle SIGABRT will need to either a different
>>> sigaction mask that do no change the blocked signals while handling
>>> the signal, call sigprocmask after SIGABRT returns from longjmp, or
>>> use sigsetjmp.
>> 
>> Sorry, I still don't see it.  The new code switches the handler to
>> SIG_DFL atomically and blocks further sigaction calls.  This extends to
>> subprocesses because creating them is inhibited, too.  I think this
>> means that the difference in signal handler masking is not observable.
>
> But this change it no to handle if raise returns, but rather if you have
> a SIGABRT handler that does not (like the fortify tests) installed with
> default flags.  In this case, the kernel will add SIGABRT on the masked
> signal, longjmp will return to setjmp with the SIGBRT handler set mask,
> and the next SIGABRT won't trigger the handler

Ahh, you mean because use removed the signal unblocking from abort?

If the signal is blocked, it is not delivered before it is unblocked.
This means that the handler will not observe it blocked.

But POSIX says this:

| The abort() function shall override blocking or ignoring the SIGABRT
| signal.

It also says:

| The SIGABRT signal shall be sent to the calling process as if by means
| of raise() with the argument SIGABRT.

Strictly speaking, it is impossible to comply with both requirements,
but I think the handler is expected to run even if SIGABRT is blocked.
As far as I understand it, the new code terminates the process in this
case, without ever running the handler.

Thanks,
Florian


  reply	other threads:[~2023-08-02 13:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-31 17:18 [PATCH 0/2] Make abort AS-safe Adhemerval Zanella
2023-07-31 17:18 ` [PATCH 1/2] setjmp: Use BSD sematic as default for setjmp Adhemerval Zanella
2023-08-01  8:35   ` Florian Weimer
2023-08-01 13:51     ` Adhemerval Zanella Netto
2023-08-02  7:59       ` Florian Weimer
2023-08-02 12:32         ` Adhemerval Zanella Netto
2023-08-02 12:42           ` Florian Weimer
2023-08-02 12:48             ` Adhemerval Zanella Netto
2023-08-02 13:17               ` Florian Weimer [this message]
2023-08-02 13:29                 ` Adhemerval Zanella Netto
2023-08-02 14:43                   ` Florian Weimer
2023-08-02 14:56                     ` Adhemerval Zanella Netto
2023-07-31 17:19 ` [PATCH 2/2] stdlib: Make abort AS-safe (BZ 26275) Adhemerval Zanella
2023-08-01  8:10   ` Florian Weimer
2023-08-01 13:52     ` Adhemerval Zanella Netto
2023-08-01  8:26   ` Florian Weimer
2023-08-01 13:57     ` Adhemerval Zanella Netto
2023-08-01 13:44   ` Cristian Rodríguez
2023-08-02  7:57   ` Florian Weimer
2023-08-02 13:08     ` Adhemerval Zanella Netto
2023-08-02 14:44       ` Florian Weimer
2023-08-02 14:48         ` Adhemerval Zanella Netto
2023-08-02 12:38   ` Florian Weimer
2023-08-02 13:08     ` Adhemerval Zanella Netto

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=87jzudshw2.fsf@oldenburg.str.redhat.com \
    --to=fweimer@redhat.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=carlos@redhat.com \
    --cc=libc-alpha@sourceware.org \
    /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).