Bug 103975 - Implicit loading/unloading of libpthread.so may crash user processes
Summary: Implicit loading/unloading of libpthread.so may crash user processes
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: threads (show other bugs)
Version: 6.2-PRERELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-threads (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-04 15:00 UTC by KUROSAWA
Modified: 2015-05-15 16:14 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description KUROSAWA freebsd_committer freebsd_triage 2006-10-04 15:00:50 UTC
A program (described as ProgA below) gets SIGSEGV if following conditions
are met:
- ProgA dlopen()s and dlclose()s a shared object (ShobjB)
- ProgA doesn't link libpthread.so
- ShbjB dynamically links libpthread.so
  (libpthread.so will be loaded when ProgA dlopen()s ShobjB)
- ProgA calls functions like gethostbyname() that uses __thr_jtable
  (in src/lib/libc/gen/_pthread_stubs.c) after unloading ShobjB.

The problem is that function pointers in __thr_jtable still point to functions
in libpthread.so after libpthread.so is unloaded from ProgA's memory space.

To fix the problem, a function that has __attribute__((destructor))
in libpthread should probably be implemented in order to recover
the initial state before unloading.

How-To-Repeat: The problem occurs on the web server built with following ports
when the httpd receives SIGHUP that is sent by newsyslog:
- www/apache20
- lang/php4
- databases/php4-pgsql
- databases/postgresql81-{client,server} with the option WITH_THREADSAFE=true

Or extract the following archive then run "make test."
The 3rd call of test() in pjt-replace.c causes SIGSEGV.

begin 644 pjt.tar.gz
M'XL(`#J[(T4``^V4WV_3,!#'^QK_%:?02>GH#[=-6JFC$Z,;?2DO6WD`(2'7
M<99L7A+%SA!"_._8257:CL%3-P'W>;GX_#W?Q9=+?J,;AX92.@X",+8_#OK6
M4CKP:[L&^C3HT_$P\`,*M#\(1N,&!`>OS%`JS0I3RFU99(I]88_IC"R*?G/.
M^CTV]B\AO]&]=^Q61(D4A\IA[F/D^X_WWQ\,]_KO#VG0`'JH@K;YS_M_?O'F
M_=QQIM"Y)K.WB[/YE>.\G$+3JS9:9'EV.;]87EF%^52Z*K.F4XA<,BX(85).
M?KFAA=(3,-O$Z?9V=K86D^V8;D:<IC>;M3;)H9-!\_6>AM39ZE"5=?G#*!6S
M0H1;T58&'9GKN!`LK$NPO@EQ.-/0"\5]+RVEA%,309Z[)4^*G?_M"^8'R/&G
M^0]&_M[\!X,!SO^3\"))N2Q#`:]"&?&T&Y^2GRZEPR3;=:5"AROK(N9"=,+A
M/DO":MH];L8.CD.A>(M\(X[21<DUQ)G2(M5P',,4KH6VZ]77E-T)SY499](Z
MW-8)<?(B277DN4=JLBN$0NBR2,U('^6?4K<--D<;8A/TG1`31.Y8DGJVDBJS
M?3#Y3@AQJL+<E8BR0D`HLURD52Y;3+WTW.K_9'XIYN#+Y>+\\^+LXP>KJ6-9
MI$6QUK9A<Q*7F1+54>MG+WX8LQ$1IWX%H+;DY^XY@B`(@B`(@B`(@B`(@B`(
2@B`(@B`(\N_Q`XCZB,H`*```
`
end
Comment 1 john 2006-10-05 14:06:20 UTC
On Wednesday 04 October 2006 09:56, KUROSAWA@freebsd.org, Takahiro wrote:
> 
> >Number:         103975
> >Category:       threads
> >Synopsis:       Implicit loading/unloading of libpthread.so may crash user processes
> >Confidential:   no
> >Severity:       serious
> >Priority:       medium
> >Responsible:    freebsd-threads
> >State:          open
> >Quarter:        
> >Keywords:       
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Wed Oct 04 14:00:50 GMT 2006
> >Closed-Date:
> >Last-Modified:
> >Originator:     KUROSAWA, Takahiro
> >Release:        6.2-PRERELEASE
> >Organization:
> >Environment:
> FreeBSD cube.nodomain.noroot 6.2-PRERELEASE FreeBSD 6.2-PRERELEASE #13: Fri Sep 29 14:34:05 JST 2006     kurosawa@cube.nodomain.noroot:/usr/obj/usr/src/sys/CUBE  i386
> 
> >Description:
> A program (described as ProgA below) gets SIGSEGV if following conditions
> are met:
> - ProgA dlopen()s and dlclose()s a shared object (ShobjB)
> - ProgA doesn't link libpthread.so
> - ShbjB dynamically links libpthread.so
>   (libpthread.so will be loaded when ProgA dlopen()s ShobjB)
> - ProgA calls functions like gethostbyname() that uses __thr_jtable
>   (in src/lib/libc/gen/_pthread_stubs.c) after unloading ShobjB.
> 
> The problem is that function pointers in __thr_jtable still point to functions
> in libpthread.so after libpthread.so is unloaded from ProgA's memory space.

Actually, I wonder if it should be allowed to unload at all.  On 4.x at work
we ran into an issue with the linuxthreads library loading, setting _is_threaded,
then unloading with a malloc() occurring during the destructors resolving a
_spinlock() weak symbol, then after the libraries were completely unloaded, the
next malloc() blew up when _spinlock() pointed off into space.  Hmm, this specific
condition is handled I think since __isthreaded in 6.x libpthread isn't set until
you do pthread_create() which at that point means a symbol is resolved, and the
library won't be unloaded (I think).  Hmm, maybe not since that doesn't guarantee
that libc depends on libpthread (that is what keeps it from being unloaded IIRC).
So, maybe when the library sets __isthreaded it should call one of the libc
functions (like malloc) to force one of the weak symbols to be resolved so it
isn't unloaded.

> To fix the problem, a function that has __attribute__((destructor))
> in libpthread should probably be implemented in order to recover
> the initial state before unloading.

I'm not sure you can recover the state actually, hence why I think maybe we
should make it so that libpthread doesn't unload once it has been loaded.

-- 
John Baldwin
Comment 2 kabaev 2006-10-06 00:47:56 UTC
On Thu, 5 Oct 2006 09:06:20 -0400
John Baldwin <john@baldwin.cx> wrote:

> 
> Actually, I wonder if it should be allowed to unload at all.  On 4.x
> at work we ran into an issue with the linuxthreads library loading,
> setting _is_threaded, then unloading with a malloc() occurring during
> the destructors resolving a _spinlock() weak symbol, then after the
> libraries were completely unloaded, the next malloc() blew up when
> _spinlock() pointed off into space.  Hmm, this specific condition is
> handled I think since __isthreaded in 6.x libpthread isn't set until
> you do pthread_create() which at that point means a symbol is
> resolved, and the library won't be unloaded (I think).  Hmm, maybe
> not since that doesn't guarantee that libc depends on libpthread
> (that is what keeps it from being unloaded IIRC). So, maybe when the
> library sets __isthreaded it should call one of the libc functions
> (like malloc) to force one of the weak symbols to be resolved so it
> isn't unloaded.
> 
> > To fix the problem, a function that has __attribute__((destructor))
> > in libpthread should probably be implemented in order to recover
> > the initial state before unloading.
> 
> I'm not sure you can recover the state actually, hence why I think
> maybe we should make it so that libpthread doesn't unload once it has
> been loaded.
> 
> -- 
> John Baldwin


Linux does not allow pthread library to be unloaded presumably because
of reasons like this. From readelf -a /compat/linux/lib/libpthread.so.0:

 0x6ffffffb (FLAGS_1)                    Flags: NODELETE INITFIRST

Infortunately, rtld does not implement NODELETE and INITFIRST. Both are
addressed in my patch that I am yet to commit.


-- 
Alexander Kabaev
Comment 3 takahiro.kurosawa 2006-10-06 09:49:48 UTC
Alexander Kabaev <kabaev@gmail.com> wrote:
> On Thu, 5 Oct 2006 09:06:20 -0400
> John Baldwin <john@baldwin.cx> wrote:
>
> > > To fix the problem, a function that has __attribute__((destructor))
> > > in libpthread should probably be implemented in order to recover
> > > the initial state before unloading.
> >
> > I'm not sure you can recover the state actually, hence why I think
> > maybe we should make it so that libpthread doesn't unload once it has
> > been loaded.

I understand that it's way easier to prohibit unloading of libpthread
than to change the code safely unloadable.
Thanks for your explanation, John!

> Linux does not allow pthread library to be unloaded presumably because
> of reasons like this. From readelf -a /compat/linux/lib/libpthread.so.0:
>
>  0x6ffffffb (FLAGS_1)                    Flags: NODELETE INITFIRST
>
> Infortunately, rtld does not implement NODELETE and INITFIRST. Both are
> addressed in my patch that I am yet to commit.

I'm looking forward to the commit of your patch into the CVS repository :-)
Maybe the following line should be added to src/lib/libpthread/Makefile
when rtld supports the NODELETE flag? :
  LDFLAGS+=-Wl,-znodelete

Thanks,
--
KUROSAWA, Takahiro
Comment 4 Daniel Eischen freebsd_committer freebsd_triage 2006-10-06 13:42:19 UTC
On Fri, 6 Oct 2006, Takahiro Kurosawa wrote:

> Alexander Kabaev <kabaev@gmail.com> wrote:
>> On Thu, 5 Oct 2006 09:06:20 -0400
>> John Baldwin <john@baldwin.cx> wrote:
>> 
>> > > To fix the problem, a function that has __attribute__((destructor))
>> > > in libpthread should probably be implemented in order to recover
>> > > the initial state before unloading.
>> >
>> > I'm not sure you can recover the state actually, hence why I think
>> > maybe we should make it so that libpthread doesn't unload once it has
>> > been loaded.
>
> I understand that it's way easier to prohibit unloading of libpthread
> than to change the code safely unloadable.
> Thanks for your explanation, John!
>
>> Linux does not allow pthread library to be unloaded presumably because
>> of reasons like this. From readelf -a /compat/linux/lib/libpthread.so.0:
>>
>>  0x6ffffffb (FLAGS_1)                    Flags: NODELETE INITFIRST
>> 
>> Infortunately, rtld does not implement NODELETE and INITFIRST. Both are
>> addressed in my patch that I am yet to commit.
>
> I'm looking forward to the commit of your patch into the CVS repository :-)
> Maybe the following line should be added to src/lib/libpthread/Makefile
> when rtld supports the NODELETE flag? :
> LDFLAGS+=-Wl,-znodelete

If that's the knob, then I'd agree.  You also want to make
the same change to libthr.

-- 
Dan