mailing list of musl libc
 help / color / mirror / code / Atom feed
* [musl] Do we need to enhance robustness in the signal mask?
@ 2024-12-03  3:06 JinCheng Li
  2024-12-03 14:44 ` Rich Felker
  0 siblings, 1 reply; 11+ messages in thread
From: JinCheng Li @ 2024-12-03  3:06 UTC (permalink / raw)
  To: musl

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

Hi


The signal 32(SIGTIMER) will only used in musl timer and block for the SIGEV_THREAD. I think it can be seen as a special internal signal like 34(SIGSYNCCALL).

But in sigprocmask and pthread_sigmask, musl lacks protection for these internal signals. Users can modify the shielding status of such signals which may cause problems with signal response. For example, I can cancel the masking of signal 32 for the SIGEV_THREAD, as aresult, the timing thread can exits abnormally.

But in android, there is a function "filter_reserved_signals" used in sigprocmask to protect the internal signals state from being modified.


Considering that upper-layer users may not know the internl signals such as 32 has been blocked, should we add a default block for these signals in pthread_sigmask like android to enhance the signal robustness? Or is there other considerations?


Best,
JinCheng


[-- Attachment #2: Type: text/html, Size: 3670 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-03  3:06 [musl] Do we need to enhance robustness in the signal mask? JinCheng Li
@ 2024-12-03 14:44 ` Rich Felker
  2024-12-03 18:18   ` Markus Wichmann
  0 siblings, 1 reply; 11+ messages in thread
From: Rich Felker @ 2024-12-03 14:44 UTC (permalink / raw)
  To: JinCheng Li; +Cc: musl

On Tue, Dec 03, 2024 at 03:06:39AM +0000, JinCheng Li wrote:
> Hi
> 
> 
> The signal 32(SIGTIMER) will only used in musl timer and block for
> the SIGEV_THREAD. I think it can be seen as a special internal
> signal like 34(SIGSYNCCALL).
> 
> But in sigprocmask and pthread_sigmask, musl lacks protection for
> these internal signals. Users can modify the shielding status of
> such signals which may cause problems with signal response. For
> example, I can cancel the masking of signal 32 for the SIGEV_THREAD,
> as aresult, the timing thread can exits abnormally.

There are only a limited number of ways you can get a sigset_t whose
use with sigprocmask etc. doesn't have undefined behavior. Either you
initialize it with sigemptyset or sigfillset then modify with
sigaddset/sigdelset, or you get it from some interface that reads back
a sigset_t.

In musl, we disallow creation of invalid signal sets via the sig*set
function, rather than attempting to alter the set when it's installed
via sigprocmask. One reason it's done this way is that returning from
a signal handler also installs a sigset_t, and we don't have control
over that code path (and can't unless we opt to wrap signal handling).

> But in android, there is a function "filter_reserved_signals" used
> in sigprocmask to protect the internal signals state from being
> modified.
> 
> Considering that upper-layer users may not know the internl signals
> such as 32 has been blocked, should we add a default block for these
> signals in pthread_sigmask like android to enhance the signal
> robustness? Or is there other considerations?

Can you clarify what situation you're concerned about?

FWIW, SIGTIMER is not of special concern. It will never be generated
except for timer threads, and these run with it explicitly blocked.
This is very different from the situation for SIGCANCEL or SIGSYNCCALL
where it's critical for arbitrary threads to be able to receive them.
Also, at some point SIGTIMER is slated to be removed in favor of using
clock_nanosleep for SIGEV_THREAD timers rather than kernelspace
timers unless a strong reason not to do that is discovered.

Rich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-03 14:44 ` Rich Felker
@ 2024-12-03 18:18   ` Markus Wichmann
  2024-12-04  7:00     ` JinCheng Li
  2024-12-05  5:56     ` Rich Felker
  0 siblings, 2 replies; 11+ messages in thread
From: Markus Wichmann @ 2024-12-03 18:18 UTC (permalink / raw)
  To: musl

Am Tue, Dec 03, 2024 at 09:44:10AM -0500 schrieb Rich Felker:
> There are only a limited number of ways you can get a sigset_t whose
> use with sigprocmask etc. doesn't have undefined behavior. Either you
> initialize it with sigemptyset or sigfillset then modify with
> sigaddset/sigdelset, or you get it from some interface that reads back
> a sigset_t.
>

You can get a valid sigset_t lacking SIGTIMER (unconditionally) with
pthread_sigmask(), and then set it, e.g like

sigset_t ss;
pthread_sigmask(SIG_SETMASK, 0, &ss);
pthread_sigmask(SIG_SETMASK, &ss, 0);

Honestly, this is close to the normal way to use pthread_sigmask(). The
same holds if the first all is used to block some other signal for some
reason and the second one then is supposed to restore the signal mask.

And you can do that in a timer notification function, where the signal
is supposed to be blocked. The timer thread will continue to execute
with the signal unblocked. And since the signal disposition is set to
SIG_DFL, and the default handling for RT signals is to terminate, the
next timer expiration will then kill the process.

> FWIW, SIGTIMER is not of special concern. It will never be generated
> except for timer threads, and these run with it explicitly blocked.
> This is very different from the situation for SIGCANCEL or SIGSYNCCALL
> where it's critical for arbitrary threads to be able to receive them.
> Also, at some point SIGTIMER is slated to be removed in favor of using
> clock_nanosleep for SIGEV_THREAD timers rather than kernelspace
> timers unless a strong reason not to do that is discovered.

I tried looking into doing that at some point and just gave up, because
of the overrun accounting. If I understand correctly, every timer
expiration between the notification being generated and being delivered
increases the overrun counter, and for each notification it is supposed
to be latched and then reset. So for SIGEV_THREAD, that would be every
expiration between the call and the return. And for SIGEV_SIGNAL, that
would be every expiration between the signal being sent and being
handled.

All of which was too complicated for me to wrap my head around, but
maybe you have more luck.

Ciao,
Markus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-03 18:18   ` Markus Wichmann
@ 2024-12-04  7:00     ` JinCheng Li
  2024-12-05  2:28       ` Rich Felker
  2024-12-05  5:56     ` Rich Felker
  1 sibling, 1 reply; 11+ messages in thread
From: JinCheng Li @ 2024-12-04  7:00 UTC (permalink / raw)
  To: musl; +Cc: Markus Wichmann


[-- Attachment #1.1: Type: text/plain, Size: 4685 bytes --]

Hi


> Can you clarify what situation you're concerned about?

The specified scenario is as Markus said, I can just use pthread_sigmask(SIG_SETMASK, 0, &ss) to unblock all signals in the timer notification function. And due to the SIG_DFL handler of the SIGTIMER signal, the next timer expiration will then kill the process. I provided a demo in the email.

What really happened was that I used the interface of a third-party library in the notification function, but the interface unblocked SIGTIMER as mentioned above, and I couldn't prevent this from happening. Sadly, the third-party library is necessary and works properly in an Android environment.

In musl,  I can easily get a sigset with 32, 33, 34 unblocked such as sigfillset(&set) or pthread_sigmask(0, NULL, &set) in demo.  So users can also easily unblock 32, 33, 34 through this sigset, even they don't realize they're unblocking these internal signals.

Since I can set the mask of the signal through the normal sigpromask and sig*set interfaces, it's possible that the setting of the internal signal may be affected and unexpected situations may occur, especially when using the capabilities of some third-party libraries which I can't control.

So my suggestion is whether musl should be like android, protect the settings of the internal signal in some situations to enhance robustness.

> You can get a valid sigset_t lacking SIGTIMER (unconditionally) with
> pthread_sigmask(), and then set it, e.g like
> sigset_t ss;
> pthread_sigmask(SIG_SETMASK, 0, &ss);
> pthread_sigmask(SIG_SETMASK, &ss, 0);
> Honestly, this is close to the normal way to use pthread_sigmask(). The
> same holds if the first all is used to block some other signal for some
> reason and the second one then is supposed to restore the signal mask.
> And you can do that in a timer notification function, where the signal
> is supposed to be blocked. The timer thread will continue to execute
> with the signal unblocked. And since the signal disposition is set to
> SIG_DFL, and the default handling for RT signals is to terminate, the
> next timer expiration will then kill the process.


Best,
JinCheng


________________________________
From: Markus Wichmann <nullplan@gmx.net>
Sent: Wednesday, December 4, 2024 2:18
To: musl@lists.openwall.com <musl@lists.openwall.com>
Subject: Re: [musl] Do we need to enhance robustness in the signal mask?

Am Tue, Dec 03, 2024 at 09:44:10AM -0500 schrieb Rich Felker:
> There are only a limited number of ways you can get a sigset_t whose
> use with sigprocmask etc. doesn't have undefined behavior. Either you
> initialize it with sigemptyset or sigfillset then modify with
> sigaddset/sigdelset, or you get it from some interface that reads back
> a sigset_t.
>

You can get a valid sigset_t lacking SIGTIMER (unconditionally) with
pthread_sigmask(), and then set it, e.g like

sigset_t ss;
pthread_sigmask(SIG_SETMASK, 0, &ss);
pthread_sigmask(SIG_SETMASK, &ss, 0);

Honestly, this is close to the normal way to use pthread_sigmask(). The
same holds if the first all is used to block some other signal for some
reason and the second one then is supposed to restore the signal mask.

And you can do that in a timer notification function, where the signal
is supposed to be blocked. The timer thread will continue to execute
with the signal unblocked. And since the signal disposition is set to
SIG_DFL, and the default handling for RT signals is to terminate, the
next timer expiration will then kill the process.

> FWIW, SIGTIMER is not of special concern. It will never be generated
> except for timer threads, and these run with it explicitly blocked.
> This is very different from the situation for SIGCANCEL or SIGSYNCCALL
> where it's critical for arbitrary threads to be able to receive them.
> Also, at some point SIGTIMER is slated to be removed in favor of using
> clock_nanosleep for SIGEV_THREAD timers rather than kernelspace
> timers unless a strong reason not to do that is discovered.

I tried looking into doing that at some point and just gave up, because
of the overrun accounting. If I understand correctly, every timer
expiration between the notification being generated and being delivered
increases the overrun counter, and for each notification it is supposed
to be latched and then reset. So for SIGEV_THREAD, that would be every
expiration between the call and the return. And for SIGEV_SIGNAL, that
would be every expiration between the signal being sent and being
handled.

All of which was too complicated for me to wrap my head around, but
maybe you have more luck.

Ciao,
Markus

[-- Attachment #1.2: Type: text/html, Size: 9579 bytes --]

[-- Attachment #2: demo.txt --]
[-- Type: text/plain, Size: 802 bytes --]

static volatile int done;
static void fn(union sigval v)
{
  sigset_t ss;
  printf("#1 mask all\n");
  sigfillset(&ss);  // 0xfffffffc7ffffffful
  sigprocmask(SIG_SETMASK, &ss, NULL); // unblock 32 33 34
  sleep(1);
  printf("#2 save & restore\n");
  sigprocmask(0, NULL, &ss); // ss &= ~0x380000000ULL
  sigprocmask(SIG_SETMASK, &ss, NULL); // unblock 32 33 34
  sleep(1);
  done = 1;
}

int main()
{
  timer_t t;
  struct sigevent sev = {
    .sigev_notify = SIGEV_THREAD,
    .sigev_notify_function = fn, 
  };
  struct itimerspec its = {
    .it_value = {0, 100000000},
    .it_interval = {0, 100000000}, 
  };
  timer_create(CLOCK_MONOTONIC, &sev, &t);
  timer_settime(t, 0, &its, NULL);
  while (done = 0) {
    usleep(10000);
  }
  timer_delete(t);
  return 0;
}

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-04  7:00     ` JinCheng Li
@ 2024-12-05  2:28       ` Rich Felker
  0 siblings, 0 replies; 11+ messages in thread
From: Rich Felker @ 2024-12-05  2:28 UTC (permalink / raw)
  To: JinCheng Li; +Cc: musl, Markus Wichmann

On Wed, Dec 04, 2024 at 07:00:02AM +0000, JinCheng Li wrote:
> Hi
> 
> 
> > Can you clarify what situation you're concerned about?
> 
> The specified scenario is as Markus said, I can just use
> pthread_sigmask(SIG_SETMASK, 0, &ss) to unblock all signals in the
> timer notification function. And due to the SIG_DFL handler of the
> SIGTIMER signal, the next timer expiration will then kill the
> process. I provided a demo in the email.

OK, indeed this is a problem. But the Android approach to protecting
from it doesn't fully solve the problem, because there's a hidden
setprocmask in every sigreturn that we can't easily (not without
wrapping signal handling) intercept and modify the mask for.

There are solutions I think that involve always masking SIGTIMER in
sigsets the way the other internal signals are always unmasked, but
this incurs special logic for SIG_UNBLOCK.

Just getting rid of SIGTIMER would be ideal. Let me review how
difficult this would be.

Rich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-03 18:18   ` Markus Wichmann
  2024-12-04  7:00     ` JinCheng Li
@ 2024-12-05  5:56     ` Rich Felker
  2024-12-05  6:28       ` Rich Felker
  2024-12-05 18:25       ` Markus Wichmann
  1 sibling, 2 replies; 11+ messages in thread
