[PATCH v1 0/3] UBSAN fixes

[PATCH v1 0/3] UBSAN fixes

[Oleksii Kurochko]

During Baptiste Le Duc's work on adding CI stuff for RISC-V several UBSAN
issues were found.

This patch series resolves the found issues.

CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2536470604

Oleksii Kurochko (3):
  xen/riscv: fix switch_stack_and_jump()
  xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()
  xen/libfdt: fix UBSAN null pointer in fdt_property()

 xen/arch/riscv/include/asm/current.h | 10 +++++-----
 xen/common/domain.c                  |  2 +-
 xen/common/libfdt/fdt_sw.c           |  3 ++-
 3 files changed, 8 insertions(+), 7 deletions(-)

-- 
2.54.0

[PATCH v1 1/3] xen/riscv: fix switch_stack_and_jump()

[Oleksii Kurochko]

The following compilation issue occurs when UBSAN related stuff is enabled:
prelink.o: in function `smp_processor_id':
  /build/xen/./arch/riscv/include/asm/current.h:46:(.init.text+0x274e2):
  relocation truncated to fit: R_RISCV_JAL against `init_done'
make[2]: *** [arch/riscv/Makefile:45: xen-syms] Error 1

The switch_stack_and_jump macro uses "j " #fn which assembles to
JAL x0, init_done is a RISC-V J-type instruction with only ±1MB range.

Without UBSAN, .init.text is small enough that init_done (which lives in
.text, not .init.text) is within 1MB of the JAL. With UBSAN enabled, all
the instrumentation calls bloat .init.text well past 1MB, so init_done
is now >1MB away from the JAL. The linker tries to truncate the 20-bit
J-type offset and fails.

The linker confusingly attributes the error to smp_processor_id:46
because the compiler inlines that function into the same init function
that ends with switch_stack_and_jump, and the debug info places the
JAL within that inlined scope.

Note that the `tail` instruction looks more natural here, but the `jr`
instruction is chosen instead to avoid depending on how the assembler
expands the `tail` instruction and which register it uses as a scratch
area (`t1` in the case of GAS), which would then need to be listed in
the clobber section of `asm volatile`.

Fixes: e66003e7be199 ("xen/riscv: introduce setup_initial_pages")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
 xen/arch/riscv/include/asm/current.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/riscv/include/asm/current.h b/xen/arch/riscv/include/asm/current.h
index 5fbee8182caa..cc004670d18c 100644
--- a/xen/arch/riscv/include/asm/current.h
+++ b/xen/arch/riscv/include/asm/current.h
@@ -51,11 +51,11 @@ DECLARE_PER_CPU(struct vcpu *, curr_vcpu);
 #define vcpu_guest_cpu_user_regs(vcpu) \
     (&(vcpu)->arch.cpu_info->guest_cpu_user_regs)
 
-#define switch_stack_and_jump(stack, fn) do {               \
-    asm volatile (                                          \
-            "mv sp, %0\n"                                   \
-            "j " #fn :: "r" (stack), "X" (fn) : "memory" ); \
-    unreachable();                                          \
+#define switch_stack_and_jump(stack, fn) do {                    \
+    asm volatile (                                               \
+            "mv sp, %0\n"                                        \
+            "jr %1" :: "r" (stack), "r" (fn) : "memory" );       \
+    unreachable();                                               \
 } while ( false )
 
 #define get_per_cpu_offset() __per_cpu_offset[smp_processor_id()]
-- 
2.54.0

[PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
and falls back to dummy_vcpu_info for vcpus beyond that limit.

However, it does not guard against d->shared_info being NULL.  The
shared_info() macro expands to a member access through d->shared_info,
so when an architecture does not allocate a shared_info page the
dereference triggers UBSAN:
  UBSAN: Undefined behaviour in common/domain.c:325:10
  member access within null pointer of type 'struct shared_info_t'

Extend the existing fallback condition to also cover the case where no
shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
instead. This is the correct behaviour: dummy_vcpu_info already serves
as the safe stand-in for vcpus that have no usable shared_info slot.

Fixes: 295514ff75506 ("common: convert vCPU info area registration")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
RISC-V does not allocate a shared_info page at the momemnt because its
guests run in dom0less mode and do not use the Xen PV ABI, so
d->shared_info remains NULL throughout domain lifetime.
---
---
 xen/common/domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index bb9e210c2895..e64b7df9b704 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -320,7 +320,7 @@ void vcpu_info_reset(struct vcpu *v)
     struct domain *d = v->domain;
 
     v->vcpu_info_area.map =
-        ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS)
+        ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS && d->shared_info)
          ? (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id])
          : &dummy_vcpu_info);
 }
-- 
2.54.0

[PATCH v1 3/3] xen/libfdt: fix UBSAN null pointer in fdt_property()

[Oleksii Kurochko]

fdt_property() unconditionally calls memcpy(ptr, val, len) even when
len is zero and val is NULL.  This is a legitimate calling convention
for adding empty FDT properties such as "interrupt-controller", which
carry no payload.

In Xen, memcpy() maps to __builtin_memcpy(). The compiler treats
__builtin_memcpy as nonnull on its pointer arguments, so UBSAN fires
before it can observe that len is zero:
  UBSAN: Undefined behaviour in common/libfdt/fdt_sw.c:333:2
         null pointer passed as argument 2, declared with nonnull
         attribute

Guard the memcpy() with a check on len so it is skipped entirely when
there is no payload to copy, bringing the code in line with the
nonnull contract.

Fixes: f0ea06558068 ("libfdt: add version 1.3.0")
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
---
 xen/common/libfdt/fdt_sw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/common/libfdt/fdt_sw.c b/xen/common/libfdt/fdt_sw.c
index 4c569ee7eb0d..96d4cf571319 100644
--- a/xen/common/libfdt/fdt_sw.c
+++ b/xen/common/libfdt/fdt_sw.c
@@ -330,7 +330,8 @@ int fdt_property(void *fdt, const char *name, const void *val, int len)
 	ret = fdt_property_placeholder(fdt, name, len, &ptr);
 	if (ret)
 		return ret;
-	memcpy(ptr, val, len);
+	if (len)
+		memcpy(ptr, val, len);
 	return 0;
 }
 
-- 
2.54.0

Re: [PATCH v1 3/3] xen/libfdt: fix UBSAN null pointer in fdt_property()

[Orzel, Michal]

Hi Oleksii,

We treat libfdt as external library and we don't accept any edits here prior to
first sending a fix to libfdt and then cherry-picking a patch (in fact, afacit
we then do the libfdt version update).

~Michal

On 19-May-26 10:39, Oleksii Kurochko wrote:
> fdt_property() unconditionally calls memcpy(ptr, val, len) even when
> len is zero and val is NULL.  This is a legitimate calling convention
> for adding empty FDT properties such as "interrupt-controller", which
> carry no payload.
> 
> In Xen, memcpy() maps to __builtin_memcpy(). The compiler treats
> __builtin_memcpy as nonnull on its pointer arguments, so UBSAN fires
> before it can observe that len is zero:
>   UBSAN: Undefined behaviour in common/libfdt/fdt_sw.c:333:2
>          null pointer passed as argument 2, declared with nonnull
>          attribute
> 
> Guard the memcpy() with a check on len so it is skipped entirely when
> there is no payload to copy, bringing the code in line with the
> nonnull contract.
> 
> Fixes: f0ea06558068 ("libfdt: add version 1.3.0")
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> ---
>  xen/common/libfdt/fdt_sw.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/libfdt/fdt_sw.c b/xen/common/libfdt/fdt_sw.c
> index 4c569ee7eb0d..96d4cf571319 100644
> --- a/xen/common/libfdt/fdt_sw.c
> +++ b/xen/common/libfdt/fdt_sw.c
> @@ -330,7 +330,8 @@ int fdt_property(void *fdt, const char *name, const void *val, int len)
>  	ret = fdt_property_placeholder(fdt, name, len, &ptr);
>  	if (ret)
>  		return ret;
> -	memcpy(ptr, val, len);
> +	if (len)
> +		memcpy(ptr, val, len);
>  	return 0;
>  }
>  

Re: [PATCH v1 3/3] xen/libfdt: fix UBSAN null pointer in fdt_property()

[Oleksii Kurochko]

Hi Michal,

On 5/19/26 10:49 AM, Orzel, Michal wrote:
> Hi Oleksii,
> 
> We treat libfdt as external library and we don't accept any edits here prior to
> first sending a fix to libfdt and then cherry-picking a patch (in fact, afacit
> we then do the libfdt version update).

Thanks for clarifying that.

Just to be sure I don't confuse something.
According to the commit ...:

commit ad9cf6bde5b90d4c1e5a79a2803e98d6344c27d7
Author: Vikram Garhwal <fnu.vikram@xilinx.com>
Date:   Thu Nov 11 23:27:20 2021 -0800

     Update libfdt to v1.6.1

     Update libfdt to v1.6.1 of libfdt taken from 
git://github.com/dgibson/dtc.
     This update is done to support device tree overlays.

... I have to send this patch to git://github.com/dgibson/dtc, right?

