[<prev] [next>] [day] [month] [year] [list]
Message-ID: <tencent_8EC40BF790FB5548CF98DD5D8DE22882900A@qq.com>
Date: Thu, 11 Jul 2024 22:42:43 +0800
From: "AK47" <250200715@...com>
To: "musl" <musl@...ts.openwall.com>
Subject: Re:Re:RE: Maybe A Bug about timer_create and pthread_barrier_wait
Sorry, there is some bug in my QQ email. I will retry to send an email
Original Email
From:"AK47"< 250200715@...com >;
Sent Time:2024/7/11 22:40
To:"musl"< musl@...ts.openwall.com >;
Subject:Re:[musl] RE: Maybe A Bug about timer_create and pthread_barrier_wait
Hello:
&gt;pthread_barrier_destroy()&nbsp;waits&nbsp;for&nbsp;all&nbsp;threads&nbsp;to&nbsp;be&nbsp;done&nbsp;using&nbsp;the
&gt;barrier&nbsp;before&nbsp;destroying&nbsp;it.&nbsp;&nbsp;Without&nbsp;it,&nbsp;the&nbsp;storage&nbsp;for&nbsp;the&nbsp;barrier
&gt;can&nbsp;be&nbsp;deallocated&nbsp;when&nbsp;timer_create()&nbsp;returns&nbsp;while&nbsp;the&nbsp;other&nbsp;thread&nbsp;is
&gt;still&nbsp;using&nbsp;it&nbsp;inside&nbsp;the&nbsp;pthread_barrier_wait()&nbsp;implementation.
Sorry, I don't think add pthread_barrier_destroy in timer_create can solve this bug. In timer_create, b-&gt;_b_limit should be 1, and it will never meet the waiting condition (b-&gt;_b_limit &lt; 0) in pthread_barrier_destroy.
And I think add additional synchronization to timer_create is a temporary method. Maybe some bugfix is needed in pthread_barrier_wait.
int pthread_barrier_destroy(pthread_barrier_t *b)
{
if (b-&gt;_b_limit &lt; 0) {
if (b-&gt;_b_lock) {
int v;
a_or(&amp;b-&gt;_b_lock, INT_MIN);
while ((v = b-&gt;_b_lock) &amp; INT_MAX)
__wait(&amp;b-&gt;_b_lock, 0, v, 0);
}
__vm_wait();
}
return 0;
}
Li
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
Original&nbsp;Email
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
From:"Tavian&nbsp;Barnes"&lt;&nbsp;tavianator@...ianator.com&nbsp;&gt;;
Sent&nbsp;Time:2024/7/11&nbsp;3:11
To:"musl"&lt;&nbsp;musl@...ts.openwall.com&nbsp;&gt;;
Cc&nbsp;recipient:"250200715"&lt;&nbsp;250200715@...com&nbsp;&gt;;
Subject:[musl]&nbsp;RE:&nbsp;Maybe&nbsp;A&nbsp;Bug&nbsp;about&nbsp;timer_create&nbsp;and&nbsp;pthread_barrier_wait
(I&nbsp;cleaned&nbsp;up&nbsp;the&nbsp;HTML&nbsp;entities&nbsp;in&nbsp;this&nbsp;email,&nbsp;but&nbsp;please&nbsp;use&nbsp;plain&nbsp;text
mode&nbsp;next&nbsp;time)
&gt;&nbsp;Hello:
&gt;
&gt;&nbsp;I&nbsp;had&nbsp;a&nbsp;low-probability&nbsp;crash&nbsp;in&nbsp;the&nbsp;child&nbsp;thread&nbsp;when&nbsp;using&nbsp;the
&gt;&nbsp;timer_create&nbsp;interface.&nbsp;After&nbsp;debug,&nbsp;I&nbsp;found&nbsp;that&nbsp;the&nbsp;crash&nbsp;occured
&gt;&nbsp;when&nbsp;the&nbsp;sub-thread&nbsp;accessed&nbsp;in&nbsp;code&nbsp;"if&nbsp;(b-&gt;_b_waiters)"&nbsp;which&nbsp;is&nbsp;a
&gt;&nbsp;stack&nbsp;variable&nbsp;created&nbsp;in&nbsp;the&nbsp;main&nbsp;thread&nbsp;and&nbsp;passed&nbsp;to&nbsp;child&nbsp;thread
&gt;&nbsp;by&nbsp;args.&nbsp;It&nbsp;looks&nbsp;like&nbsp;the&nbsp;main&nbsp;thread's&nbsp;timer_create&nbsp;has&nbsp;finished
&gt;&nbsp;executing&nbsp;at&nbsp;this&nbsp;point,&nbsp;so&nbsp;the&nbsp;variables&nbsp;(start_args)&nbsp;on&nbsp;the&nbsp;stack
&gt;&nbsp;have&nbsp;been&nbsp;cleaned&nbsp;up.&nbsp;I&nbsp;take&nbsp;a&nbsp;look&nbsp;at&nbsp;the&nbsp;pthread_barrier_wait&nbsp;code
&gt;&nbsp;and&nbsp;I&nbsp;think&nbsp;it&nbsp;should&nbsp;be&nbsp;a&nbsp;scheduling&nbsp;problem&nbsp;in&nbsp;pthread_barrier_wait.
&gt;
&gt;&nbsp;Take&nbsp;the&nbsp;timer_create&nbsp;as&nbsp;an&nbsp;example,&nbsp;when&nbsp;the&nbsp;child&nbsp;thread&nbsp;is&nbsp;the
&gt;&nbsp;first&nbsp;thread&nbsp;for&nbsp;"pthread_barrier_wait"&nbsp;and&nbsp;it&nbsp;is&nbsp;suspened&nbsp;after&nbsp;it
&gt;&nbsp;executes&nbsp;the&nbsp;code&nbsp;"a_store(&amp;b-&gt;_b_lock,&nbsp;0)",&nbsp;then&nbsp;the&nbsp;main&nbsp;thread&nbsp;in
&gt;&nbsp;timer_create&nbsp;will&nbsp;arrive&nbsp;as&nbsp;the&nbsp;last&nbsp;thread,&nbsp;it&nbsp;will&nbsp;nerver&nbsp;wait&nbsp;for
&gt;&nbsp;the&nbsp;child&nbsp;thread&nbsp;to&nbsp;be&nbsp;rescheduled,&nbsp;the&nbsp;main&nbsp;thread&nbsp;can&nbsp;pass&nbsp;the
&gt;&nbsp;barrier&nbsp;and&nbsp;continue&nbsp;execution,&nbsp;the&nbsp;args&nbsp;created&nbsp;in&nbsp;timer_create&nbsp;will
&gt;&nbsp;be&nbsp;cleaned&nbsp;up.&nbsp;when&nbsp;the&nbsp;child&nbsp;thread&nbsp;is&nbsp;finally&nbsp;rescheduled,&nbsp;it&nbsp;access
&gt;&nbsp;the&nbsp;"b-&gt;_b_waiters"&nbsp;which&nbsp;has&nbsp;already&nbsp;been&nbsp;cleaned&nbsp;up&nbsp;by&nbsp;main&nbsp;thread
&gt;&nbsp;and&nbsp;the&nbsp;crash&nbsp;will&nbsp;occur.
&gt;
&gt;&nbsp;Is&nbsp;there&nbsp;a&nbsp;bug&nbsp;here?&nbsp;Looking&nbsp;forward&nbsp;to&nbsp;your&nbsp;reply.
&gt;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/*&nbsp;First&nbsp;thread&nbsp;to&nbsp;enter&nbsp;the&nbsp;barrier&nbsp;becomes&nbsp;the&nbsp;"instance&nbsp;owner"&nbsp;*/
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if&nbsp;(!inst)&nbsp;{
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;struct&nbsp;instance&nbsp;new_inst&nbsp;=&nbsp;{&nbsp;0&nbsp;};
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int&nbsp;spins&nbsp;=&nbsp;200;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;b-&gt;_b_inst&nbsp;=&nbsp;inst&nbsp;=&nbsp;&amp;new_inst;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;a_store(&amp;b-&gt;_b_lock,&nbsp;0);
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if&nbsp;(b-&gt;_b_waiters)&nbsp;__wake(&amp;b-&gt;_b_lock,&nbsp;1,&nbsp;1);&nbsp;&nbsp;//&nbsp;crash&nbsp;here&nbsp;b-&gt;_b_waiters
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;while&nbsp;(spins--&nbsp;&amp;&amp;&nbsp;!inst-&gt;finished)
&gt;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;/*&nbsp;First&nbsp;thread&nbsp;to&nbsp;enter&nbsp;the&nbsp;barrier&nbsp;becomes&nbsp;the&nbsp;"instance&nbsp;owner"&nbsp;*/
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if&nbsp;(!inst)&nbsp;{
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;struct&nbsp;instance&nbsp;new_inst&nbsp;=&nbsp;{&nbsp;0&nbsp;};
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;int&nbsp;spins&nbsp;=&nbsp;200;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;b-&gt;_b_inst&nbsp;=&nbsp;inst&nbsp;=&nbsp;&amp;new_inst;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;a_store(&amp;b-&gt;_b_lock,&nbsp;0);
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;//&nbsp;when&nbsp;the&nbsp;child&nbsp;thread&nbsp;is&nbsp;the&nbsp;first&nbsp;thread&nbsp;and&nbsp;is&nbsp;scheduled&nbsp;out&nbsp;here
&gt;
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;if&nbsp;(b-&gt;_b_waiters)&nbsp;__wake(&amp;b-&gt;_b_lock,&nbsp;1,&nbsp;1);
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;while&nbsp;(spins--&nbsp;&amp;&amp;&nbsp;!inst-&gt;finished)
This&nbsp;looks&nbsp;like&nbsp;a&nbsp;real&nbsp;bug&nbsp;in&nbsp;timer_create()&nbsp;to&nbsp;me.&nbsp;&nbsp;Here's&nbsp;an&nbsp;untested
fix:
From:&nbsp;Tavian&nbsp;Barnes&nbsp;
Date:&nbsp;Wed,&nbsp;10&nbsp;Jul&nbsp;2024&nbsp;14:27:21&nbsp;-0400
Subject:&nbsp;[PATCH]&nbsp;timer_create:&nbsp;destroy&nbsp;the&nbsp;barrier&nbsp;before&nbsp;returning
pthread_barrier_destroy()&nbsp;waits&nbsp;for&nbsp;all&nbsp;threads&nbsp;to&nbsp;be&nbsp;done&nbsp;using&nbsp;the
barrier&nbsp;before&nbsp;destroying&nbsp;it.&nbsp;&nbsp;Without&nbsp;it,&nbsp;the&nbsp;storage&nbsp;for&nbsp;the&nbsp;barrier
can&nbsp;be&nbsp;deallocated&nbsp;when&nbsp;timer_create()&nbsp;returns&nbsp;while&nbsp;the&nbsp;other&nbsp;thread&nbsp;is
still&nbsp;using&nbsp;it&nbsp;inside&nbsp;the&nbsp;pthread_barrier_wait()&nbsp;implementation.
Link:&nbsp;https://www.openwall.com/lists/musl/2024/07/08/1
---
&nbsp;src/time/timer_create.c&nbsp;|&nbsp;1&nbsp;+
&nbsp;1&nbsp;file&nbsp;changed,&nbsp;1&nbsp;insertion(+)
diff&nbsp;--git&nbsp;a/src/time/timer_create.c&nbsp;b/src/time/timer_create.c
index&nbsp;9216b3ab..42c69848&nbsp;100644
---&nbsp;a/src/time/timer_create.c
+++&nbsp;b/src/time/timer_create.c
@@&nbsp;-121,6&nbsp;+121,7&nbsp;@@&nbsp;int&nbsp;timer_create(clockid_t&nbsp;clk,&nbsp;struct&nbsp;sigevent&nbsp;*restrict&nbsp;evp,&nbsp;timer_t&nbsp;*restrict
&nbsp;&nbsp;&nbsp;}
&nbsp;&nbsp;&nbsp;td-&gt;timer_id&nbsp;=&nbsp;timerid;
&nbsp;&nbsp;&nbsp;pthread_barrier_wait(&amp;args.b);
+&nbsp;&nbsp;pthread_barrier_destroy(&amp;args.b);
&nbsp;&nbsp;&nbsp;if&nbsp;(timerid&nbsp;&lt;&nbsp;0)&nbsp;return&nbsp;-1;
&nbsp;&nbsp;&nbsp;*res&nbsp;=&nbsp;(void&nbsp;*)(INTPTR_MIN&nbsp;|&nbsp;(uintptr_t)td&gt;&gt;1);
&nbsp;&nbsp;&nbsp;break;
Content of type "text/html" skipped
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.
