Ext2 Filesystems Utilities

Logged In: YES
user_id=628

Fixed in e2fsprogs 1.38

Logged In: YES
user_id=50364

Ok. So what about this:

If remove_error_table knows whether the list node has been
allocated by add_error_table or not it can decide whether to
free() the node or not.

Since we can't change the structure, we need another way to
identify those. What about putting the node pointers in an
sorted array or something?

Logged In: YES
user_id=628

The whole point of com_err is that it is a _common_ error
code handling system. For example, you can have a Zephyr
library which links to a Kerberos v5 Library, and the
application could define its own set of error codes. So
what if the Zephyr library is using a different version of
the library than what the Krb5 library is using? So it's
important to maximize cross-portability as much as possible.

The reason why this bug turned up is precisely because the
krb5 code made changes to their com_err package in the 1.4
which wasn't compatible with another com_err library.
Since e2fsprogs's com_err package is the system com_err
library for Linux, we need to keep it as portable as
possible, or it's an invitation for future bugs.

Logged In: YES
user_id=50364

Don't other com_err libraries come with their own compile_et
version? Or does this need to be interchangable?

Well, do what you think would be best. :)

Perhaps some kind of way to know whether the linked list
element has been allocated by libcom_err or not and not
freeing it (does unlink it) accordingly?

The current compile_et uses a static list node. Just leaving
it is okay, no memory leaks there, as long as it gets
removed from the _et_list before unloading the shared library.

Logged In: YES
user_id=628

chtephan,

The problem with changing compile_et to use
add_error_table() is not all com_err libraries supports the
add_error_table() interface. So using it can cause
compatibility problems.

Logged In: YES
user_id=628

I just looked at your patch
(e2fsprogs-1.36-et-dynamic.patch) and it has some definite
advantages. I will probably use its approach as well, so
that old libraries with old et.c files will work correctly.

Logged In: YES
user_id=50364

As I already told, I think this is a compile_et error which
generates bad code in mit-krb5. initialize_XXX_error_table()
should register its error table using add_error_table()
instead of manually fiddling with _et_list.

Logged In: YES
user_id=628

OK, I've found the problem. The issue is that one library
(pam_krb5) is calling the old interface,
initialize_XXX_error_table(), and then the krb5 library is
calling add_error_table(), which returns since the error
table is already defined, but then when remove_error_table()
gets called, tries to free a non-allocated memory, and then
boom.

The attached patch will fix the problem, but you will need
to compile and install compile_et, and then recompile the
krb5 library in order to get the fixed version of
initialize_XXX_error_table.

Logged In: YES
user_id=50364

Please also look at

http://bugs.gentoo.org/show_bug.cgi?id=95283

and seem my slightly different solution.

compile_et produces code that modifies _et_list directly and
adds static list members which can't be freed instead of
using the exported function to add a dynamic list member.

My opinion is that compile_et should be fixed so that
_et_list nodes can always be freed.

Patch: http://bugs.gentoo.org/attachment.cgi?id=60751

This makes the glibc invalid free errors go away
(mit-krb5-1.4, pam_krb5, su).

Logged In: YES
user_id=7796

The MIT Kerberos 1.4 source tree bundles a modified version
of libcom_err, which handles things slightly differently
than the version in e2fsprogs does, but we're linking with
the e2fsprogs version to avoid having multiple versions of
the library on the system.

The attached patch should prevent this error from corrupting
the heap by separating the list of dynamically-registered
tables from the list of static tables, and only searching
the dynamic list in remove_error_table().

Logged In: YES
user_id=628

You're not supposed to call remove_error_table() on an error
table which
was initialized using initialize_XXX_error_table().
remove_error_table() and
add_error_table() are a matched pair.
initialize_XXX_error_table() and remove_error_table() are not.

There's definitely a bug here, but it's not clear whether
it's in the PAM krb5 code or in the MIT Kerberos 1.4 code...