Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#60702 closed defect (fixed)

darwintrace's setup is not safe in the presence of other initializers

Reported by: saagarjha (Saagar Jha) Owned by: saagarjha (Saagar Jha)
Priority: Normal Milestone: MacPorts 2.6.3
Component: base Version:
Keywords: tracemode Cc:


darwintrace initializes its pthread keys in a library initializer in an attempt to ensure they are set up before any other code using them runs. However, this is not good enough: as darwintrace interposes standard library functions and library initializer order is largely indeterminate, this means that other constructors may run before ours does and call a function we interpose. This means we will use uninitialized keys; on my system this now causes a SIGSEGV. I have a rough patch for this but I'm still trying to figure out how to get MacPorts to actually compile in my changes to darwintrace.h/c–no matter what I do to it, configure/make seems to ignore my changes. If anybody has any pointers on how I can build my changes they would be much appreciated and likely result in a patch being sent :)

Change History (8)

comment:1 Changed 2 years ago by ryandesign (Ryan Schmidt)

Not sure what to say, other than you get a copy of the code and make your modifications to it. If you've modified or files, run to regenerate configure and Makefiles. Then you can run configure, make, and sudo make install.

comment:2 Changed 2 years ago by saagarjha (Saagar Jha)

Ok, so the issue with my changes not being picked up was actually a problem with my OS and not MacPorts, so that's resolved.

However, I ran into another problem: there's actually another static constructor in darwintrace, store_env. This one is an issue because it uses the COPYENV macro, which calls malloc. But the problem is that we're in the middle of one of libsystem_malloc.dylib's static initializers (which calls a function we interpose), and we can't call malloc at this point because it hasn't initialized it's arenas. That's exactly what it was trying to do when we interposed it and tried to do our own setup in the middle! So I'm unsure what the best way forwards from here would be. Should I allocate a fixed size buffer as a global and call abort if we overflow that? Use alloca?

comment:3 Changed 2 years ago by saagarjha (Saagar Jha)

Summary: darwintrace's tid_key/sock_key creation is not safe in the presence of other initializersdarwintrace's setup is not safe in the presence of other initializers

comment:4 Changed 2 years ago by saagarjha (Saagar Jha)

Ok, so I wrote a simple little bump allocator for the environment keys…but immediately ran into more crashes due to malloc being called from the rest of setup code. Not completely sure what we should be doing here; I think I'll give forcing libmalloc to load and then using dynamic interposition a shot and if that doesn't work ask on the mailing list.

comment:5 Changed 2 years ago by saagarjha (Saagar Jha)

Turns out with a bit of boilerplate you don't need to do this. Pull request:

comment:6 Changed 2 years ago by saagarjha (Saagar Jha)

Owner: set to saagarjha
Resolution: fixed
Status: newclosed

In d96e5a85afd0ce3cdd9d8e56409ba89bafd9cccc/macports-base (master):

Do not interpose functions before initialization

Static initializers have no defined order in which they are run;
interposed functions, however, take effect at program initialization. In
some cases, our interposed functions would be called by libSystem's
constructors and prior to *our* constructors, and this would lead to
crashes in trace mode as we would be in an uninitialized state. All our
interposers now immediately delegate to their interposees until the are
sure initialization has been performed.

Closes: #60702

comment:7 Changed 2 years ago by neverpanic (Clemens Lang)

In 7e9c35463501ef384d644b30845e05af89ef64c9/macports-base (master):

Update ChangeLog for 2.6.3

Document d96e5a85a.

See: #60702

comment:8 Changed 2 years ago by neverpanic (Clemens Lang)

Milestone: MacPorts 2.6.3
Note: See TracTickets for help on using tickets.