~ Oleksii
> On 19-May-26 10:39, Oleksii Kurochko wrote:
>> fdt_property() unconditionally calls memcpy(ptr, val, len) even when
>> len is zero and val is NULL.  This is a legitimate calling convention
>> for adding empty FDT properties such as "interrupt-controller", which
>> carry no payload.
>>
>> In Xen, memcpy() maps to __builtin_memcpy(). The compiler treats
>> __builtin_memcpy as nonnull on its pointer arguments, so UBSAN fires
>> before it can observe that len is zero:
>>    UBSAN: Undefined behaviour in common/libfdt/fdt_sw.c:333:2
>>           null pointer passed as argument 2, declared with nonnull
>>           attribute
>>
>> Guard the memcpy() with a check on len so it is skipped entirely when
>> there is no payload to copy, bringing the code in line with the
>> nonnull contract.
>>
>> Fixes: f0ea06558068 ("libfdt: add version 1.3.0")
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>> ---
>>   xen/common/libfdt/fdt_sw.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/common/libfdt/fdt_sw.c b/xen/common/libfdt/fdt_sw.c
>> index 4c569ee7eb0d..96d4cf571319 100644
>> --- a/xen/common/libfdt/fdt_sw.c
>> +++ b/xen/common/libfdt/fdt_sw.c
>> @@ -330,7 +330,8 @@ int fdt_property(void *fdt, const char *name, const void *val, int len)
>>   	ret = fdt_property_placeholder(fdt, name, len, &ptr);
>>   	if (ret)
>>   		return ret;
>> -	memcpy(ptr, val, len);
>> +	if (len)
>> +		memcpy(ptr, val, len);
>>   	return 0;
>>   }
>>   
> 

Re: [PATCH v1 1/3] xen/riscv: fix switch_stack_and_jump()

[Jan Beulich]

On 19.05.2026 10:39, Oleksii Kurochko wrote:
> The following compilation issue occurs when UBSAN related stuff is enabled:
> prelink.o: in function `smp_processor_id':
>   /build/xen/./arch/riscv/include/asm/current.h:46:(.init.text+0x274e2):
>   relocation truncated to fit: R_RISCV_JAL against `init_done'
> make[2]: *** [arch/riscv/Makefile:45: xen-syms] Error 1

There's no init_done() as of yet.

> The switch_stack_and_jump macro uses "j " #fn which assembles to
> JAL x0, init_done is a RISC-V J-type instruction with only ±1MB range.
> 
> Without UBSAN, .init.text is small enough that init_done (which lives in
> .text, not .init.text) is within 1MB of the JAL. With UBSAN enabled, all
> the instrumentation calls bloat .init.text well past 1MB, so init_done
> is now >1MB away from the JAL. The linker tries to truncate the 20-bit
> J-type offset and fails.

.init.text is well below 64k right now. Are you telling us that it grows
by more than a factor of 16 when UBSAN is enabled? IOW while the change
may indeed be needed, I question this explanation. .text growth may matter
as well, and e.g. .rodata (living between both sections) might also grow.

Jan

Re: [PATCH v1 3/3] xen/libfdt: fix UBSAN null pointer in fdt_property()

[Orzel, Michal]

On 19-May-26 11:16, Oleksii Kurochko wrote:
> Hi Michal,
> 
> On 5/19/26 10:49 AM, Orzel, Michal wrote:
>> Hi Oleksii,
>>
>> We treat libfdt as external library and we don't accept any edits here prior to
>> first sending a fix to libfdt and then cherry-picking a patch (in fact, afacit
>> we then do the libfdt version update).
> 
> Thanks for clarifying that.
> 
> Just to be sure I don't confuse something.
> According to the commit ...:
> 
> commit ad9cf6bde5b90d4c1e5a79a2803e98d6344c27d7
> Author: Vikram Garhwal <fnu.vikram@xilinx.com>
> Date:   Thu Nov 11 23:27:20 2021 -0800
> 
>      Update libfdt to v1.6.1
> 
>      Update libfdt to v1.6.1 of libfdt taken from 
> git://github.com/dgibson/dtc.
>      This update is done to support device tree overlays.
> 
> ... I have to send this patch to git://github.com/dgibson/dtc, right?
Yes, that's the main DTC/libfdt repository.

~Michal

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Jan Beulich]

On 19.05.2026 10:39, Oleksii Kurochko wrote:
> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
> and falls back to dummy_vcpu_info for vcpus beyond that limit.
> 
> However, it does not guard against d->shared_info being NULL.  The
> shared_info() macro expands to a member access through d->shared_info,
> so when an architecture does not allocate a shared_info page the
> dereference triggers UBSAN:
>   UBSAN: Undefined behaviour in common/domain.c:325:10
>   member access within null pointer of type 'struct shared_info_t'
> 
> Extend the existing fallback condition to also cover the case where no
> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
> instead. This is the correct behaviour: dummy_vcpu_info already serves
> as the safe stand-in for vcpus that have no usable shared_info slot.
> 
> Fixes: 295514ff75506 ("common: convert vCPU info area registration")

I question this, largely (but not only) because I also ...

> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
> ---
> RISC-V does not allocate a shared_info page at the momemnt because its
> guests run in dom0less mode and do not use the Xen PV ABI, so
> d->shared_info remains NULL throughout domain lifetime.

... question this mode of operation. Yes, you may (for now) be able to get
away without, but e.g. event channels will want supporting at some point.
Which will require a shared info page. Better put that in place right away,
even if the guests you test with don't use it (yet). Certain other common
code also assumes d->shared_info to never be NULL for an alive domain.

Jan

Re: [PATCH v1 1/3] xen/riscv: fix switch_stack_and_jump()

[Oleksii Kurochko]

On 5/19/26 11:28 AM, Jan Beulich wrote:
> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>> The following compilation issue occurs when UBSAN related stuff is enabled:
>> prelink.o: in function `smp_processor_id':
>>    /build/xen/./arch/riscv/include/asm/current.h:46:(.init.text+0x274e2):
>>    relocation truncated to fit: R_RISCV_JAL against `init_done'
>> make[2]: *** [arch/riscv/Makefile:45: xen-syms] Error 1
> 
> There's no init_done() as of yet.

It was found based on downstream version of RISC-V port.

> 
>> The switch_stack_and_jump macro uses "j " #fn which assembles to
>> JAL x0, init_done is a RISC-V J-type instruction with only ±1MB range.
>>
>> Without UBSAN, .init.text is small enough that init_done (which lives in
>> .text, not .init.text) is within 1MB of the JAL. With UBSAN enabled, all
>> the instrumentation calls bloat .init.text well past 1MB, so init_done
>> is now >1MB away from the JAL. The linker tries to truncate the 20-bit
>> J-type offset and fails.
> 
> .init.text is well below 64k right now. Are you telling us that it grows
> by more than a factor of 16 when UBSAN is enabled? IOW while the change
> may indeed be needed, I question this explanation. .text growth may matter
> as well, and e.g. .rodata (living between both sections) might also grow.

No, it won't grow so much.

With UBSAN enabled:

$ objdump -h xen/prelink.o

xen/prelink.o:     file format elf64-little

Sections:
Idx Name          Size      VMA               LMA               File off
   0 .text         0011c79e  0000000000000000  0000000000000000  00000040
                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
   1 .init.text    000285fe  0000000000000000  0000000000000000  0011c7de
                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE

With UBSAN, .text itself is 0x11c79e ≈ 1.11 MiB — already exceeding the 
JAL range on its own. Even if .init.text directly followed .text (which 
it doesn't), a call from .init.text to a symbol near the start of .text 
would be ~1.11 MiB away. init_done likely sits somewhere specific within 
.text rather than at its very end, but add the .rodata + .data sections 
on top and the gap is comfortably past ±1 MiB.

As a result, the target symbol init_done may end up outside the range 
supported by the R_RISCV_JAL relocation, which is limited to 
approximately ±1 MiB.

Without UBSAN enabled:

xen/prelink.o:     file format elf64-little

Sections:
Idx Name          Size      VMA               LMA               File off
   0 .text         00044618  0000000000000000  0000000000000000  00000040
                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
   1 .init.text    00012c72  0000000000000000  0000000000000000  00044658
                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE

Does it make sense now? I can use the text above for commit message 
instead of what is mentioned now for more accuracy.

Would it be better to send this patch when this issue will occur in 
upstream?

~ Oleksii

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/19/26 11:37 AM, Jan Beulich wrote:
> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>
>> However, it does not guard against d->shared_info being NULL.  The
>> shared_info() macro expands to a member access through d->shared_info,
>> so when an architecture does not allocate a shared_info page the
>> dereference triggers UBSAN:
>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>    member access within null pointer of type 'struct shared_info_t'
>>
>> Extend the existing fallback condition to also cover the case where no
>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>
>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
> 
> I question this, largely (but not only) because I also ...
> 
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>> ---
>> RISC-V does not allocate a shared_info page at the momemnt because its
>> guests run in dom0less mode and do not use the Xen PV ABI, so
>> d->shared_info remains NULL throughout domain lifetime.
> 
> ... question this mode of operation. Yes, you may (for now) be able to get
> away without, but e.g. event channels will want supporting at some point.
> Which will require a shared info page. Better put that in place right away,
> even if the guests you test with don't use it (yet). Certain other common
> code also assumes d->shared_info to never be NULL for an alive domain.
> 

Would it be fine than to allocate it in arch_domain_create() ... :

     if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
         goto fail;

     clear_page(d->shared_info);

... but without calling share_xen_page_with_guest() after that 
allocation as share_xen_page_with_guest() isn't implemented at the moment?

Thanks.

~ Oleksii

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
> 
> 
> On 5/19/26 11:37 AM, Jan Beulich wrote:
>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>
>>> However, it does not guard against d->shared_info being NULL.  The
>>> shared_info() macro expands to a member access through d->shared_info,
>>> so when an architecture does not allocate a shared_info page the
>>> dereference triggers UBSAN:
>>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>>    member access within null pointer of type 'struct shared_info_t'
>>>
>>> Extend the existing fallback condition to also cover the case where no
>>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>
>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>
>> I question this, largely (but not only) because I also ...
>>
>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>> ---
>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>> d->shared_info remains NULL throughout domain lifetime.
>>
>> ... question this mode of operation. Yes, you may (for now) be able to 
>> get
>> away without, but e.g. event channels will want supporting at some point.
>> Which will require a shared info page. Better put that in place right 
>> away,
>> even if the guests you test with don't use it (yet). Certain other common
>> code also assumes d->shared_info to never be NULL for an alive domain.
>>
> 
> Would it be fine than to allocate it in arch_domain_create() ... :
> 
>      if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>          goto fail;
> 
>      clear_page(d->shared_info);
> 
> ... but without calling share_xen_page_with_guest() after that 
> allocation as share_xen_page_with_guest() isn't implemented at the moment?

Or could it be an option for all arch-s move allocation of 
d->shared_info to domain_create() in common just after arch_domain_create()?

The only question if share_xen_page_with_guest() could be ifdef-ed 
somehow so not to block new ports to implement it from the start.

~ Oleksii

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Andrew Cooper]

