[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190925173424.GX9017@brightrain.aerifal.cx>
Date: Wed, 25 Sep 2019 13:34:24 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Cc: changdiankang <changdiankang@...wei.com>
Subject: Re: [PATCH] time: Fix bug in timer_create
On Wed, Sep 25, 2019 at 01:19:05PM -0400, Rich Felker wrote:
> On Tue, Sep 24, 2019 at 11:16:41AM +0000, changdiankang wrote:
> > time: Fix bug in timer_create
> >
> > When create a SIGEV_THREAD timer, in helper thread "start",
> > "id" is assigned with self->timer_id immediately after pthread_create.
> > But the real timer_id is assigned to self->timer_id after SYS_timer_create.
> > So "id" in "start" always be 0, and timer_delete will always fail.
> > Put assignement of "id" after pthread_barrier_wait can fix this bug.
> >
> > Signed-off-by: Chang Diankang <changdiankang@...wei.com<mailto:changdiankang@...wei.com>>
> >
> > diff --git a/src/time/timer_create.c b/src/time/timer_create.c
> > index c5e40a1..4172b9e 100644
> > --- a/src/time/timer_create.c
> > +++ b/src/time/timer_create.c
> > @@ -48,13 +48,14 @@ static void *start(void *arg)
> > {
> > pthread_t self = __pthread_self();
> > struct start_args *args = arg;
> > - int id = self->timer_id;
> > + int id;
> > jmp_buf jb;
> >
> > void (*notify)(union sigval) = args->sev->sigev_notify_function;
> > union sigval val = args->sev->sigev_value;
> >
> > pthread_barrier_wait(&args->b);
> > + id = self->timer_id;
> > for (;;) {
> > siginfo_t si;
> > while (sigwaitinfo(SIGTIMER_SET, &si) < 0);
> >
>
> Thanks! In the future, please send patches as attachments rather than
> inline since it looks like your mailer software is corrupting the
> patch (converting tabs to spaces, maybe other problems too). I can
> apply it manually though.
Actually, I don't think this patch is a complete fix. After the
barrier wait passes, the caller can immediately call timer_delete,
which sets the sign bit of ->timer_id, possibly before the start
function reads the initial value of ->timer_id and saves it.
I'll extend this to a full fix. Just masking off the bit would be a
hack that would work, but would still leave the formal data race, so I
think it would make sense to just drop use of the barrier and instead
use a semaphore that both sides have to post (timer thread does
wait->post, calling thread does post->wait). That would be nice in
terms of getting rid of linking dependency on pthread barriers too.
Rich
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.
