Undefined behavior in libxenvchan

Undefined behavior in libxenvchan

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 441 bytes --]

I noticed that libxenvchan has undefined behavior: it passes pointers
to guest memory to memcpy() even though they can be concurrently
changed.

Would it make sense to reuse some of Xen's copy_from_guest() code
instead?  There might be a licensing problem (GPL vs LGPL), though.
I think the only approach that isn't UB and has decent performance
is to do the whole copy in assembly.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Undefined behavior in libxenvchan

[Andrew Cooper]

On 14/12/2025 7:09 pm, Demi Marie Obenour wrote:
> I noticed that libxenvchan has undefined behavior: it passes pointers
> to guest memory to memcpy() even though they can be concurrently
> changed.
>
> Would it make sense to reuse some of Xen's copy_from_guest() code
> instead?  There might be a licensing problem (GPL vs LGPL), though.
> I think the only approach that isn't UB and has decent performance
> is to do the whole copy in assembly.

memcpy() is well defined.

The problem is the potential for creating TOCTOU races if suitable
barriers aren't used, due to the compiler being able to optimise through
memcpy().

Xen's copy to/from guest are not appropriate in userspace.  They're
guarding against pagefaults and address ranges not belonging to the
target context.

If more compiler/smp barriers are needed, then that's the appropriate fix.

~Andrew

Re: Undefined behavior in libxenvchan

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 1326 bytes --]

On 12/14/25 17:50, Andrew Cooper wrote:
> On 14/12/2025 7:09 pm, Demi Marie Obenour wrote:
>> I noticed that libxenvchan has undefined behavior: it passes pointers
>> to guest memory to memcpy() even though they can be concurrently
>> changed.
>>
>> Would it make sense to reuse some of Xen's copy_from_guest() code
>> instead?  There might be a licensing problem (GPL vs LGPL), though.
>> I think the only approach that isn't UB and has decent performance
>> is to do the whole copy in assembly.
> 
> memcpy() is well defined.

Rich Felker wrote otherwise on the musl mailing list.  Specifically,
it is undefined behavior if the data is changed while memcpy() is
accessing it, either for reading or for writing.

> The problem is the potential for creating TOCTOU races if suitable
> barriers aren't used, due to the compiler being able to optimise through
> memcpy().

The concern here is about races in memcpy() itself.

> Xen's copy to/from guest are not appropriate in userspace.  They're
> guarding against pagefaults and address ranges not belonging to the
> target context.
> 
> If more compiler/smp barriers are needed, then that's the appropriate fix.

Rich Felker suggested to use an open-coded memcpy() that used volatile
accesses.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Undefined behavior in libxenvchan

[Jan Beulich]

On 15.12.2025 00:08, Demi Marie Obenour wrote:
> On 12/14/25 17:50, Andrew Cooper wrote:
>> On 14/12/2025 7:09 pm, Demi Marie Obenour wrote:
>>> I noticed that libxenvchan has undefined behavior: it passes pointers
>>> to guest memory to memcpy() even though they can be concurrently
>>> changed.
>>>
>>> Would it make sense to reuse some of Xen's copy_from_guest() code
>>> instead?  There might be a licensing problem (GPL vs LGPL), though.
>>> I think the only approach that isn't UB and has decent performance
>>> is to do the whole copy in assembly.
>>
>> memcpy() is well defined.
> 
> Rich Felker wrote otherwise on the musl mailing list.  Specifically,
> it is undefined behavior if the data is changed while memcpy() is
> accessing it, either for reading or for writing.

Aren't you talking about undefined-ness beyond what the C spec uses the
term for? Of course it is always unpredictable what the result of a
function will be when you fiddle with its source behind its back. But
that's not of concern as far as safety is concerned (while the
correctness issue that results is solely a problem for the party doing
the undue modifications).

What we need to guarantee is that whatever copy is made of whatever
in-flight data, any sanity and consistency checking as well as subsequent
use would take only the one, exact same copy of source data. Which, as
Andrew said, may require some extra barriers, while using memcpy() for
the mechanical copying ought to be okay.

Jan

>> The problem is the potential for creating TOCTOU races if suitable
>> barriers aren't used, due to the compiler being able to optimise through
>> memcpy().
> 
> The concern here is about races in memcpy() itself.
> 
>> Xen's copy to/from guest are not appropriate in userspace.  They're
>> guarding against pagefaults and address ranges not belonging to the
>> target context.
>>
>> If more compiler/smp barriers are needed, then that's the appropriate fix.
> 
> Rich Felker suggested to use an open-coded memcpy() that used volatile
> accesses.

Re: Undefined behavior in libxenvchan

[Teddy Astie]