From: Rich Felker @ 2024-12-05  5:56 UTC (permalink / raw)
  To: Markus Wichmann; +Cc: musl

On Tue, Dec 03, 2024 at 07:18:10PM +0100, Markus Wichmann wrote:
> Am Tue, Dec 03, 2024 at 09:44:10AM -0500 schrieb Rich Felker:
> > There are only a limited number of ways you can get a sigset_t whose
> > use with sigprocmask etc. doesn't have undefined behavior. Either you
> > initialize it with sigemptyset or sigfillset then modify with
> > sigaddset/sigdelset, or you get it from some interface that reads back
> > a sigset_t.
> >
> 
> You can get a valid sigset_t lacking SIGTIMER (unconditionally) with
> pthread_sigmask(), and then set it, e.g like
> 
> sigset_t ss;
> pthread_sigmask(SIG_SETMASK, 0, &ss);
> pthread_sigmask(SIG_SETMASK, &ss, 0);
> 
> Honestly, this is close to the normal way to use pthread_sigmask(). The
> same holds if the first all is used to block some other signal for some
> reason and the second one then is supposed to restore the signal mask.
> 
> And you can do that in a timer notification function, where the signal
> is supposed to be blocked. The timer thread will continue to execute
> with the signal unblocked. And since the signal disposition is set to
> SIG_DFL, and the default handling for RT signals is to terminate, the
> next timer expiration will then kill the process.
> 
> > FWIW, SIGTIMER is not of special concern. It will never be generated
> > except for timer threads, and these run with it explicitly blocked.
> > This is very different from the situation for SIGCANCEL or SIGSYNCCALL
> > where it's critical for arbitrary threads to be able to receive them.
> > Also, at some point SIGTIMER is slated to be removed in favor of using
> > clock_nanosleep for SIGEV_THREAD timers rather than kernelspace
> > timers unless a strong reason not to do that is discovered.
> 
> I tried looking into doing that at some point and just gave up, because
> of the overrun accounting. If I understand correctly, every timer
> expiration between the notification being generated and being delivered
> increases the overrun counter, and for each notification it is supposed
> to be latched and then reset. So for SIGEV_THREAD, that would be every
> expiration between the call and the return. And for SIGEV_SIGNAL, that
> would be every expiration between the signal being sent and being
> handled.
> 
> All of which was too complicated for me to wrap my head around, but
> maybe you have more luck.

