need to understand remove_error_table()

Jeffrey Altman jaltman at secure-endpoints.com
Sun Sep 30 10:24:45 EDT 2007


remove_error_table(struct error_table *et) removes an error table from
either the dynamic or static error table lists.  It does so by comparing
the error table 'base' values instead of the pointer values to the error
tables themselves.

I have tracked down random crashes in applications on Windows that
load/unload modules that use the Kerberos libraries to corruption of the
dynamic error table.  The problem is caused by multiple error tables
being added to the dynamic list with the same base value.  It turns out
that all of the instances of the error table are from the same library,
for example xpprof32.dll, but they are being added from different
instances.  The et pointer values are different for each instance. 

If the instances are not unloaded in the reverse of the order in which
they were added, the existing remove_error_table() will remove the first
entry in the dynamic error table list for the matching base value.  This
will leave a table pointer to memory that is no longer valid when the
current instance of the library is unloaded.

I believe (at least in the dynamic list case) that the comparison for
removal should be based upon the unique table pointer values and not the
error base values.

What I am not sure is whether or not the same logic applies to the
static table.  Under what circumstances should a static error table
entry be removed using a different error table instance than was used
during its addition to the list?  If there are none, then I believe this
patch should be committed.

Index: error_message.c
===================================================================
--- error_message.c     (revision 19964)
+++ error_message.c     (working copy)
@@ -338,17 +338,17 @@
     if (merr)
        return merr;

-    /* Remove the first occurrance we can find.  Prefer dynamic
+    /* Remove the entry that matches the error table instance.  Prefer
dynamic
        entries, but if there are none, check for a static one too.  */
     for (del = &et_list_dynamic; *del; del = &(*del)->next)
-       if ((*del)->table->base == et->base) {
+       if ((*del)->table == et) {
            /*@only@*/ struct dynamic_et_list *old = *del;
            *del = old->next;
            free (old);
            return k5_mutex_unlock(&et_list_lock);
        }
     for (el = &_et_list; *el; el = &(*el)->next)
-       if ((*el)->table != NULL && (*el)->table->base == et->base) {
+       if ((*el)->table == et) {
            struct et_list *old = *el;
            *el = old->next;
            old->next = NULL;

Jeffrey Altman

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3355 bytes
Desc: S/MIME Cryptographic Signature
Url : http://mailman.mit.edu/pipermail/krbdev/attachments/20070930/c2c4feb4/attachment.bin


More information about the krbdev mailing list