Le 15/12/2025 à 00:08, Demi Marie Obenour a écrit :
> On 12/14/25 17:50, Andrew Cooper wrote:
>> On 14/12/2025 7:09 pm, Demi Marie Obenour wrote:
>>> I noticed that libxenvchan has undefined behavior: it passes pointers
>>> to guest memory to memcpy() even though they can be concurrently
>>> changed.
>>>
>>> Would it make sense to reuse some of Xen's copy_from_guest() code
>>> instead?  There might be a licensing problem (GPL vs LGPL), though.
>>> I think the only approach that isn't UB and has decent performance
>>> is to do the whole copy in assembly.
>>
>> memcpy() is well defined.
>
> Rich Felker wrote otherwise on the musl mailing list.  Specifically,
> it is undefined behavior if the data is changed while memcpy() is
> accessing it, either for reading or for writing.
>
>> The problem is the potential for creating TOCTOU races if suitable
>> barriers aren't used, due to the compiler being able to optimise through
>> memcpy().
>
> The concern here is about races in memcpy() itself.
>
>> Xen's copy to/from guest are not appropriate in userspace.  They're
>> guarding against pagefaults and address ranges not belonging to the
>> target context.
>>
>> If more compiler/smp barriers are needed, then that's the appropriate fix.
>
> Rich Felker suggested to use an open-coded memcpy() that used volatile
> accesses.

Do you mean that if a libc uses something like this as a memcpy.

void *memcpy(
     void *restrict dest_str,
     const void *restrict src_str,
     size_t n)
{
     const char *src = src_str;
     char *dest = dest_str;
     size_t i = 0;

     while (i < n)
     {
         dest[i] = src[i];
         i++;
     }

     return dest_str;
}

that the compiler is free to optimize inside this function in ways that
conflict with the "actual volatile-ness" of dest/src ?

Anything said regarding regarding TOCTOU can also happens from within
the memcpy (even though most memcpy() implementations and what compiler
would emit here is very unlikely unaffected by this).

Unfortunately, there is no available volatile memcpy in C, LLVM has a
volatile memcpy, but not usable from C, Rust exposes it through
"unstable" volatile_copy_non_overlapping, which got discussed in [1],
there is also something regarding "atomic memcpy" [2], but I don't know
the exact status of all this.

[1]
https://doc.rust-lang.org/stable/core/intrinsics/fn.volatile_copy_nonoverlapping_memory.html

[2] https://github.com/rust-lang/rfcs/pull/3301

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: Undefined behavior in libxenvchan

[Demi Marie Obenour]

[-- Attachment #1.1.1: Type: text/plain, Size: 2845 bytes --]

On 12/15/25 06:10, Teddy Astie wrote:
> Le 15/12/2025 à 00:08, Demi Marie Obenour a écrit :
>> On 12/14/25 17:50, Andrew Cooper wrote:
>>> On 14/12/2025 7:09 pm, Demi Marie Obenour wrote:
>>>> I noticed that libxenvchan has undefined behavior: it passes pointers
>>>> to guest memory to memcpy() even though they can be concurrently
>>>> changed.
>>>>
>>>> Would it make sense to reuse some of Xen's copy_from_guest() code
>>>> instead?  There might be a licensing problem (GPL vs LGPL), though.
>>>> I think the only approach that isn't UB and has decent performance
>>>> is to do the whole copy in assembly.
>>>
>>> memcpy() is well defined.
>>
>> Rich Felker wrote otherwise on the musl mailing list.  Specifically,
>> it is undefined behavior if the data is changed while memcpy() is
>> accessing it, either for reading or for writing.
>>
>>> The problem is the potential for creating TOCTOU races if suitable
>>> barriers aren't used, due to the compiler being able to optimise through
>>> memcpy().
>>
>> The concern here is about races in memcpy() itself.
>>
>>> Xen's copy to/from guest are not appropriate in userspace.  They're
>>> guarding against pagefaults and address ranges not belonging to the
>>> target context.
>>>
>>> If more compiler/smp barriers are needed, then that's the appropriate fix.
>>
>> Rich Felker suggested to use an open-coded memcpy() that used volatile
>> accesses.
> 
> Do you mean that if a libc uses something like this as a memcpy.
> 
> void *memcpy(
>      void *restrict dest_str,
>      const void *restrict src_str,
>      size_t n)
> {
>      const char *src = src_str;
>      char *dest = dest_str;
>      size_t i = 0;
> 
>      while (i < n)
>      {
>          dest[i] = src[i];
>          i++;
>      }
> 
>      return dest_str;
> }
> 
> that the compiler is free to optimize inside this function in ways that 
> conflict with the "actual volatile-ness" of dest/src ?

Correct.

> Anything said regarding regarding TOCTOU can also happens from within 
> the memcpy (even though most memcpy() implementations and what compiler 
> would emit here is very unlikely unaffected by this).
> 
> Unfortunately, there is no available volatile memcpy in C, LLVM has a 
> volatile memcpy, but not usable from C, Rust exposes it through 
> "unstable" volatile_copy_non_overlapping, which got discussed in [1], 
> there is also something regarding "atomic memcpy" [2], but I don't know 
> the exact status of all this.
> 
> [1] 
> https://doc.rust-lang.org/stable/core/intrinsics/fn.volatile_copy_nonoverlapping_memory.html
> 
> [2] https://github.com/rust-lang/rfcs/pull/3301
> 
> Teddy

The C standard expects one to write an open-coded loop and to
not use libc memcpy().
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]