lt_dllopenext() returns invalid handle (was: 1.1.2 is broken if you have closefrom() )

[Frank Cusack]

On June 15, 2006 4:30:19 PM -0400 Alan DeKok <aland at nitros9.org> wrote:
> Frank Cusack <fcusack at fcusack.com> wrote:
>> Dunno if you didn't find time, or weren't able to reproduce it
>
>   I couldn't bear the though of fighting with that crap again.
>
>   Look at the code, it does the following:
>
> ...
>   strcat (tmp, archive_ext);
>   errors = try_dlopen (&handle, tmp);
>   if (errors) return 0;
>
>   /* If we found FILENAME, stop searching -- whether we were able to
>      load the file as a module or not.  If the file exists but loading
>      failed, it is better to return an error message here than to
>      report FILE_NOT_FOUND when the alternatives (foo.so etc) are not
>      in the module search path.  */
>   if (handle || ((errors > 0) && !file_not_found ()))
>     {
>       LT_DLFREE (tmp);
>       return handle;
>     }
> ...
>
>   Note that if (errors > 0) AND the file was found, it returns the
> handle.  i.e. It finds the CURRENT library, but not one that the
> current library needs.
>
>   The idea of returning the handle if (errors > 0) is something that
> is mind-blowingly retarded.

If the file was found, but errors > 0, handle is supposed to be NULL.
I couldn't find a case (from inspection) where this isn't true.
Dependencies are loaded before the library itself is loaded (if there
is a .la file and dependencies are known), and if this fails handle
is returned as NULL.  If dependencies are not known, the system dlopen()
is used which will implicitly load dependencies, and again, fail and
return NULL if a dependency cannot be loaded.

Apparently, it CAN happen that handle is not NULL, but I have to assume
that this line of code doesn't think that can happen. (Because of course,
it is a pretty bad idea to return a non-NULL handle on failure, and ltdl
is obviously not written by idiots.)

You can't just read this one line of code and say "handle is returned!",
you have to look at what is expected from try_dlopen().

>> But anyway, it looks easy to address in freeradius (as opposed to
>> ltdl.c) so I'm going to try that.
>
>   I don't see how.  The problem is that the handle is returned, but is
> NOT completely populated.  So ltdl_sym() de-references a NULL entry in
> the handle, and dies.
>
>   Since the handle structure is opaque, it's impossible for FreeRADIUS
> to know if the returned handle is OK, is is broken.  So I have no idea
> how you would handle this in FreeRADIUS.

Bah.  I didn't realize lt_dlhandle was opaque (because how does gdb know
what it looks like).  phooey.  OK, we'll need to patch ltdl.c, but
otherwise (ie autoconf stuff) we can still get away with untouched original
source.

-frank