On 19/05/2026 12:22 pm, Oleksii Kurochko wrote:
>
>
> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>
>>
>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot
>>>> inside
>>>> the domain's shared_info page for vcpus with id <
>>>> XEN_LEGACY_MAX_VCPUS,
>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>
>>>> However, it does not guard against d->shared_info being NULL.  The
>>>> shared_info() macro expands to a member access through d->shared_info,
>>>> so when an architecture does not allocate a shared_info page the
>>>> dereference triggers UBSAN:
>>>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>    member access within null pointer of type 'struct shared_info_t'
>>>>
>>>> Extend the existing fallback condition to also cover the case where no
>>>> shared_info page has been allocated, mapping the vcpu to
>>>> dummy_vcpu_info
>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>
>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>
>>> I question this, largely (but not only) because I also ...
>>>
>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>> ---
>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>> d->shared_info remains NULL throughout domain lifetime.
>>>
>>> ... question this mode of operation. Yes, you may (for now) be able
>>> to get
>>> away without, but e.g. event channels will want supporting at some
>>> point.
>>> Which will require a shared info page. Better put that in place
>>> right away,
>>> even if the guests you test with don't use it (yet). Certain other
>>> common
>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>
>>
>> Would it be fine than to allocate it in arch_domain_create() ... :
>>
>>      if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>          goto fail;
>>
>>      clear_page(d->shared_info);
>>
>> ... but without calling share_xen_page_with_guest() after that
>> allocation as share_xen_page_with_guest() isn't implemented at the
>> moment?
>
> Or could it be an option for all arch-s move allocation of
> d->shared_info to domain_create() in common just after
> arch_domain_create()?
>
> The only question if share_xen_page_with_guest() could be ifdef-ed
> somehow so not to block new ports to implement it from the start.

shared_info is an x86-PV-ism which escaped into HVM and then infected
ARM too.

Sadly it's ABI there, but this is one of many areas where I really want
RISC-V not to inherit the mistakes of prior ports.

~Andrew

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/19/26 1:32 PM, Andrew Cooper wrote:
> On 19/05/2026 12:22 pm, Oleksii Kurochko wrote:
>>
>>
>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>
>>>
>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot
>>>>> inside
>>>>> the domain's shared_info page for vcpus with id <
>>>>> XEN_LEGACY_MAX_VCPUS,
>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>
>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>> so when an architecture does not allocate a shared_info page the
>>>>> dereference triggers UBSAN:
>>>>>     UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>     member access within null pointer of type 'struct shared_info_t'
>>>>>
>>>>> Extend the existing fallback condition to also cover the case where no
>>>>> shared_info page has been allocated, mapping the vcpu to
>>>>> dummy_vcpu_info
>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>
>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>
>>>> I question this, largely (but not only) because I also ...
>>>>
>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>> ---
>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>
>>>> ... question this mode of operation. Yes, you may (for now) be able
>>>> to get
>>>> away without, but e.g. event channels will want supporting at some
>>>> point.
>>>> Which will require a shared info page. Better put that in place
>>>> right away,
>>>> even if the guests you test with don't use it (yet). Certain other
>>>> common
>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>
>>>
>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>
>>>       if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>           goto fail;
>>>
>>>       clear_page(d->shared_info);
>>>
>>> ... but without calling share_xen_page_with_guest() after that
>>> allocation as share_xen_page_with_guest() isn't implemented at the
>>> moment?
>>
>> Or could it be an option for all arch-s move allocation of
>> d->shared_info to domain_create() in common just after
>> arch_domain_create()?
>>
>> The only question if share_xen_page_with_guest() could be ifdef-ed
>> somehow so not to block new ports to implement it from the start.
> 
> shared_info is an x86-PV-ism which escaped into HVM and then infected
> ARM too.
> 
> Sadly it's ABI there, but this is one of many areas where I really want
> RISC-V not to inherit the mistakes of prior ports.
> 

Could you please clarify what could be done better now? It seems like 
shared_info is used in a lot of places in common code base and it is 
needed for event channel stuff for example.

~ Oleksii

Re: [PATCH v1 1/3] xen/riscv: fix switch_stack_and_jump()

[Jan Beulich]

On 19.05.2026 12:50, Oleksii Kurochko wrote:
> 
> 
> On 5/19/26 11:28 AM, Jan Beulich wrote:
>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>> The following compilation issue occurs when UBSAN related stuff is enabled:
>>> prelink.o: in function `smp_processor_id':
>>>    /build/xen/./arch/riscv/include/asm/current.h:46:(.init.text+0x274e2):
>>>    relocation truncated to fit: R_RISCV_JAL against `init_done'
>>> make[2]: *** [arch/riscv/Makefile:45: xen-syms] Error 1
>>
>> There's no init_done() as of yet.
> 
> It was found based on downstream version of RISC-V port.
> 
>>
>>> The switch_stack_and_jump macro uses "j " #fn which assembles to
>>> JAL x0, init_done is a RISC-V J-type instruction with only ±1MB range.
>>>
>>> Without UBSAN, .init.text is small enough that init_done (which lives in
>>> .text, not .init.text) is within 1MB of the JAL. With UBSAN enabled, all
>>> the instrumentation calls bloat .init.text well past 1MB, so init_done
>>> is now >1MB away from the JAL. The linker tries to truncate the 20-bit
>>> J-type offset and fails.
>>
>> .init.text is well below 64k right now. Are you telling us that it grows
>> by more than a factor of 16 when UBSAN is enabled? IOW while the change
>> may indeed be needed, I question this explanation. .text growth may matter
>> as well, and e.g. .rodata (living between both sections) might also grow.
> 
> No, it won't grow so much.
> 
> With UBSAN enabled:
> 
> $ objdump -h xen/prelink.o
> 
> xen/prelink.o:     file format elf64-little
> 
> Sections:
> Idx Name          Size      VMA               LMA               File off
>    0 .text         0011c79e  0000000000000000  0000000000000000  00000040
>                    CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
>    1 .init.text    000285fe  0000000000000000  0000000000000000  0011c7de
>                    CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> 
> With UBSAN, .text itself is 0x11c79e ≈ 1.11 MiB — already exceeding the 
> JAL range on its own. Even if .init.text directly followed .text (which 
> it doesn't), a call from .init.text to a symbol near the start of .text 
> would be ~1.11 MiB away. init_done likely sits somewhere specific within 
> .text rather than at its very end, but add the .rodata + .data sections 
> on top and the gap is comfortably past ±1 MiB.
> 
> As a result, the target symbol init_done may end up outside the range 
> supported by the R_RISCV_JAL relocation, which is limited to 
> approximately ±1 MiB.
> 
> Without UBSAN enabled:
> 
> xen/prelink.o:     file format elf64-little
> 
> Sections:
> Idx Name          Size      VMA               LMA               File off
>    0 .text         00044618  0000000000000000  0000000000000000  00000040
>                    CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
>    1 .init.text    00012c72  0000000000000000  0000000000000000  00044658
>                    CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
> 
> Does it make sense now? I can use the text above for commit message 
> instead of what is mentioned now for more accuracy.
> 
> Would it be better to send this patch when this issue will occur in 
> upstream?

Having the change right away is fine, but the description needs to match
what's presently upstream (i.e. mention any non-upstream aspect as such).

Jan

Re: [PATCH v1 1/3] xen/riscv: fix switch_stack_and_jump()

[Andrew Cooper]

On 19/05/2026 11:50 am, Oleksii Kurochko wrote:
>
>
> On 5/19/26 11:28 AM, Jan Beulich wrote:
>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>> The following compilation issue occurs when UBSAN related stuff is
>>> enabled:
>>> prelink.o: in function `smp_processor_id':
>>>   
>>> /build/xen/./arch/riscv/include/asm/current.h:46:(.init.text+0x274e2):
>>>    relocation truncated to fit: R_RISCV_JAL against `init_done'
>>> make[2]: *** [arch/riscv/Makefile:45: xen-syms] Error 1
>>
>> There's no init_done() as of yet.
>
> It was found based on downstream version of RISC-V port.
>
>>
>>> The switch_stack_and_jump macro uses "j " #fn which assembles to
>>> JAL x0, init_done is a RISC-V J-type instruction with only ±1MB range.
>>>
>>> Without UBSAN, .init.text is small enough that init_done (which
>>> lives in
>>> .text, not .init.text) is within 1MB of the JAL. With UBSAN enabled,
>>> all
>>> the instrumentation calls bloat .init.text well past 1MB, so init_done
>>> is now >1MB away from the JAL. The linker tries to truncate the 20-bit
>>> J-type offset and fails.
>>
>> .init.text is well below 64k right now. Are you telling us that it grows
>> by more than a factor of 16 when UBSAN is enabled? IOW while the change
>> may indeed be needed, I question this explanation. .text growth may
>> matter
>> as well, and e.g. .rodata (living between both sections) might also
>> grow.
>
> No, it won't grow so much.
>
> With UBSAN enabled:
>
> $ objdump -h xen/prelink.o
>
> xen/prelink.o:     file format elf64-little
>
> Sections:
> Idx Name          Size      VMA               LMA               File off
>   0 .text         0011c79e  0000000000000000  0000000000000000  00000040
>                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
>   1 .init.text    000285fe  0000000000000000  0000000000000000  0011c7de
>                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
>
> With UBSAN, .text itself is 0x11c79e ≈ 1.11 MiB — already exceeding
> the JAL range on its own. Even if .init.text directly followed .text
> (which it doesn't), a call from .init.text to a symbol near the start
> of .text would be ~1.11 MiB away. init_done likely sits somewhere
> specific within .text rather than at its very end, but add the .rodata
> + .data sections on top and the gap is comfortably past ±1 MiB.
>
> As a result, the target symbol init_done may end up outside the range
> supported by the R_RISCV_JAL relocation, which is limited to
> approximately ±1 MiB.
>
> Without UBSAN enabled:
>
> xen/prelink.o:     file format elf64-little
>
> Sections:
> Idx Name          Size      VMA               LMA               File off
>   0 .text         00044618  0000000000000000  0000000000000000  00000040
>                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
>   1 .init.text    00012c72  0000000000000000  0000000000000000  00044658
>                   CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
>
> Does it make sense now? I can use the text above for commit message
> instead of what is mentioned now for more accuracy.
>
> Would it be better to send this patch when this issue will occur in
> upstream?

You want to make the commit message less specific.

The problem here is that JAL only has a +- 1M range, and that this can
be exceeded in some configurations.

The fact it's init_done(), and indeed that it's also UBSAN, are rather
incidental.  It's useful to state once, (including "found in a
downstream branch"), but don't focus on init_done().

Also the subject should be "fix switch_stack_and_jump() for range beyond
1M" or similar.  "fix" on it's own could be one of many things.

~Andrew

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Jan Beulich]

On 19.05.2026 13:32, Andrew Cooper wrote:
> On 19/05/2026 12:22 pm, Oleksii Kurochko wrote:
>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot
>>>>> inside
>>>>> the domain's shared_info page for vcpus with id <
>>>>> XEN_LEGACY_MAX_VCPUS,
>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>
>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>> so when an architecture does not allocate a shared_info page the
>>>>> dereference triggers UBSAN:
>>>>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>    member access within null pointer of type 'struct shared_info_t'
>>>>>
>>>>> Extend the existing fallback condition to also cover the case where no
>>>>> shared_info page has been allocated, mapping the vcpu to
>>>>> dummy_vcpu_info
>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>
>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>
>>>> I question this, largely (but not only) because I also ...
>>>>
>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>> ---
>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>
>>>> ... question this mode of operation. Yes, you may (for now) be able
>>>> to get
>>>> away without, but e.g. event channels will want supporting at some
>>>> point.
>>>> Which will require a shared info page. Better put that in place
>>>> right away,
>>>> even if the guests you test with don't use it (yet). Certain other
>>>> common
>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>
>>>
>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>
>>>      if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>          goto fail;
>>>
>>>      clear_page(d->shared_info);
>>>
>>> ... but without calling share_xen_page_with_guest() after that
>>> allocation as share_xen_page_with_guest() isn't implemented at the
>>> moment?
>>
>> Or could it be an option for all arch-s move allocation of
>> d->shared_info to domain_create() in common just after
>> arch_domain_create()?
>>
>> The only question if share_xen_page_with_guest() could be ifdef-ed
>> somehow so not to block new ports to implement it from the start.
> 
> shared_info is an x86-PV-ism which escaped into HVM and then infected
> ARM too.
> 
> Sadly it's ABI there, but this is one of many areas where I really want
> RISC-V not to inherit the mistakes of prior ports.

In which case, how do you propose e.g. event channels to be handled in
whatever is going to be the alternative?

Jan

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Jan Beulich]