Counting the number of overruns is conceptually just a division:
taking the difference between the current time and last notification
time and dividing by the interval length. There is some complexity to
handler with making changes to the interval length (special case:
stopping and starting the timer) but it's not that hard a problem.

Note that sor SIGEV_SIGNAL we should ocntinue to use kernel timer
objects. There's no other way without making gratuitous threads, and
making the overrun accounting work in AS-safe manner is gratuitously
difficult. It's only SIGEV_THREAD that would benefit from dropping
kernel timers.

One short-term fix that might be worth exploring is adding back a
signal handler for SIGTIMER so it doesn't kill the process. The
handler would just increment an "extra overruns" counter for the
thread. It could only run during execution of the function, if the
function unblocked the signal, since we would re-block the signal each
time before the next sigwaitinfo.

Rich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-05  5:56     ` Rich Felker
@ 2024-12-05  6:28       ` Rich Felker
  2024-12-05 18:25       ` Markus Wichmann
  1 sibling, 0 replies; 11+ messages in thread
From: Rich Felker @ 2024-12-05  6:28 UTC (permalink / raw)
  To: Markus Wichmann; +Cc: musl

On Thu, Dec 05, 2024 at 12:56:44AM -0500, Rich Felker wrote:
> On Tue, Dec 03, 2024 at 07:18:10PM +0100, Markus Wichmann wrote:
> > Am Tue, Dec 03, 2024 at 09:44:10AM -0500 schrieb Rich Felker:
> > > There are only a limited number of ways you can get a sigset_t whose
> > > use with sigprocmask etc. doesn't have undefined behavior. Either you
> > > initialize it with sigemptyset or sigfillset then modify with
> > > sigaddset/sigdelset, or you get it from some interface that reads back
> > > a sigset_t.
> > >
> > 
> > You can get a valid sigset_t lacking SIGTIMER (unconditionally) with
> > pthread_sigmask(), and then set it, e.g like
> > 
> > sigset_t ss;
> > pthread_sigmask(SIG_SETMASK, 0, &ss);
> > pthread_sigmask(SIG_SETMASK, &ss, 0);
> > 
> > Honestly, this is close to the normal way to use pthread_sigmask(). The
> > same holds if the first all is used to block some other signal for some
> > reason and the second one then is supposed to restore the signal mask.
> > 
> > And you can do that in a timer notification function, where the signal
> > is supposed to be blocked. The timer thread will continue to execute
> > with the signal unblocked. And since the signal disposition is set to
> > SIG_DFL, and the default handling for RT signals is to terminate, the
> > next timer expiration will then kill the process.
> > 
> > > FWIW, SIGTIMER is not of special concern. It will never be generated
> > > except for timer threads, and these run with it explicitly blocked.
> > > This is very different from the situation for SIGCANCEL or SIGSYNCCALL
> > > where it's critical for arbitrary threads to be able to receive them.
> > > Also, at some point SIGTIMER is slated to be removed in favor of using
> > > clock_nanosleep for SIGEV_THREAD timers rather than kernelspace
> > > timers unless a strong reason not to do that is discovered.
> > 
> > I tried looking into doing that at some point and just gave up, because
> > of the overrun accounting. If I understand correctly, every timer
> > expiration between the notification being generated and being delivered
> > increases the overrun counter, and for each notification it is supposed
> > to be latched and then reset. So for SIGEV_THREAD, that would be every
> > expiration between the call and the return. And for SIGEV_SIGNAL, that
> > would be every expiration between the signal being sent and being
> > handled.
> > 
> > All of which was too complicated for me to wrap my head around, but
> > maybe you have more luck.
> 
> Counting the number of overruns is conceptually just a division:
> taking the difference between the current time and last notification
> time and dividing by the interval length. There is some complexity to
> handler with making changes to the interval length (special case:
> stopping and starting the timer) but it's not that hard a problem.

We're in luck! Per POSIX:

    "When a timer for which a signal is still pending expires, no
    signal shall be queued, and a timer overrun shall occur. When a
    timer expiration signal is delivered to or accepted by a process,
    the timer_getoverrun() function shall return the timer expiration
    overrun count for the specified timer. The overrun count returned
    contains the number of extra timer expirations that occurred
    between the time the signal was generated (queued) and when it was
    delivered or accepted, up to but not including an
    implementation-defined maximum of {DELAYTIMER_MAX}. If the number
    of such extra expirations is greater than or equal to
    {DELAYTIMER_MAX}, then the overrun count shall be set to
    {DELAYTIMER_MAX}. The value returned by timer_getoverrun() shall
    apply to the most recent expiration signal delivery or acceptance
    for the timer. If no expiration signal has been delivered for the
    timer, the return value of timer_getoverrun() is unspecified."

TL;DR: Overruns are only defined for SIGEV_SIGNAL. So we can just
return 0 if we like, or some reasonable interpretation.

> One short-term fix that might be worth exploring is adding back a
> signal handler for SIGTIMER so it doesn't kill the process. The
> handler would just increment an "extra overruns" counter for the
> thread. It could only run during execution of the function, if the
> function unblocked the signal, since we would re-block the signal each
> time before the next sigwaitinfo.

In particular this means no "extra overruns" fixup is required here.
The short-term fix is just adding back the no-op signal handler.
Arguably even we *should* unblock the signal during execution of the
handler function, so that an expiration that arrives while the handler
is still running doesn't cause the next handler to run late, but
instead to be skipped as an overrun, and not delay/impede subsequent
expirations.

Rich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-05  5:56     ` Rich Felker
  2024-12-05  6:28       ` Rich Felker
@ 2024-12-05 18:25       ` Markus Wichmann
  2024-12-06 13:24         ` JinCheng Li
  1 sibling, 1 reply; 11+ messages in thread
From: Markus Wichmann @ 2024-12-05 18:25 UTC (permalink / raw)
  To: musl

Am Thu, Dec 05, 2024 at 12:56:44AM -0500 schrieb Rich Felker:
> One short-term fix that might be worth exploring is adding back a
> signal handler for SIGTIMER so it doesn't kill the process. The
> handler would just increment an "extra overruns" counter for the
> thread. It could only run during execution of the function, if the
> function unblocked the signal, since we would re-block the signal each
> time before the next sigwaitinfo.
>
> Rich

The easiest fix for the instant bug would probably be to just not mask
SIGTIMER out of the old signal mask returned by pthread_sigmask(). We
don't really care if it is blocked or not in most threads and explicitly
block it in the timer thread. This way, the signal just keeps its
blocking status forever in all other threads, and remains blocked in the
timer thread.

Side effect: Since SIGTIMER is 32, we can remove one masking instruction
on 32-bit architectures.

Ciao,
Markus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-05 18:25       ` Markus Wichmann
@ 2024-12-06 13:24         ` JinCheng Li
  2024-12-07  5:01           ` Rich Felker
  0 siblings, 1 reply; 11+ messages in thread
