[syslog-ng] [PATCH 0/3] WIP: Convert syslog-ng to use upstream ivykis

[Gergely Nagy]

Lennert Buytenhek <buytenh at 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]