On 19.05.2026 13:22, Oleksii Kurochko wrote:
> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>>>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>
>>>> However, it does not guard against d->shared_info being NULL.  The
>>>> shared_info() macro expands to a member access through d->shared_info,
>>>> so when an architecture does not allocate a shared_info page the
>>>> dereference triggers UBSAN:
>>>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>    member access within null pointer of type 'struct shared_info_t'
>>>>
>>>> Extend the existing fallback condition to also cover the case where no
>>>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>
>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>
>>> I question this, largely (but not only) because I also ...
>>>
>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>> ---
>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>> d->shared_info remains NULL throughout domain lifetime.
>>>
>>> ... question this mode of operation. Yes, you may (for now) be able to 
>>> get
>>> away without, but e.g. event channels will want supporting at some point.
>>> Which will require a shared info page. Better put that in place right 
>>> away,
>>> even if the guests you test with don't use it (yet). Certain other common
>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>
>>
>> Would it be fine than to allocate it in arch_domain_create() ... :
>>
>>      if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>          goto fail;
>>
>>      clear_page(d->shared_info);
>>
>> ... but without calling share_xen_page_with_guest() after that 
>> allocation as share_xen_page_with_guest() isn't implemented at the moment?

I would have said "yes" here, but ...

> Or could it be an option for all arch-s move allocation of 
> d->shared_info to domain_create() in common just after arch_domain_create()?

... Andrew's reply pretty much rules out not only this option, but the
shared-info-page concept as a whole (for RISC-V). See my reply there. In
the meantime, the change as suggested may then indeed be what we want to
go with, albeit (a) with a better description and (b) perhaps covering
all d->shared_info uses.

Jan

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Andrew Cooper]

On 19/05/2026 12:51 pm, Jan Beulich wrote:
> On 19.05.2026 13:32, Andrew Cooper wrote:
>> On 19/05/2026 12:22 pm, Oleksii Kurochko wrote:
>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot
>>>>>> inside
>>>>>> the domain's shared_info page for vcpus with id <
>>>>>> XEN_LEGACY_MAX_VCPUS,
>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>
>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>> dereference triggers UBSAN:
>>>>>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>    member access within null pointer of type 'struct shared_info_t'
>>>>>>
>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>> shared_info page has been allocated, mapping the vcpu to
>>>>>> dummy_vcpu_info
>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>
>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>> I question this, largely (but not only) because I also ...
>>>>>
>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>> ---
>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>> ... question this mode of operation. Yes, you may (for now) be able
>>>>> to get
>>>>> away without, but e.g. event channels will want supporting at some
>>>>> point.
>>>>> Which will require a shared info page. Better put that in place
>>>>> right away,
>>>>> even if the guests you test with don't use it (yet). Certain other
>>>>> common
>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>
>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>
>>>>      if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>          goto fail;
>>>>
>>>>      clear_page(d->shared_info);
>>>>
>>>> ... but without calling share_xen_page_with_guest() after that
>>>> allocation as share_xen_page_with_guest() isn't implemented at the
>>>> moment?
>>> Or could it be an option for all arch-s move allocation of
>>> d->shared_info to domain_create() in common just after
>>> arch_domain_create()?
>>>
>>> The only question if share_xen_page_with_guest() could be ifdef-ed
>>> somehow so not to block new ports to implement it from the start.
>> shared_info is an x86-PV-ism which escaped into HVM and then infected
>> ARM too.
>>
>> Sadly it's ABI there, but this is one of many areas where I really want
>> RISC-V not to inherit the mistakes of prior ports.
> In which case, how do you propose e.g. event channels to be handled in
> whatever is going to be the alternative?

Implement proper enumeration of virtual capabilities (to be retrofitted
to x86/ARM too), and only offer the FIFO ABI (which is superior in every
way to the 2L ABI).

~Andrew

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Jan Beulich]

On 19.05.2026 13:56, Andrew Cooper wrote:
> On 19/05/2026 12:51 pm, Jan Beulich wrote:
>> On 19.05.2026 13:32, Andrew Cooper wrote:
>>> On 19/05/2026 12:22 pm, Oleksii Kurochko wrote:
>>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot
>>>>>>> inside
>>>>>>> the domain's shared_info page for vcpus with id <
>>>>>>> XEN_LEGACY_MAX_VCPUS,
>>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>>
>>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>>> dereference triggers UBSAN:
>>>>>>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>>    member access within null pointer of type 'struct shared_info_t'
>>>>>>>
>>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>>> shared_info page has been allocated, mapping the vcpu to
>>>>>>> dummy_vcpu_info
>>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>>
>>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>>> I question this, largely (but not only) because I also ...
>>>>>>
>>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>>> ---
>>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>> ... question this mode of operation. Yes, you may (for now) be able
>>>>>> to get
>>>>>> away without, but e.g. event channels will want supporting at some
>>>>>> point.
>>>>>> Which will require a shared info page. Better put that in place
>>>>>> right away,
>>>>>> even if the guests you test with don't use it (yet). Certain other
>>>>>> common
>>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>>
>>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>>
>>>>>      if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>>          goto fail;
>>>>>
>>>>>      clear_page(d->shared_info);
>>>>>
>>>>> ... but without calling share_xen_page_with_guest() after that
>>>>> allocation as share_xen_page_with_guest() isn't implemented at the
>>>>> moment?
>>>> Or could it be an option for all arch-s move allocation of
>>>> d->shared_info to domain_create() in common just after
>>>> arch_domain_create()?
>>>>
>>>> The only question if share_xen_page_with_guest() could be ifdef-ed
>>>> somehow so not to block new ports to implement it from the start.
>>> shared_info is an x86-PV-ism which escaped into HVM and then infected
>>> ARM too.
>>>
>>> Sadly it's ABI there, but this is one of many areas where I really want
>>> RISC-V not to inherit the mistakes of prior ports.
>> In which case, how do you propose e.g. event channels to be handled in
>> whatever is going to be the alternative?
> 
> Implement proper enumeration of virtual capabilities (to be retrofitted
> to x86/ARM too), and only offer the FIFO ABI (which is superior in every
> way to the 2L ABI).

What about the wc_* fields then? And about everything in struct arch_shared_info?

Jan

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Andrew Cooper]

