Re: Undefined behavior in libxenvchan

[Demi Marie Obenour <demiobenour@xxxxxxxxx>]

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: OpenPGP_0xB288B55FFF9C22C1.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature