pypcode icon indicating copy to clipboard operation
pypcode copied to clipboard

added mutexes to xml globals

Open whoismissing opened this issue 2 years ago • 1 comments

On the call-path from m_document = m_document_storage.openDocument(path); in sleigh/xml.cc::xml_parse(), the global variables global_scan and handler are set and used to perform the xml parsing. This is problematic in a multithreaded environment because both variables are modified without a thread locking mechanism.

Using pypcode with an additional thread will result in a SIGSEGV or SIGABRT when loading the sla file.

This fix is ported from the sleighcraft project.

I will duplicate this fix in a PR to upstream ghidra and track it here as well.

A POC for reproducing the crash is below. I've tested on my local pypcode version 1.0.2.

"""
Test the pypcode sleigh bindings in a multithreaded environment.
"""
import logging
import threading

from pypcode import Arch, Context

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)


def get_pcode(lang_id: str = "MIPS:LE:32:default"):
    """
    :param lang_id: a string that identifies the architecture, endianness, and
        bits of the code. See 'python3 -m pypcode -l' for a list of lang_ids.
    """
    langs = {l.id:l for arch in Arch.enumerate() for l in arch.languages}
    context = Context(langs[lang_id])


def main():
    t1 = threading.Thread(target=get_pcode, args=())
    t2 = threading.Thread(target=get_pcode, args=())
    t1.start()
    t2.start()

    t1.join()
    t2.join()
    print("done")

if __name__ == "__main__":
    main()

"""
user@pc:~/test$ python test_pypcode_multithreaded.py
Segmentation fault                                                        
user@pc:~/test$ python test_pypcode_multithreaded.py
terminate called after throwing an instance of 'XmlError'
terminate called recursively
Aborted
"""

whoismissing avatar Jul 14 '22 11:07 whoismissing

PR to upstream ghidra https://github.com/NationalSecurityAgency/ghidra/pull/4443

whoismissing avatar Jul 14 '22 11:07 whoismissing

Good find, thanks! Hopefully this gets resolved to not use globals in the future. Apologies for the merge delay.

mborgerson avatar Oct 13 '22 19:10 mborgerson