On 19/05/2026 1:06 pm, Jan Beulich wrote:
> On 19.05.2026 13:56, Andrew Cooper wrote:
>> On 19/05/2026 12:51 pm, Jan Beulich wrote:
>>> On 19.05.2026 13:32, Andrew Cooper wrote:
>>>> On 19/05/2026 12:22 pm, Oleksii Kurochko wrote:
>>>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot
>>>>>>>> inside
>>>>>>>> the domain's shared_info page for vcpus with id <
>>>>>>>> XEN_LEGACY_MAX_VCPUS,
>>>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>>>
>>>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>>>> dereference triggers UBSAN:
>>>>>>>>    UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>>>    member access within null pointer of type 'struct shared_info_t'
>>>>>>>>
>>>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>>>> shared_info page has been allocated, mapping the vcpu to
>>>>>>>> dummy_vcpu_info
>>>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>>>
>>>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>>>> I question this, largely (but not only) because I also ...
>>>>>>>
>>>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>>>> ---
>>>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>>> ... question this mode of operation. Yes, you may (for now) be able
>>>>>>> to get
>>>>>>> away without, but e.g. event channels will want supporting at some
>>>>>>> point.
>>>>>>> Which will require a shared info page. Better put that in place
>>>>>>> right away,
>>>>>>> even if the guests you test with don't use it (yet). Certain other
>>>>>>> common
>>>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>>>
>>>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>>>
>>>>>>      if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>>>          goto fail;
>>>>>>
>>>>>>      clear_page(d->shared_info);
>>>>>>
>>>>>> ... but without calling share_xen_page_with_guest() after that
>>>>>> allocation as share_xen_page_with_guest() isn't implemented at the
>>>>>> moment?
>>>>> Or could it be an option for all arch-s move allocation of
>>>>> d->shared_info to domain_create() in common just after
>>>>> arch_domain_create()?
>>>>>
>>>>> The only question if share_xen_page_with_guest() could be ifdef-ed
>>>>> somehow so not to block new ports to implement it from the start.
>>>> shared_info is an x86-PV-ism which escaped into HVM and then infected
>>>> ARM too.
>>>>
>>>> Sadly it's ABI there, but this is one of many areas where I really want
>>>> RISC-V not to inherit the mistakes of prior ports.
>>> In which case, how do you propose e.g. event channels to be handled in
>>> whatever is going to be the alternative?
>> Implement proper enumeration of virtual capabilities (to be retrofitted
>> to x86/ARM too), and only offer the FIFO ABI (which is superior in every
>> way to the 2L ABI).
> What about the wc_* fields then?

Well - ARM seems to have the right idea by entirely ignoring them and
leaving it all 0.

At least it's obviously got no data in it, as opposed to what we do on
x86 where we pretend it's possible to use some stale value to determine
the wallclock time without even a tied TSC reference.

This is one of several things contributing to our in-guest timekeeping bugs.

>  And about everything in struct arch_shared_info?

There is nothing else in any arch_shared_info applicable outside of PV
guests.

~Andrew

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/19/26 1:56 PM, Andrew Cooper wrote:
> On 19/05/2026 12:51 pm, Jan Beulich wrote:
>> On 19.05.2026 13:32, Andrew Cooper wrote:
>>> On 19/05/2026 12:22 pm, Oleksii Kurochko wrote:
>>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot
>>>>>>> inside
>>>>>>> the domain's shared_info page for vcpus with id <
>>>>>>> XEN_LEGACY_MAX_VCPUS,
>>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>>
>>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>>> dereference triggers UBSAN:
>>>>>>>     UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>>     member access within null pointer of type 'struct shared_info_t'
>>>>>>>
>>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>>> shared_info page has been allocated, mapping the vcpu to
>>>>>>> dummy_vcpu_info
>>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>>
>>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>>> I question this, largely (but not only) because I also ...
>>>>>>
>>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>>> ---
>>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>> ... question this mode of operation. Yes, you may (for now) be able
>>>>>> to get
>>>>>> away without, but e.g. event channels will want supporting at some
>>>>>> point.
>>>>>> Which will require a shared info page. Better put that in place
>>>>>> right away,
>>>>>> even if the guests you test with don't use it (yet). Certain other
>>>>>> common
>>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>>
>>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>>
>>>>>       if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>>           goto fail;
>>>>>
>>>>>       clear_page(d->shared_info);
>>>>>
>>>>> ... but without calling share_xen_page_with_guest() after that
>>>>> allocation as share_xen_page_with_guest() isn't implemented at the
>>>>> moment?
>>>> Or could it be an option for all arch-s move allocation of
>>>> d->shared_info to domain_create() in common just after
>>>> arch_domain_create()?
>>>>
>>>> The only question if share_xen_page_with_guest() could be ifdef-ed
>>>> somehow so not to block new ports to implement it from the start.
>>> shared_info is an x86-PV-ism which escaped into HVM and then infected
>>> ARM too.
>>>
>>> Sadly it's ABI there, but this is one of many areas where I really want
>>> RISC-V not to inherit the mistakes of prior ports.
>> In which case, how do you propose e.g. event channels to be handled in
>> whatever is going to be the alternative?
> 
> Implement proper enumeration of virtual capabilities (to be retrofitted
> to x86/ARM too), and only offer the FIFO ABI (which is superior in every
> way to the 2L ABI).

I'm not familiar with the FIFO ABI, but after a quick look it seems 
d->shared_info is used here for example:
static void setup_ports(struct domain *d, unsigned int prev_evtchns)
{
...
     for ( port = 1; port < prev_evtchns; port++ )
     {
...

         evtchn = evtchn_from_port(d, port);

         if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending)) )
             evtchn->pending = true;

         evtchn_fifo_set_priority(d, evtchn, EVTCHN_FIFO_PRIORITY_DEFAULT);
     }
}

So shouldn't it be still allocated in arch_domain_create() or as I 
suggested in domain_create()?

~ Oleksii

Re: [PATCH v1 3/3] xen/libfdt: fix UBSAN null pointer in fdt_property()

[Oleksii Kurochko]

Hello Michal,

On 5/19/26 11:37 AM, Orzel, Michal wrote:
>>> We treat libfdt as external library and we don't accept any edits here prior to
>>> first sending a fix to libfdt and then cherry-picking a patch (in fact, afacit
>>> we then do the libfdt version update).
>>
>> Thanks for clarifying that.
>>
>> Just to be sure I don't confuse something.
>> According to the commit ...:
>>
>> commit ad9cf6bde5b90d4c1e5a79a2803e98d6344c27d7
>> Author: Vikram Garhwal <fnu.vikram@xilinx.com>
>> Date:   Thu Nov 11 23:27:20 2021 -0800
>>
>>       Update libfdt to v1.6.1
>>
>>       Update libfdt to v1.6.1 of libfdt taken from
>> git://github.com/dgibson/dtc.
>>       This update is done to support device tree overlays.
>>
>> ... I have to send this patch to git://github.com/dgibson/dtc, right?
> Yes, that's the main DTC/libfdt repository.

The patch to dtc repo was accepted and merged:
https://github.com/dgibson/dtc/commit/f57e7df35df4a301961cbbf9433ba4e85c2ee5ed

But current version of dtc is:
   $ cat https://github.com/dgibson/dtc/blob/main/VERSION.txt
     1.7.2

Does it make sense to backport just one patch to Xen instead of updating 
update libfdt to 1.7.2 as it would be necessary to apply 61 patch on top 
of Xen's libfdt 1.6.1:
   $ git format-patch v1.6.1..main -- libfdt/ | wc -l
     61

~ Oleksii

Re: [PATCH v1 3/3] xen/libfdt: fix UBSAN null pointer in fdt_property()

[Orzel, Michal]

On 20-May-26 09:51, Oleksii Kurochko wrote:
> Hello Michal,
> 
> On 5/19/26 11:37 AM, Orzel, Michal wrote:
>>>> We treat libfdt as external library and we don't accept any edits here prior to
>>>> first sending a fix to libfdt and then cherry-picking a patch (in fact, afacit
>>>> we then do the libfdt version update).
>>>
>>> Thanks for clarifying that.
>>>
>>> Just to be sure I don't confuse something.
>>> According to the commit ...:
>>>
>>> commit ad9cf6bde5b90d4c1e5a79a2803e98d6344c27d7
>>> Author: Vikram Garhwal <fnu.vikram@xilinx.com>
>>> Date:   Thu Nov 11 23:27:20 2021 -0800
>>>
>>>       Update libfdt to v1.6.1
>>>
>>>       Update libfdt to v1.6.1 of libfdt taken from
>>> git://github.com/dgibson/dtc.
>>>       This update is done to support device tree overlays.
>>>
>>> ... I have to send this patch to git://github.com/dgibson/dtc, right?
>> Yes, that's the main DTC/libfdt repository.
> 
> The patch to dtc repo was accepted and merged:
> https://github.com/dgibson/dtc/commit/f57e7df35df4a301961cbbf9433ba4e85c2ee5ed
> 
> But current version of dtc is:
>    $ cat https://github.com/dgibson/dtc/blob/main/VERSION.txt
>      1.7.2
> 
> Does it make sense to backport just one patch to Xen instead of updating 
We can backport this patch only the same way as for example here:
d434dc4ac182 ("libfdt: overlay: change overlay_get_target()")