From: JinCheng Li @ 2024-12-06 13:24 UTC (permalink / raw)
  To: musl; +Cc: Markus Wichmann

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

Hi

> The easiest fix for the instant bug would probably be to just not mask
> SIGTIMER out of the old signal mask returned by pthread_sigmask(). We
> don't really care if it is blocked or not in most threads and explicitly
> block it in the timer thread. This way, the signal just keeps its
> blocking status forever in all other threads, and remains blocked in the
> timer thread.

Maybe just judge the old sigset in pthread_sigmask, if SIGTIMER in old sigset is blocked, it should be in a timer thread, we can just block SIGTIMER  whatever is set for SIGTIMER in new sigset. If SIGTIMER in old sigset is unblocked, just remain its status set by new sigset. As long as it is not in a timer thread, the status of SIGTIMER is unblocked or blocked doesn't affect  anything.


Best,
JinCheng

________________________________
From: Markus Wichmann <nullplan@gmx.net>
Sent: Friday, December 6, 2024 2:25
To: musl@lists.openwall.com <musl@lists.openwall.com>
Subject: Re: [musl] Do we need to enhance robustness in the signal mask?

Am Thu, Dec 05, 2024 at 12:56:44AM -0500 schrieb Rich Felker:
> One short-term fix that might be worth exploring is adding back a
> signal handler for SIGTIMER so it doesn't kill the process. The
> handler would just increment an "extra overruns" counter for the
> thread. It could only run during execution of the function, if the
> function unblocked the signal, since we would re-block the signal each
> time before the next sigwaitinfo.
>
> Rich

