Problem with ext/libxml RINIT/RSHUTDOWN

php.internals

Dmitry Stogov

20 years ago
Hi Wez, I found a bad code in ext/libxml. On each request startup/shutdown it changing some libxml callbacks. At first this slowdown each request, at second multi-threaded PHP may fail, because different threads may set/clean the same callbacks in the same time. May be it is possible to setup these callbacks forever in MINIT/MSHUTDOWN, set some flag (somthing like LIBXML(in_request)) in RINIT/RSHUTDOWN, and check it in callbacks. Thanks. Dmitry.

Rob Richards

20 years ago
Dmitry Stogov wrote:
> Hi Wez, > > I found a bad code in ext/libxml. > > On each request startup/shutdown it changing some libxml callbacks. > At first this slowdown each request, at second multi-threaded PHP may fail, > because different threads may set/clean the same callbacks in the same time. > > May be it is possible to setup these callbacks forever in MINIT/MSHUTDOWN, > set some flag (somthing like LIBXML(in_request)) in RINIT/RSHUTDOWN, and > check it in callbacks. >
Which ones in particular? Each of the callbacks being utilized are thread safe. The reason why they need to be set per request is in order to play nice with any other modules that might be loaded and use libxml2. for example: prior to how the current callbacks are implemented it was possible to blow away PHP stream usage by any of the xml extensions using mod_perl and its xsl module (could do this whether or not PHP was running threaded). There are also many other ways to screw with the libxml2 globals and effect PHP extensions based on libxml2 as well using other modules so the only way to protect them is on each request. In short putting them into the MINIT/MSHUTDOWN will open security holes. Rob

Wez Furlong

20 years ago
Sounds like a nasty situation. I don't think it's possible to sanely restore the handlers at rshutdown in a threaded build without screwing up the other threads that are currently executing. My suggestion is to not restore the handlers for threaded builds, and if there is a library sharing/abuse issue, then treat it as an installation/configuration problem for the person deploying it (eg: don't do that!). There's not much you can do about that without rewriting libxml2 to not use globals for the callbacks. :-/ --Wez. On 3/17/06, Rob Richards <rrichards@ctindustries.net> wrote:

Rob Richards

20 years ago
I don't think I made myself clear on the globals. The only situation there would be a problem is if PHP were built threaded and libxml2 were not (which as of 2.5.8 it is built threaded by default). When running threaded, each thread has its own global state, so when modified at request startup/shutdown the global is only changed for that thread, rather than being set at the module startup/shutdown which would set the default values used to initialize the global states of the threads. Rob Wez Furlong wrote:

Dmitry Stogov

20 years ago
BTW: libxml can be compiled with LIBXML_THREAD_ENABLED. Dmitry.

Marcus Börger

20 years ago
Hello Dmitry, which is the default.... however we could for optimization add some configure option "--i-damn-know-what-the-fuck-i-am-doing" that would setup the handlers only once. Obviously such a thing can only be used if nothing else is using libxml2. marcus Friday, March 17, 2006, 2:58:33 PM, you wrote:
> BTW: libxml can be compiled with LIBXML_THREAD_ENABLED.
> Dmitry.
>> -----Original Message----- >> From: Wez Furlong [mailto:kingwez@gmail.com] >> Sent: Friday, March 17, 2006 4:33 PM >> To: Rob Richards >> Cc: Dmitry Stogov; internals@lists.php.net >> Subject: Re: [PHP-DEV] Problem with ext/libxml RINIT/RSHUTDOWN >> >> >> Sounds like a nasty situation. >> I don't think it's possible to sanely restore the handlers at >> rshutdown in a threaded build without screwing up the other >> threads that are currently executing. >> >> My suggestion is to not restore the handlers for threaded >> builds, and if there is a library sharing/abuse issue, then >> treat it as an installation/configuration problem for the >> person deploying it (eg: don't do that!). There's not much >> you can do about that without rewriting libxml2 to not use >> globals for the callbacks. :-/ >> >> --Wez. >> >> On 3/17/06, Rob Richards <rrichards@ctindustries.net> wrote: >> > Dmitry Stogov wrote: >> > > Hi Wez, >> > > >> > > I found a bad code in ext/libxml. >> > > >> > > On each request startup/shutdown it changing some libxml >> callbacks. >> > > At first this slowdown each request, at second multi-threaded PHP >> > > may fail, because different threads may set/clean the >> same callbacks >> > > in the same time. >> > > >> > > May be it is possible to setup these callbacks forever in >> > > MINIT/MSHUTDOWN, set some flag (somthing like >> LIBXML(in_request)) in >> > > RINIT/RSHUTDOWN, and check it in callbacks. >> > > >> > Which ones in particular? >> > Each of the callbacks being utilized are thread safe. >> > The reason why they need to be set per request is in order to play >> > nice with any other modules that might be loaded and use libxml2. >> > >> > for example: prior to how the current callbacks are >> implemented it was >> > possible to blow away PHP stream usage by any of the xml extensions >> > using mod_perl and its xsl module (could do this whether or not PHP >> > was running threaded). There are also many other ways to screw with >> > the libxml2 globals and effect PHP extensions based on >> libxml2 as well >> > using other modules so the only way to protect them is on each >> > request. In short putting them into the MINIT/MSHUTDOWN will open >> > security holes. >> > >> > Rob >> > >> >>
Best regards, Marcus

Dmitry Stogov

20 years ago
Hi, After looking into libxml sources I don't think this should be fixed. The code is right (libxml is thread-safe) and slowdown is really small. Thanks. Dmitry.