~Michal

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/19/26 1:53 PM, Jan Beulich wrote:
> On 19.05.2026 13:22, Oleksii Kurochko wrote:
>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>>>>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>
>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>> so when an architecture does not allocate a shared_info page the
>>>>> dereference triggers UBSAN:
>>>>>     UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>     member access within null pointer of type 'struct shared_info_t'
>>>>>
>>>>> Extend the existing fallback condition to also cover the case where no
>>>>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>
>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>
>>>> I question this, largely (but not only) because I also ...
>>>>
>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>> ---
>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>
>>>> ... question this mode of operation. Yes, you may (for now) be able to
>>>> get
>>>> away without, but e.g. event channels will want supporting at some point.
>>>> Which will require a shared info page. Better put that in place right
>>>> away,
>>>> even if the guests you test with don't use it (yet). Certain other common
>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>
>>>
>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>
>>>       if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>           goto fail;
>>>
>>>       clear_page(d->shared_info);
>>>
>>> ... but without calling share_xen_page_with_guest() after that
>>> allocation as share_xen_page_with_guest() isn't implemented at the moment?
> 
> I would have said "yes" here, but ...
> 
>> Or could it be an option for all arch-s move allocation of
>> d->shared_info to domain_create() in common just after arch_domain_create()?
> 
> ... Andrew's reply pretty much rules out not only this option, but the
> shared-info-page concept as a whole (for RISC-V). See my reply there. In
> the meantime, the change as suggested may then indeed be what we want to
> go with, albeit (a) with a better description and (b) perhaps covering
> all d->shared_info uses.

Looking at guest kernel code (Linux), FIFO is tried first, so if RISC-V 
is going to support only FIFO, d->shared_info could legally be NULL.

Looking at the Xen side, if an architecture decides to support only 
FIFO, d->shared_info is touched only in vcpu_info_reset(), which is 
called from vcpu_create().

All other places where d->shared_info is accessed should not be 
reachable except for one case in event_fifo.c: when a guest issues the 
EVTCHNOP_init_control hypercall, setup_ports() reads from shared_info(d, 
evtchn_pending):
   static void setup_ports(struct domain *d, unsigned int prev_evtchns)
   {
   ...
           if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending))
               evtchn->pending = true;
   ...
       }
   }

This looks like it handles the transition from the 2L ABI to the FIFO 
ABI: if a guest started with 2L and then switched to FIFO, any events 
already pending in shared_info(d, evtchn_pending) need to be migrated to 
FIFO's per-channel evtchn->pending flag. But it looks like I am missing 
something here as I mentioned at the start that Linux uses or FIFO or 2L.

Am I missing something?

~ Oleksii

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Jan Beulich]

On 20.05.2026 13:33, Oleksii Kurochko wrote:
> 
> 
> On 5/19/26 1:53 PM, Jan Beulich wrote:
>> On 19.05.2026 13:22, Oleksii Kurochko wrote:
>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>>>>>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>
>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>> dereference triggers UBSAN:
>>>>>>     UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>     member access within null pointer of type 'struct shared_info_t'
>>>>>>
>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>
>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>>
>>>>> I question this, largely (but not only) because I also ...
>>>>>
>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>> ---
>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>
>>>>> ... question this mode of operation. Yes, you may (for now) be able to
>>>>> get
>>>>> away without, but e.g. event channels will want supporting at some point.
>>>>> Which will require a shared info page. Better put that in place right
>>>>> away,
>>>>> even if the guests you test with don't use it (yet). Certain other common
>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>
>>>>
>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>
>>>>       if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>           goto fail;
>>>>
>>>>       clear_page(d->shared_info);
>>>>
>>>> ... but without calling share_xen_page_with_guest() after that
>>>> allocation as share_xen_page_with_guest() isn't implemented at the moment?
>>
>> I would have said "yes" here, but ...
>>
>>> Or could it be an option for all arch-s move allocation of
>>> d->shared_info to domain_create() in common just after arch_domain_create()?
>>
>> ... Andrew's reply pretty much rules out not only this option, but the
>> shared-info-page concept as a whole (for RISC-V). See my reply there. In
>> the meantime, the change as suggested may then indeed be what we want to
>> go with, albeit (a) with a better description and (b) perhaps covering
>> all d->shared_info uses.
> 
> Looking at guest kernel code (Linux), FIFO is tried first, so if RISC-V 
> is going to support only FIFO, d->shared_info could legally be NULL.
> 
> Looking at the Xen side, if an architecture decides to support only 
> FIFO, d->shared_info is touched only in vcpu_info_reset(), which is 
> called from vcpu_create().
> 
> All other places where d->shared_info is accessed should not be 
> reachable except for one case in event_fifo.c: when a guest issues the 
> EVTCHNOP_init_control hypercall, setup_ports() reads from shared_info(d, 
> evtchn_pending):
>    static void setup_ports(struct domain *d, unsigned int prev_evtchns)
>    {
>    ...
>            if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending))
>                evtchn->pending = true;
>    ...
>        }
>    }
> 
> This looks like it handles the transition from the 2L ABI to the FIFO 
> ABI: if a guest started with 2L and then switched to FIFO, any events 
> already pending in shared_info(d, evtchn_pending) need to be migrated to 
> FIFO's per-channel evtchn->pending flag. But it looks like I am missing 
> something here as I mentioned at the start that Linux uses or FIFO or 2L.
> 
> Am I missing something?

Quite likely you aren't, but I didn't check. My earlier "covering all" may
well resolve to merely stating things accordingly in the patch description.

Jan

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/20/26 2:03 PM, Jan Beulich wrote:
> On 20.05.2026 13:33, Oleksii Kurochko wrote:
>>
>>
>> On 5/19/26 1:53 PM, Jan Beulich wrote:
>>> On 19.05.2026 13:22, Oleksii Kurochko wrote:
>>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>>>>>>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>>
>>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>>> dereference triggers UBSAN:
>>>>>>>      UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>>      member access within null pointer of type 'struct shared_info_t'
>>>>>>>
>>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>>
>>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>>>
>>>>>> I question this, largely (but not only) because I also ...
>>>>>>
>>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>>> ---
>>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>>
>>>>>> ... question this mode of operation. Yes, you may (for now) be able to
>>>>>> get
>>>>>> away without, but e.g. event channels will want supporting at some point.
>>>>>> Which will require a shared info page. Better put that in place right
>>>>>> away,
>>>>>> even if the guests you test with don't use it (yet). Certain other common
>>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>>
>>>>>
>>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>>
>>>>>        if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>>            goto fail;
>>>>>
>>>>>        clear_page(d->shared_info);
>>>>>
>>>>> ... but without calling share_xen_page_with_guest() after that
>>>>> allocation as share_xen_page_with_guest() isn't implemented at the moment?
>>>
>>> I would have said "yes" here, but ...
>>>
>>>> Or could it be an option for all arch-s move allocation of
>>>> d->shared_info to domain_create() in common just after arch_domain_create()?
>>>
>>> ... Andrew's reply pretty much rules out not only this option, but the
>>> shared-info-page concept as a whole (for RISC-V). See my reply there. In
>>> the meantime, the change as suggested may then indeed be what we want to
>>> go with, albeit (a) with a better description and (b) perhaps covering
>>> all d->shared_info uses.
>>
>> Looking at guest kernel code (Linux), FIFO is tried first, so if RISC-V
>> is going to support only FIFO, d->shared_info could legally be NULL.
>>
>> Looking at the Xen side, if an architecture decides to support only
>> FIFO, d->shared_info is touched only in vcpu_info_reset(), which is
>> called from vcpu_create().
>>
>> All other places where d->shared_info is accessed should not be
>> reachable except for one case in event_fifo.c: when a guest issues the
>> EVTCHNOP_init_control hypercall, setup_ports() reads from shared_info(d,
>> evtchn_pending):
>>     static void setup_ports(struct domain *d, unsigned int prev_evtchns)
>>     {
>>     ...
>>             if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending))
>>                 evtchn->pending = true;
>>     ...
>>         }
>>     }
>>
>> This looks like it handles the transition from the 2L ABI to the FIFO
>> ABI: if a guest started with 2L and then switched to FIFO, any events
>> already pending in shared_info(d, evtchn_pending) need to be migrated to
>> FIFO's per-channel evtchn->pending flag. But it looks like I am missing
>> something here as I mentioned at the start that Linux uses or FIFO or 2L.
>>
>> Am I missing something?
> 
> Quite likely you aren't, but I didn't check. My earlier "covering all" may
> well resolve to merely stating things accordingly in the patch description.

If either FIFO or 2L can be used, shouldn't guest_test_bit(d, port, 
&shared_info(d, evtchn_pending)) in setup_ports() be dropped? If FIFO 
was chosen by Linux, there won't be any events in &shared_info(d, 
evtchn_pending), so it is essentially dead code that could just be 
dropped. Or would it be better to leave it and skip only if 
d->shared_info is allocated: if ( d->shared_info && guest_test_bit(...) 
)  to cover the case when a guest wants to switch from 2L to FIFO (if 
that is even a possible case at all, since as I mentioned above, the 
guest (Linux) chooses the event ABI once and it stays for its lifetime)?

~ Oleksii

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Jan Beulich]

