Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190909145429.GG22009@port70.net>
Date: Mon, 9 Sep 2019 16:54:29 +0200
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: Subject: [PATCH] pthread: Fix bug that pthread_create may
 cause priority inversion

* zhaohang (F) <zhaohang14@...wei.com> [2019-09-09 13:57:36 +0000]:
> diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
> index 7d4dc2e..ae08c0f 100644
> --- a/src/thread/pthread_create.c
> +++ b/src/thread/pthread_create.c
> @@ -181,15 +181,8 @@ static int start(void *p)
>  {
>         struct start_args *args = p;
>         if (args->attr) {
> -               pthread_t self = __pthread_self();
> -               int ret = -__syscall(SYS_sched_setscheduler, self->tid,
> -                       args->attr->_a_policy, &args->attr->_a_prio);
> -               if (a_swap(args->perr, ret)==-2)
> -                       __wake(args->perr, 1, 1);
> -               if (ret) {
> -                       self->detach_state = DT_DETACHED;
> -                       __pthread_exit(0);
> -               }
> +               if (a_cas(args->perr, -1, -2) == -1)
> +                       __wait(args->perr, 0, -2, 1);
>         }
>         __syscall(SYS_rt_sigprocmask, SIG_SETMASK, &args->sig_mask, 0, _NSIG/8);
>         __pthread_exit(args->start_func(args->start_arg));
> @@ -367,10 +360,14 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
>         }
> 
>         if (attr._a_sched) {
> -               if (a_cas(&err, -1, -2)==-1)
> -                       __wait(&err, 0, -2, 1);
> -               ret = err;
> -               if (ret) return ret;
> +               ret = -__syscall(SYS_sched_setscheduler, new->tid, attr._a_policy, &attr._a_prio);
> +               if (ret) {
> +                       new->detach_state = DT_DETACHED;
> +                       pthread_cancel(new);
> +                       return ret;

the child has the cancel signal blocked so it will never act on the signal.

but even if that's fixed, the detached child may not get scheduled to
handle the signal for a long time and will take up stack/tid resources.

i think Rich already has a solution that will deal with these issues.

> +               }
> +               if (a_swap(&err, 0) == -2)
> +                       __wake(&err, 1, 1);
>         }
> 
>         *res = new;

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.