On 2025/11/07 2:50, Peter Xu wrote:
On Thu, Nov 06, 2025 at 11:23:32AM +0900, Akihiko Odaki wrote:
Generally speaking, we will not necessarily "always" get an issue report
when things went wrong with memory management. A bug in memory management
may not cause an immediate crash but corrupt the memory state which you will
find only later. The end result of memory corruption may look random and
result in a hard-to-debug issue report. A user may not even bother writing
an issue report at all; this is especially true for this kind of corner
cases that rarely happen.
There should have been no such a hazard of memory corruption if the code did
exactly what the documentation said in the first place. The consistency of
the code and the documentation is essential, especially for this kind of
complex and fundamental code.
Do you have encountered such case before?
I wasn't expecting that, because what you were saying looks more like what
Linux kernel would have a bug in mm. QEMU is still special as it has the
default unassigned MR trapping everything by default, meanwhile normally
what is moving is MMIO / alias regions rather than real ramblocks. It
means when things go wrong we have much higher chance to trap them
properly.
When I said "memory management" I meant the methods we use to allocate
and free memory (the Linux equivalents would be kmalloc()/free()/kref),
not the MM tracking or unassigned MR trapping behavior you mentioned.
The unassigned MR trap and MMIO/alias movement are a separate concern
and don’t change the underlying risk.
Concrete example: imagine an alias is allocated with g_new() and freed
immediately after object_unparent(). If that alias accidentally becomes
the FlatView root, destroying the FlatView later will call
memory_region_unref() and produce a use-after-free. We cannot predict
what memory_region_unref() will read or write in that scenario — the
result can be arbitrary memory corruption that surfaces much later as a
hard-to-debug, intermittent problem. Users often won’t file an issue for
these rare corner cases.
I also confess though that I'm pretty conservative on fixing things with
hypothetical issues. In general, I prefer fixing things with explicit
problems, so we know how to measure and justify a fix (depending on how
aggressive the fix is and how much maintanence burden it will bring to
QEMU). Without a real problem, it's harder to quantify that even if such
evaluation will also normally be subjective too.
Regarding your preference to fix only explicit problems: I understand
the conservatism, but here are the facts we need to weigh:
- The documentation claims we may free aliases because
memory_region_ref() is never called, yet there is code that does call
memory_region_ref().
- The patch adds code to align behavior with the documentation.
The significance of both potential impacts (the behavioral divergence
for devices other than pci-bridge, and the added complexity needed for
consistency) may be subjective and hypothetical, but that applies
equally to both sides.
In this case, the long-term reliability and maintainability of QEMU
depend on having the code behave as documented. Correctness should take
precedence over simplicity.
Regards,
Akihiko Odaki