On 20.05.2026 15:40, Oleksii Kurochko wrote:
> 
> 
> On 5/20/26 2:03 PM, Jan Beulich wrote:
>> On 20.05.2026 13:33, Oleksii Kurochko wrote:
>>>
>>>
>>> On 5/19/26 1:53 PM, Jan Beulich wrote:
>>>> On 19.05.2026 13:22, Oleksii Kurochko wrote:
>>>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>>>>>>>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>>>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>>>
>>>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>>>> dereference triggers UBSAN:
>>>>>>>>      UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>>>      member access within null pointer of type 'struct shared_info_t'
>>>>>>>>
>>>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>>>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>>>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>>>
>>>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>>>>
>>>>>>> I question this, largely (but not only) because I also ...
>>>>>>>
>>>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>>>> ---
>>>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>>>
>>>>>>> ... question this mode of operation. Yes, you may (for now) be able to
>>>>>>> get
>>>>>>> away without, but e.g. event channels will want supporting at some point.
>>>>>>> Which will require a shared info page. Better put that in place right
>>>>>>> away,
>>>>>>> even if the guests you test with don't use it (yet). Certain other common
>>>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>>>
>>>>>>
>>>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>>>
>>>>>>        if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>>>            goto fail;
>>>>>>
>>>>>>        clear_page(d->shared_info);
>>>>>>
>>>>>> ... but without calling share_xen_page_with_guest() after that
>>>>>> allocation as share_xen_page_with_guest() isn't implemented at the moment?
>>>>
>>>> I would have said "yes" here, but ...
>>>>
>>>>> Or could it be an option for all arch-s move allocation of
>>>>> d->shared_info to domain_create() in common just after arch_domain_create()?
>>>>
>>>> ... Andrew's reply pretty much rules out not only this option, but the
>>>> shared-info-page concept as a whole (for RISC-V). See my reply there. In
>>>> the meantime, the change as suggested may then indeed be what we want to
>>>> go with, albeit (a) with a better description and (b) perhaps covering
>>>> all d->shared_info uses.
>>>
>>> Looking at guest kernel code (Linux), FIFO is tried first, so if RISC-V
>>> is going to support only FIFO, d->shared_info could legally be NULL.
>>>
>>> Looking at the Xen side, if an architecture decides to support only
>>> FIFO, d->shared_info is touched only in vcpu_info_reset(), which is
>>> called from vcpu_create().
>>>
>>> All other places where d->shared_info is accessed should not be
>>> reachable except for one case in event_fifo.c: when a guest issues the
>>> EVTCHNOP_init_control hypercall, setup_ports() reads from shared_info(d,
>>> evtchn_pending):
>>>     static void setup_ports(struct domain *d, unsigned int prev_evtchns)
>>>     {
>>>     ...
>>>             if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending))
>>>                 evtchn->pending = true;
>>>     ...
>>>         }
>>>     }
>>>
>>> This looks like it handles the transition from the 2L ABI to the FIFO
>>> ABI: if a guest started with 2L and then switched to FIFO, any events
>>> already pending in shared_info(d, evtchn_pending) need to be migrated to
>>> FIFO's per-channel evtchn->pending flag. But it looks like I am missing
>>> something here as I mentioned at the start that Linux uses or FIFO or 2L.
>>>
>>> Am I missing something?
>>
>> Quite likely you aren't, but I didn't check. My earlier "covering all" may
>> well resolve to merely stating things accordingly in the patch description.
> 
> If either FIFO or 2L can be used, shouldn't guest_test_bit(d, port, 
> &shared_info(d, evtchn_pending)) in setup_ports() be dropped? If FIFO 
> was chosen by Linux, there won't be any events in &shared_info(d, 
> evtchn_pending), so it is essentially dead code that could just be 
> dropped.

Why would it be dead code? Who said that a guest couldn't to 2L for a
while, then switch to FIFO? Think of boot loaders, for example.

Jan

> Or would it be better to leave it and skip only if 
> d->shared_info is allocated: if ( d->shared_info && guest_test_bit(...) 
> )  to cover the case when a guest wants to switch from 2L to FIFO (if 
> that is even a possible case at all, since as I mentioned above, the 
> guest (Linux) chooses the event ABI once and it stays for its lifetime)?
> 
> ~ Oleksii

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/20/26 4:21 PM, Jan Beulich wrote:
> On 20.05.2026 15:40, Oleksii Kurochko wrote:
>>
>>
>> On 5/20/26 2:03 PM, Jan Beulich wrote:
>>> On 20.05.2026 13:33, Oleksii Kurochko wrote:
>>>>
>>>>
>>>> On 5/19/26 1:53 PM, Jan Beulich wrote:
>>>>> On 19.05.2026 13:22, Oleksii Kurochko wrote:
>>>>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
>>>>>>>>> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
>>>>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>>>>
>>>>>>>>> However, it does not guard against d->shared_info being NULL.  The
>>>>>>>>> shared_info() macro expands to a member access through d->shared_info,
>>>>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>>>>> dereference triggers UBSAN:
>>>>>>>>>       UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>>>>       member access within null pointer of type 'struct shared_info_t'
>>>>>>>>>
>>>>>>>>> Extend the existing fallback condition to also cover the case where no
>>>>>>>>> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
>>>>>>>>> instead. This is the correct behaviour: dummy_vcpu_info already serves
>>>>>>>>> as the safe stand-in for vcpus that have no usable shared_info slot.
>>>>>>>>>
>>>>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area registration")
>>>>>>>>
>>>>>>>> I question this, largely (but not only) because I also ...
>>>>>>>>
>>>>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>>>>> ---
>>>>>>>>> RISC-V does not allocate a shared_info page at the momemnt because its
>>>>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>>>>
>>>>>>>> ... question this mode of operation. Yes, you may (for now) be able to
>>>>>>>> get
>>>>>>>> away without, but e.g. event channels will want supporting at some point.
>>>>>>>> Which will require a shared info page. Better put that in place right
>>>>>>>> away,
>>>>>>>> even if the guests you test with don't use it (yet). Certain other common
>>>>>>>> code also assumes d->shared_info to never be NULL for an alive domain.
>>>>>>>>
>>>>>>>
>>>>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>>>>
>>>>>>>         if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>>>>             goto fail;
>>>>>>>
>>>>>>>         clear_page(d->shared_info);
>>>>>>>
>>>>>>> ... but without calling share_xen_page_with_guest() after that
>>>>>>> allocation as share_xen_page_with_guest() isn't implemented at the moment?
>>>>>
>>>>> I would have said "yes" here, but ...
>>>>>
>>>>>> Or could it be an option for all arch-s move allocation of
>>>>>> d->shared_info to domain_create() in common just after arch_domain_create()?
>>>>>
>>>>> ... Andrew's reply pretty much rules out not only this option, but the
>>>>> shared-info-page concept as a whole (for RISC-V). See my reply there. In
>>>>> the meantime, the change as suggested may then indeed be what we want to
>>>>> go with, albeit (a) with a better description and (b) perhaps covering
>>>>> all d->shared_info uses.
>>>>
>>>> Looking at guest kernel code (Linux), FIFO is tried first, so if RISC-V
>>>> is going to support only FIFO, d->shared_info could legally be NULL.
>>>>
>>>> Looking at the Xen side, if an architecture decides to support only
>>>> FIFO, d->shared_info is touched only in vcpu_info_reset(), which is
>>>> called from vcpu_create().
>>>>
>>>> All other places where d->shared_info is accessed should not be
>>>> reachable except for one case in event_fifo.c: when a guest issues the
>>>> EVTCHNOP_init_control hypercall, setup_ports() reads from shared_info(d,
>>>> evtchn_pending):
>>>>      static void setup_ports(struct domain *d, unsigned int prev_evtchns)
>>>>      {
>>>>      ...
>>>>              if ( guest_test_bit(d, port, &shared_info(d, evtchn_pending))
>>>>                  evtchn->pending = true;
>>>>      ...
>>>>          }
>>>>      }
>>>>
>>>> This looks like it handles the transition from the 2L ABI to the FIFO
>>>> ABI: if a guest started with 2L and then switched to FIFO, any events
>>>> already pending in shared_info(d, evtchn_pending) need to be migrated to
>>>> FIFO's per-channel evtchn->pending flag. But it looks like I am missing
>>>> something here as I mentioned at the start that Linux uses or FIFO or 2L.
>>>>
>>>> Am I missing something?
>>>
>>> Quite likely you aren't, but I didn't check. My earlier "covering all" may
>>> well resolve to merely stating things accordingly in the patch description.
>>
>> If either FIFO or 2L can be used, shouldn't guest_test_bit(d, port,
>> &shared_info(d, evtchn_pending)) in setup_ports() be dropped? If FIFO
>> was chosen by Linux, there won't be any events in &shared_info(d,
>> evtchn_pending), so it is essentially dead code that could just be
>> dropped.
> 
> Why would it be dead code? Who said that a guest couldn't to 2L for a
> while, then switch to FIFO? Think of boot loaders, for example.

I jsut based my assumption on Linux use case, if generally such switch 
is okay then I will add to my original patch what I suggested here ...

> 
> Jan
> 
>> Or would it be better to leave it and skip only if
>> d->shared_info is allocated: if ( d->shared_info && guest_test_bit(...)
>> )  to cover the case when a guest wants to switch from 2L to FIFO (if
>> that is even a possible case at all, since as I mentioned above, the
>> guest (Linux) chooses the event ABI once and it stays for its lifetime)?

... + refactor commit message:
```
xen/domain: fix UBSAN null pointer dereference of d->shared_info

It is legal to have d->shared_info equal to NULL for architectures which
support only the FIFO ABI for event channel management.

Having d->shared_info == NULL leads to a UBSAN issue on such architectures:
   UBSAN: Undefined behaviour in common/domain.c:325:10
          member access within null pointer of type 'struct shared_info_t'

vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
and falls back to dummy_vcpu_info for vcpus beyond that limit.
Extend the existing fallback condition to also cover the case where no
shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
instead. This is the correct behaviour: dummy_vcpu_info already serves
as the safe stand-in for vcpus that have no usable shared_info slot.

Additionally, if an architecture supports only the FIFO ABI, setup_ports()
should be updated to avoid a NULL pointer dereference of d->shared_info,
since in that case there will be no pending events in
shared_info->evtchn_pending and the pending flag of the FIFO event 
channel does not need to be set to true.
```

~ Oleksii

Re: [PATCH v1 2/3] xen/domain: fix UBSAN null pointer dereference in vcpu_info_reset()

[Oleksii Kurochko]

