Re: [BUG] Potential double-free in Xen dt-overlay attach/remove error path

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/04/2026 11:06 am, Nicola Vetrini wrote:
> On 2026-04-10 08:31, Jan Beulich wrote:
>> On 09.04.2026 23:28, Gyujeong Jin wrote:
>>> Hello Team, I was advised to report this issue in this way because
>>> dt-overlay is currently experimental and not security supported.
>>>
>>> I would like to report a potential memory safety issue in Xen
>>> related to
>>> the Device Tree overlay handling logic.
>>> ------------------------------
>>> Problem Description
>>>
>>> A double-free / use-after-free condition may occur in the dt-overlay
>>> handling path when an overlay attachment fails and the same overlay is
>>> later removed.
>>>
>>> The issue arises because rangeset objects are freed on the failure
>>> path of
>>> handle_attach_overlay_nodes(), but the corresponding pointers are not
>>> cleared. Subsequently, handle_remove_overlay_nodes() may operate on
>>> these
>>> stale pointers, leading to a second free.
>>> Affected Component
>>>
>>>    - Xen ARM
>>>    - Device Tree overlay subsystem
>>>    - File: xen/common/device-tree/dt-overlay.c
>>>
>>> Relevant functions:
>>>
>>>    - handle_attach_overlay_nodes()
>>>    - handle_remove_overlay_nodes()
>>>
>>> Impact
>>>
>>> This issue may lead to:
>>>
>>>    - Double-free of rangeset structures
>>>    - Use-after-free when accessing stale pointers
>>>    - Potential hypervisor crash (DoS)
>>>    - Possible memory corruption depending on allocator behavior
>>>
>>> Given that this occurs in the hypervisor context, the impact could
>>> extend
>>> beyond a simple crash under certain conditions.
>>> Root Cause
>>>
>>> The issue originates from inconsistent memory management between the
>>> attach
>>> failure path and the remove path.
>>>
>>> In handle_attach_overlay_nodes(), the failure path frees rangeset
>>> objects:
>>>
>>> static long handle_attach_overlay_nodes(...)
>>> {
>>>     ...
>>>
>>>     if ( entry )
>>>     {
>>>         rangeset_destroy(entry->irq_ranges);
>>>         rangeset_destroy(entry->iomem_ranges);
>>>     }
>>>
>>>     return rc;
>>> }
>>>
>>> However, the corresponding pointers (entry->irq_ranges and
>>> entry->iomem_ranges) are not set to NULL afterward, leaving dangling
>>> pointers in the entry structure.
>>
>> Further to this, am I overlooking any check preventing an already
>> created
>> pair of rangesets to be replaced by new ones, leaking the original pair?
>>
>> And then there's a Misra issue as well: dt_overlay_domctl() has
>> unreachable
>> code. Anything other than XEN_DOMCTL_DT_OVERLAY_ATTACH is excluded at
>> the
>> top, so the "else" body near the bottom is unreachable. (This in turn
>> makes
>> me wonder: How come there's no "detach"?) Yet then, that's probably
>> pretty
>> meaningless, as there look to be other issues (Misra and general
>> robustness
>> ones) as well.
>>
>> Jan
>
> Is it by any chance enabled in *-allcode analyses? I don't see such
> reports for unreachable code on ARM64.
>

eclair-ARM64-allcode:
    ...
    EXTRA_XEN_CONFIG: |
        ...
        CONFIG_OVERLAY_DTB=y

Seems to be, yes.

~Andrew