Lennert Buytenhek <buytenh@wantstofly.org> writes:
On Mon, May 07, 2012 at 11:42:30PM +0200, Gergely Nagy wrote:
A long long time ago, upstream and BalaBit's version of ivykis started to diverge, which - at the time - was necessary, but the intent always was to eventually get things upstream and merged, to allow syslog-ng to use upstream ivykis, as-is.
With the help of Lennert Buytenhek (ivykis upstream) and Balazs Scheidler, I present you the first work in progress patch set against syslog-ng that enables it to work with an unmodified ivykis!
(Whoops, I didn't see this email until now.)
Yay! Thanks for doing this work.
Most of the work has been done by you and Bazsi. I'm just putting the pieces together. O:)
Another thing is that the first patch stops libsyslog-ng-crypto from being directly linked to ivykis. This is not an issue when ivykis is statically compiled into libsyslog-ng, since all those symbols will be exported to other modules, including libsyslog-ng-crypto. However, it might become a problem when using a dynamically linked ivykis. I haven't tested that case yet.
The reason I remove the link is that if ivykis is statically compiled in to both shared objects, the constructors will run twice, as dlopen()ing anything linked against libsyslog-ng-crypto will trigger them again. This leads to an abort() inside ivykis.
This can be done on the ivykis side too: separate the constructors from the init functions, and make the constructors no-op if they ran already: this allows the init functions to continue aborting if called more than once, but allows the constructors to run twice. I have to discuss this with upstream, and see if we need this at all, or if my workaround of removing the linkage from libsyslog-ng-crypto is enough and without side effects.
If the constructors run twice but bss variables are still shared, then yeah, we should do something like what you said.
As far as I see, what happens is that when syslog-ng is started, the ivykis statically linked into libsyslog-ng gets loaded and the constructors run. So far, so good, that's how it is supposed to work! When syslog-ng dlopen()s a module linked against libsyslog-ng-crypto, which also has an ivykis statically linked in, ld.so notices that the symbols are already resolved, so won't load them again, BUT the constructors *are* different, since the constructor can be thought of as a function in the lib that runs whatever we had an __attribute__((constructor))__ on. This means that the init functions will run again, even though they were not redefined, because the newly loaded constructor-collecting function-like thingy does get run regardless. This means that constructors and destructors should be made idempotent. Which might be tricky if we consider that a module can (in theory, syslog-ng doesn't do this AFAIK) be unloaded, which would trigger the destructor, while the main code would still use it. So deinit should only be called from the destructors if there are no users left. This makes things a little bit more complicated, I think. For syslog-ng, it's enough if nothing but libsyslog-ng gets linked to ivykis, so my workaround will do the trick. For other things, though, this may not be enough.
Nevertheless! The good news still is, that we're *very* close to being able to build with upstream ivykis! A few issues to fix, and then comes the next part: going over all the changes that were made to the BalaBit fork of ivykis, and send everything upstream that has not been sent yet.
Right, there's several changes still in your version of ivykis that haven't made it into mine (e.g. to have an abstraction for syslog(), which seems a generally good idea to have, and probably there's more bits as well). Do you think syslog-ng is ready for inclusion into e.g. Fedora as-is, or do you want to get those changes merged into upstream ivykis as well first?
The move to upstream ivykis is something that I wouldn't feel comfortable with doing in 3.3 (at least not yet), and 3.4 is still only alpha, and I wouldn't push it towards Fedora just yet. However, as far as I'm concerned, syslog-ng 3.4 + current ivykis (with the io handlers fixed, patch will be on its way in a bit) is ready to be experimented with, and included in development/experimental distro branches (such as OpenSUSE Factory and Debian experimental, and whatever Fedora has - if anything - that is similar in spirit). The other differences can be resolved later, since 3.4 is still in development. In the long run, when everything was sent your way upstream and the code differences resolved, I might consider merging the patches into syslog-ng 3.3, but I want to be very careful about it. I prefer fixing things instead of accidentally breaking them, and replacing the forked ivykis with upstream is quite an invasive change. -- |8]