qiling icon indicating copy to clipboard operation
qiling copied to clipboard

Unmapping part of a MMIO region causes future `ql.save`s to crash

Open LukeSerne opened this issue 3 years ago • 2 comments

Describe the bug When saving a qiling state after having unmapped part of an MMIO region, the save fails with a KeyError. See the below script.

Sample Code

import qiling

ql = qiling.Qiling(code=b"\0", archtype=qiling.const.QL_ARCH.ARM, ostype='linux')
ql.mem.map_mmio(0, 0x2000, read_cb=lambda *args: 0, write_cb=lambda *args: None)
ql.mem.unmap(0x1000, 0x1000)
ql.save()

Real Behavior

$ python main.py 
Traceback (most recent call last):
  File "main.py", line 7, in <module>
    ql.save()
  File "~/.local/lib/python3.8/site-packages/qiling/core.py", line 753, in save
    saved_states.update({"mem": self.mem.save()})
  File "~/.local/lib/python3.8/site-packages/qiling/os/memory.py", line 249, in save
    mem_dict['mmio'].append((lbound, ubound, perm, label, *self.mmio_cbs[(lbound, ubound)]))
KeyError: (0, 4096)

Expected behavior No KeyError is raised.

Additional context When calling ql.mem.unmap, the MMIO region is split into two parts (0-0x1000 and 0x1000 - 0x2000). The second part is then removed, but the field ql.mem.mmio_cbs is not updated correctly. Only the entry representing the (original) 0-0x2000 region is removed, but no new entry is added for the remaining 0-0x1000 region.

LukeSerne avatar Apr 24 '22 16:04 LukeSerne

Linked to #1136

wtdcode avatar Apr 26 '22 22:04 wtdcode

The error is easy to reproduce. It seems unnmap does not consider the address split for mmio_cbs, https://github.com/qilingframework/qiling/blob/dev/qiling/os/memory.py#L422-L423 This should be easy to fix for now but it warns us that keep consistency of address ranges between uc and QlMem is error-prone. My refactoring of QlMem does not handle MMIO as well since actually I know very little about it and unicorn does not have mmio_map_ptr. After reading the mmio related code, I think mmio_cbs can be removed since the key (addr, addr+size) is the same as (start, end) in map_info. So instead of bothering with address range in two structures, we do it all in map_info by replacing is_mmio: bool with mmio: (read_cb, write_cb) and just check the tuple == (None, None)

chinggg avatar Oct 08 '22 21:10 chinggg