The easiest fix for the instant bug would probably be to just not mask
SIGTIMER out of the old signal mask returned by pthread_sigmask(). We
don't really care if it is blocked or not in most threads and explicitly
block it in the timer thread. This way, the signal just keeps its
blocking status forever in all other threads, and remains blocked in the
timer thread.

Side effect: Since SIGTIMER is 32, we can remove one masking instruction
on 32-bit architectures.

Ciao,
Markus

[-- Attachment #2: Type: text/html, Size: 4518 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-06 13:24         ` JinCheng Li
@ 2024-12-07  5:01           ` Rich Felker
  2024-12-11 16:51             ` JinCheng Li
  0 siblings, 1 reply; 11+ messages in thread
From: Rich Felker @ 2024-12-07  5:01 UTC (permalink / raw)
  To: JinCheng Li; +Cc: musl, Markus Wichmann

On Fri, Dec 06, 2024 at 01:24:20PM +0000, JinCheng Li wrote:
> Hi
> 
> > The easiest fix for the instant bug would probably be to just not mask
> > SIGTIMER out of the old signal mask returned by pthread_sigmask(). We
> > don't really care if it is blocked or not in most threads and explicitly
> > block it in the timer thread. This way, the signal just keeps its
> > blocking status forever in all other threads, and remains blocked in the
> > timer thread.
> 
> Maybe just judge the old sigset in pthread_sigmask, if SIGTIMER in
> old sigset is blocked, it should be in a timer thread, we can just
> block SIGTIMER whatever is set for SIGTIMER in new sigset. If
> SIGTIMER in old sigset is unblocked, just remain its status set by
> new sigset. As long as it is not in a timer thread, the status of
> SIGTIMER is unblocked or blocked doesn't affect anything.

As I've said repeatedly, this can't be solved in
pthread_sigmask/sigprocmask. There are ways to install sigsets that
don't go thru this path. Either we need to make it so there's no way
to get a (valid, non-UB) sigset_t with SIGTIMER unblocked, or we just
need to make it so it doesn't matter if it's unblocked or so unblocked
is the desired state. The latter approach looks like it fixes other
things too.

Rich

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [musl] Do we need to enhance robustness in the signal mask?
  2024-12-07  5:01           ` Rich Felker
@ 2024-12-11 16:51             ` JinCheng Li
  0 siblings, 0 replies; 11+ messages in thread
From: JinCheng Li @ 2024-12-11 16:51 UTC (permalink / raw)
  To: musl

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

HI

> Either we need to make it so there's no way
> to get a (valid, non-UB) sigset_t with SIGTIMER unblocked, or we just
> need to make it so it doesn't matter if it's unblocked or so unblocked
> is the desired state.

1.Use a noop handler instead of SIG_DFL for SIGTIMER. In this way, no matter how the user operates the sigmask, there is no need to worry the process exited unexpectedly.
2.To avoid frequent signal responses, re-block SIGTIMER after each notify is executed in SIGEV-THREAD.
This method can skip the additional protection of internal signals and prevent developers from being aware of changes in sig*set.
Does this introduce other risks, for example, if sigwaitinfo is triggered after the handler responds, is it possible to cause time response abnormality?
POSIX does not appear to restrict the behavior of this signal.  Is the old SIG_DFL for SIGTIMER necessary?

Best,
JinCheng

________________________________
From: Rich Felker <dalias@libc.org>
Sent: Saturday, December 7, 2024 13:01
To: JinCheng Li <naiveli233@outlook.com>
Cc: musl@lists.openwall.com <musl@lists.openwall.com>; Markus Wichmann <nullplan@gmx.net>
Subject: Re: [musl] Do we need to enhance robustness in the signal mask?

On Fri, Dec 06, 2024 at 01:24:20PM +0000, JinCheng Li wrote:
> Hi
>
> > The easiest fix for the instant bug would probably be to just not mask
> > SIGTIMER out of the old signal mask returned by pthread_sigmask(). We
> > don't really care if it is blocked or not in most threads and explicitly
> > block it in the timer thread. This way, the signal just keeps its
> > blocking status forever in all other threads, and remains blocked in the
> > timer thread.
>
> Maybe just judge the old sigset in pthread_sigmask, if SIGTIMER in
> old sigset is blocked, it should be in a timer thread, we can just
> block SIGTIMER whatever is set for SIGTIMER in new sigset. If
> SIGTIMER in old sigset is unblocked, just remain its status set by
> new sigset. As long as it is not in a timer thread, the status of
> SIGTIMER is unblocked or blocked doesn't affect anything.

As I've said repeatedly, this can't be solved in
pthread_sigmask/sigprocmask. There are ways to install sigsets that
don't go thru this path. Either we need to make it so there's no way
to get a (valid, non-UB) sigset_t with SIGTIMER unblocked, or we just
need to make it so it doesn't matter if it's unblocked or so unblocked
is the desired state. The latter approach looks like it fixes other
things too.

Rich

[-- Attachment #2: Type: text/html, Size: 5649 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-12-11 16:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-12-03  3:06 [musl] Do we need to enhance robustness in the signal mask? JinCheng Li
2024-12-03 14:44 ` Rich Felker
2024-12-03 18:18   ` Markus Wichmann
2024-12-04  7:00     ` JinCheng Li
2024-12-05  2:28       ` Rich Felker
2024-12-05  5:56     ` Rich Felker
2024-12-05  6:28       ` Rich Felker
2024-12-05 18:25       ` Markus Wichmann
2024-12-06 13:24         ` JinCheng Li
2024-12-07  5:01           ` Rich Felker
2024-12-11 16:51             ` JinCheng Li

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

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 NNTP newsgroup(s).