On 5/20/26 5:08 PM, Oleksii Kurochko wrote:
> 
> 
> On 5/20/26 4:21 PM, Jan Beulich wrote:
>> On 20.05.2026 15:40, Oleksii Kurochko wrote:
>>>
>>>
>>> On 5/20/26 2:03 PM, Jan Beulich wrote:
>>>> On 20.05.2026 13:33, Oleksii Kurochko wrote:
>>>>>
>>>>>
>>>>> On 5/19/26 1:53 PM, Jan Beulich wrote:
>>>>>> On 19.05.2026 13:22, Oleksii Kurochko wrote:
>>>>>>> On 5/19/26 12:55 PM, Oleksii Kurochko wrote:
>>>>>>>> On 5/19/26 11:37 AM, Jan Beulich wrote:
>>>>>>>>> On 19.05.2026 10:39, Oleksii Kurochko wrote:
>>>>>>>>>> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu 
>>>>>>>>>> slot inside
>>>>>>>>>> the domain's shared_info page for vcpus with id < 
>>>>>>>>>> XEN_LEGACY_MAX_VCPUS,
>>>>>>>>>> and falls back to dummy_vcpu_info for vcpus beyond that limit.
>>>>>>>>>>
>>>>>>>>>> However, it does not guard against d->shared_info being NULL.  
>>>>>>>>>> The
>>>>>>>>>> shared_info() macro expands to a member access through d- 
>>>>>>>>>> >shared_info,
>>>>>>>>>> so when an architecture does not allocate a shared_info page the
>>>>>>>>>> dereference triggers UBSAN:
>>>>>>>>>>       UBSAN: Undefined behaviour in common/domain.c:325:10
>>>>>>>>>>       member access within null pointer of type 'struct 
>>>>>>>>>> shared_info_t'
>>>>>>>>>>
>>>>>>>>>> Extend the existing fallback condition to also cover the case 
>>>>>>>>>> where no
>>>>>>>>>> shared_info page has been allocated, mapping the vcpu to 
>>>>>>>>>> dummy_vcpu_info
>>>>>>>>>> instead. This is the correct behaviour: dummy_vcpu_info 
>>>>>>>>>> already serves
>>>>>>>>>> as the safe stand-in for vcpus that have no usable shared_info 
>>>>>>>>>> slot.
>>>>>>>>>>
>>>>>>>>>> Fixes: 295514ff75506 ("common: convert vCPU info area 
>>>>>>>>>> registration")
>>>>>>>>>
>>>>>>>>> I question this, largely (but not only) because I also ...
>>>>>>>>>
>>>>>>>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>>>>>>>>> Reviewed-by: Baptiste Le Duc <baptiste.le-duc@vates.tech>
>>>>>>>>>> ---
>>>>>>>>>> RISC-V does not allocate a shared_info page at the momemnt 
>>>>>>>>>> because its
>>>>>>>>>> guests run in dom0less mode and do not use the Xen PV ABI, so
>>>>>>>>>> d->shared_info remains NULL throughout domain lifetime.
>>>>>>>>>
>>>>>>>>> ... question this mode of operation. Yes, you may (for now) be 
>>>>>>>>> able to
>>>>>>>>> get
>>>>>>>>> away without, but e.g. event channels will want supporting at 
>>>>>>>>> some point.
>>>>>>>>> Which will require a shared info page. Better put that in place 
>>>>>>>>> right
>>>>>>>>> away,
>>>>>>>>> even if the guests you test with don't use it (yet). Certain 
>>>>>>>>> other common
>>>>>>>>> code also assumes d->shared_info to never be NULL for an alive 
>>>>>>>>> domain.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Would it be fine than to allocate it in arch_domain_create() ... :
>>>>>>>>
>>>>>>>>         if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
>>>>>>>>             goto fail;
>>>>>>>>
>>>>>>>>         clear_page(d->shared_info);
>>>>>>>>
>>>>>>>> ... but without calling share_xen_page_with_guest() after that
>>>>>>>> allocation as share_xen_page_with_guest() isn't implemented at 
>>>>>>>> the moment?
>>>>>>
>>>>>> I would have said "yes" here, but ...
>>>>>>
>>>>>>> Or could it be an option for all arch-s move allocation of
>>>>>>> d->shared_info to domain_create() in common just after 
>>>>>>> arch_domain_create()?
>>>>>>
>>>>>> ... Andrew's reply pretty much rules out not only this option, but 
>>>>>> the
>>>>>> shared-info-page concept as a whole (for RISC-V). See my reply 
>>>>>> there. In
>>>>>> the meantime, the change as suggested may then indeed be what we 
>>>>>> want to
>>>>>> go with, albeit (a) with a better description and (b) perhaps 
>>>>>> covering
>>>>>> all d->shared_info uses.
>>>>>
>>>>> Looking at guest kernel code (Linux), FIFO is tried first, so if 
>>>>> RISC-V
>>>>> is going to support only FIFO, d->shared_info could legally be NULL.
>>>>>
>>>>> Looking at the Xen side, if an architecture decides to support only
>>>>> FIFO, d->shared_info is touched only in vcpu_info_reset(), which is
>>>>> called from vcpu_create().
>>>>>
>>>>> All other places where d->shared_info is accessed should not be
>>>>> reachable except for one case in event_fifo.c: when a guest issues the
>>>>> EVTCHNOP_init_control hypercall, setup_ports() reads from 
>>>>> shared_info(d,
>>>>> evtchn_pending):
>>>>>      static void setup_ports(struct domain *d, unsigned int 
>>>>> prev_evtchns)
>>>>>      {
>>>>>      ...
>>>>>              if ( guest_test_bit(d, port, &shared_info(d, 
>>>>> evtchn_pending))
>>>>>                  evtchn->pending = true;
>>>>>      ...
>>>>>          }
>>>>>      }
>>>>>
>>>>> This looks like it handles the transition from the 2L ABI to the FIFO
>>>>> ABI: if a guest started with 2L and then switched to FIFO, any events
>>>>> already pending in shared_info(d, evtchn_pending) need to be 
>>>>> migrated to
>>>>> FIFO's per-channel evtchn->pending flag. But it looks like I am 
>>>>> missing
>>>>> something here as I mentioned at the start that Linux uses or FIFO 
>>>>> or 2L.
>>>>>
>>>>> Am I missing something?
>>>>
>>>> Quite likely you aren't, but I didn't check. My earlier "covering 
>>>> all" may
>>>> well resolve to merely stating things accordingly in the patch 
>>>> description.
>>>
>>> If either FIFO or 2L can be used, shouldn't guest_test_bit(d, port,
>>> &shared_info(d, evtchn_pending)) in setup_ports() be dropped? If FIFO
>>> was chosen by Linux, there won't be any events in &shared_info(d,
>>> evtchn_pending), so it is essentially dead code that could just be
>>> dropped.
>>
>> Why would it be dead code? Who said that a guest couldn't to 2L for a
>> while, then switch to FIFO? Think of boot loaders, for example.
> 
> I jsut based my assumption on Linux use case, if generally such switch 
> is okay then I will add to my original patch what I suggested here ...
> 
>>
>> Jan
>>
>>> Or would it be better to leave it and skip only if
>>> d->shared_info is allocated: if ( d->shared_info && guest_test_bit(...)
>>> )  to cover the case when a guest wants to switch from 2L to FIFO (if
>>> that is even a possible case at all, since as I mentioned above, the
>>> guest (Linux) chooses the event ABI once and it stays for its lifetime)?
> 
> ... + refactor commit message:
> ```
> xen/domain: fix UBSAN null pointer dereference of d->shared_info
> 
> It is legal to have d->shared_info equal to NULL for architectures which
> support only the FIFO ABI for event channel management.
> 
> Having d->shared_info == NULL leads to a UBSAN issue on such architectures:
>    UBSAN: Undefined behaviour in common/domain.c:325:10
>           member access within null pointer of type 'struct shared_info_t'
> 
> vcpu_info_reset() maps v->vcpu_info_area.map to the per-vcpu slot inside
> the domain's shared_info page for vcpus with id < XEN_LEGACY_MAX_VCPUS,
> and falls back to dummy_vcpu_info for vcpus beyond that limit.
> Extend the existing fallback condition to also cover the case where no
> shared_info page has been allocated, mapping the vcpu to dummy_vcpu_info
> instead. This is the correct behaviour: dummy_vcpu_info already serves
> as the safe stand-in for vcpus that have no usable shared_info slot.
> 
> Additionally, if an architecture supports only the FIFO ABI, setup_ports()
> should be updated to avoid a NULL pointer dereference of d->shared_info,
> since in that case there will be no pending events in
> shared_info->evtchn_pending and the pending flag of the FIFO event 
> channel does not need to be set to true.
> ```
> Additionally it seems it is needed to add the following:

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 93738931c575..2fa2dcdf4ded 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -104,9 +104,12 @@ void getdomaininfo(struct domain *d, struct 
xen_domctl_getdomaininfo *info)
  #ifdef CONFIG_MEM_PAGING
      info->paged_pages       = atomic_read(&d->paged_pages);
  #endif
-    info->shared_info_frame =
-        gfn_x(mfn_to_gfn(d, _mfn(virt_to_mfn(d->shared_info))));
-    BUG_ON(SHARED_M2P(info->shared_info_frame));
+    if ( d->shared_info )
+    {
+        info->shared_info_frame =
+            gfn_x(mfn_to_gfn(d, _mfn(virt_to_mfn(d->shared_info))));
+        BUG_ON(SHARED_M2P(info->shared_info_frame));
+    }

      info->cpupool = cpupool_get_id(d);

diff --git a/xen/common/time.c b/xen/common/time.c
index 04a65f00b35c..1ee49a8b0d13 100644
--- a/xen/common/time.c
+++ b/xen/common/time.c
@@ -94,6 +94,9 @@ void update_domain_wallclock_time(struct domain *d)
      uint32_t *wc_version;
      uint64_t sec;

+    if ( !d->shared_info )
+        return;
+
      spin_lock(&wc_lock);

      wc_version = &shared_info(d, wc_version);

~ Oleksii