Re: [PATCH 1/2] tools/ocaml: Fix crash in Xenctrl.domain_getinfo{,list} on ARM

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 29/07/2026 12:46 pm, Teddy Astie wrote:
> Le 28/07/2026 à 17:49, Andrew Cooper a écrit :
>> The Store_field(result, 16, arch_config) sits inside an ifdef x86,
>> meaning
>> that on other archtiectures the pointer is not filled in.  The Ocaml
>> runtime
>> then falls over a NULL pointer (really the Val_unit used to initialise
>> 'result') when the layout in the heap doesn't match the type system.
>>
>> Rearrange alloc_domaininfo() to avoid this.  Similarly to
>> physinfo_arch_caps(), raise an exception if the architecture code hasn't
>> filled in an appropriate tag.  Move the setup of arch_domainconfig to be
>> common logic.
>>
>> In order to simplify the addition of other architectures, remove the
>> arch_config variable (resuing tmp as it's touched exactly once), and
>> rename
>> x86_arch_config to be arch_config so each architecture can fill in a
>> suitable
>> one without needing more local variables.
>>
>> Reported-by: Julian Vetter <julian.vetter@xxxxxxxxxx>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> Reviewed-by: Teddy Astie <teddy.astie@xxxxxxxxxx> 

Thanks.

> I guess that requires the OCaml user side to be appropriately patched
> to not get data in the wrong order ?

I don't quite understand the question.  There are no changes needed to
the OCaml side.  The problem is that this piece of C is not producing an
object that the type system describes.

Prior to this patch, Xenctrl.domain_getinfo on ARM produces a malformed
object with a missing (NULL-ish) interior pointer.  Attempts to
interpret this object in Ocaml code segfault.

With this patch, Xenctrl.domain_getinfo on ARM will unconditionally
raise Failure.  Ocaml code won't segfault, but the overall behaviour
isn't helpful.

With patch 2, Xenctrl.domain_getinfo on ARM gets you back a good object.

~Andrew