Re: [PATCH v3 2/2] xen: introduce CONFIG_HAS_SHARED_INFO for archs without a shared page

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.06.2026 10:50, Oleksii Kurochko wrote:
> 
> 
> On 6/17/26 4:24 PM, Jan Beulich wrote:
>> On 17.06.2026 16:02, Oleksii Kurochko wrote:
>>> On 6/17/26 3:26 PM, Jan Beulich wrote:
>>>> On 03.06.2026 16:25, Oleksii Kurochko wrote:
>>>>> --- a/xen/common/domain.c
>>>>> +++ b/xen/common/domain.c
>>>>> @@ -320,9 +320,9 @@ void vcpu_info_reset(struct vcpu *v)
>>>>>        struct domain *d = v->domain;
>>>>>    
>>>>>        v->vcpu_info_area.map =
>>>>> -        ((v->vcpu_id < XEN_LEGACY_MAX_VCPUS)
>>>>> -         ? (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id])
>>>>> -         : &dummy_vcpu_info);
>>>>> +        IS_ENABLED(CONFIG_HAS_SHARED_INFO) && v->vcpu_id < 
>>>>> XEN_LEGACY_MAX_VCPUS
>>>>> +        ? (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id])
>>>>> +        : &dummy_vcpu_info;
>>>>>    }
>>>>
>>>> While the change here is likely okay, it points at possible further 
>>>> omissions.
>>>> You've dealt with all uses of shared_info(), but you've left alone all 
>>>> uses of
>>>> vcpu_info() (and __vcpu_info()). Reads are presumably okay, but writes to
>>>> dummy_vcpu_info open a side channel for possible info leaks. Looking over 
>>>> uses
>>>> in common code, no code changes may be needed; extending the description 
>>>> may
>>>> be all that's wanted here.
>>>
>>> Isn't there already a side channel that could allow leaks, even without
>>> this change?
>>
>> There are multiple aspects here. First, for PV secondary vCPU-s cannot be
>> launched when their vcpu-info still points at dummy_vcpu_info. HVM vCPU-s
>> make very limited use of vcpu-info fields. Writes look to be limited to
>> the evtchn_upcall_{mask,pending} fields, which isn't really an info leak.
>>
>> My main point here is: None of this goes without making clear that the
>> necessary auditing was properly done.
>>
>>> The change here just made it worsen because now info leak
>>> will happen for all vCPUs when  CONFIG_HAS_SHARED_INFO=n.
>>>
>>> I will add to the description the following:
>>> ```
>>> With CONFIG_HAS_SHARED_INFO=n all vCPUs fall back to the global
>>> dummy_vcpu_info, so writes through vcpu_info() could leak data between
>>> vCPUs.  Reviewing the write paths in common code: the write in
>>> map_guest_area() stores the constant ~0 so nothing serious will happen
>>> if it will be leaked; the event_2l.c paths are unreachable because the
>>> preceding shared_info() call would trap first; the write in
>>> vcpu_info_populate() targets the new mapping buffer, not
>>> dummy_vcpu_info; all remaining writes are x86 PV-specific for which
>>> CONFIG_HAS_SHARED_INFO=y.  No code changes are needed.
>>> ```
>>
>> As you start with "common code", how come the "x86 PV-specific" part is
>> still there (i.e. relevant)? Isn't all PV stuff in x86-specific code?
> 
> Good point. The "x86 PV-specific" part is not part of the review of 
> common code. I mentioned it separately to complete the audit of all 
> write paths reachable through vcpu_info(). The intent was:
> 
> * the writes discussed before the semicolon are the common-code paths;
> * the writes after the semicolon are outside common code and are x86 
> PV-specific, where CONFIG_HAS_SHARED_INFO=y anyway.
> 
> To avoid the ambiguity, I can reword the sentence to make that 
> separation explicit:
> ```
> With CONFIG_HAS_SHARED_INFO=n all vCPUs fall back to the global
> dummy_vcpu_info, so writes through vcpu_info() could leak data between
> vCPUs. Reviewing the write paths in common code: the write in
> map_guest_area() stores the constant ~0 so nothing serious would happen
> if it were leaked; the event_2l.c paths are unreachable because the
> preceding shared_info() call would trap first; the write in
> vcpu_info_populate() targets the new mapping buffer, not
> dummy_vcpu_info.
> 
> Outside common code, the remaining writes are x86 PV-specific, for which
> CONFIG_HAS_SHARED_INFO=y. No code changes are needed.
> ```
> 
> Would that wording work for you?

I think so, yes.

Jan