xkbcommon / libxkbcommon

keymap handling library for toolkits and window systems
https://xkbcommon.org
Other
286 stars 125 forks source link

registry: Restore default libxml2 error handler after parsing #530

Closed sbstnk closed 1 month ago

sbstnk commented 1 month ago

Leaving the custom error handler could have resulted in a crash after the context has been freed.

Closes: https://github.com/xkbcommon/libxkbcommon/issues/529

sbstnk commented 1 month ago

To test this you can use the following C program:

gcc -I/usr/include/libxml2 ./xkb_test.c -o ./xkb_test -lxml2 -lxkbregistry

#include <xkbcommon/xkbregistry.h>
#include <libxml/parser.h>
#include <libxml/tree.h>
#include <string.h>

int main (int argc, char *argv[]) {
  struct rxkb_context *ctx;
  xmlDocPtr doc;
  const char *invalid_xml = "<test";
  char *temp;

  ctx = rxkb_context_new (0);
  rxkb_context_parse (ctx, "evdev");
  rxkb_context_unref (ctx);

  LIBXML_TEST_VERSION
  doc = xmlParseMemory (invalid_xml, strlen(invalid_xml));
  xmlFreeDoc (doc);
  xmlCleanupParser();

  return 0;
}

Without the patch it should randomly segfault if you run it a couple of times with backtraces like:

#0  0x00000000c9734006 in ?? ()
#1  0x00007ffff7e326f2 in rxkb_log () from /lib64/libxkbregistry.so.0
#2  0x00007ffff7e32b3a in xml_error_func () from /lib64/libxkbregistry.so.0
#3  0x00007ffff7e6b61f in xmlReportError () from /lib64/libxml2.so.2
#4  0x00007ffff7e6d4b1 in __xmlRaiseError () from /lib64/libxml2.so.2
#5  0x00007ffff7e71041 in xmlFatalErrMsgStrIntStr.lto_priv.0 () from /lib64/libxml2.so.2
#6  0x00007ffff7e82227 in xmlParseElementStart () from /lib64/libxml2.so.2
#7  0x00007ffff7e83e45 in xmlParseElement () from /lib64/libxml2.so.2
#8  0x00007ffff7e84ab0 in xmlParseDocument () from /lib64/libxml2.so.2
#9  0x00007ffff7e8503c in xmlSAXParseMemoryWithData () from /lib64/libxml2.so.2
#10 0x00000000004011fe in main ()
wismill commented 1 month ago

Thanks for the excellent work! I completed with comments, a test and a changelog entry.

sbstnk commented 1 month ago

Thanks for the quick review, improvements and merge! Also glad that you noticed that I missed an early return, that I somehow kept reading as another goto :)