On 05/04/2019 19:29, Norbert Manthey wrote:
> On 4/5/19 17:34, Andrew Cooper wrote:
>> On 14/03/2019 12:50, Norbert Manthey wrote:
>>> When checking for being an hvm domain, or PV domain, we have to make
>>> sure that speculation cannot bypass that check, and eventually access
>>> data that should not end up in cache for the current domain type.
>>>
>>> This is part of the speculative hardening effort.
>>>
>>> Signed-off-by: Norbert Manthey <nmant...@amazon.de>
>>> Acked-by: Jan Beulich <jbeul...@suse.com>
>>>
>>> ---
>>> xen/include/xen/sched.h | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>>> --- a/xen/include/xen/sched.h
>>> +++ b/xen/include/xen/sched.h
>>> @@ -922,7 +922,8 @@ void watchdog_domain_destroy(struct domain *d);
>>>
>>> static inline bool is_pv_domain(const struct domain *d)
>>> {
>>> - return IS_ENABLED(CONFIG_PV) ? d->guest_type == guest_type_pv : false;
>>> + return IS_ENABLED(CONFIG_PV)
>>> + ? evaluate_nospec(d->guest_type == guest_type_pv) : false;
>>> }
>>>
>>> static inline bool is_pv_vcpu(const struct vcpu *v)
>>> @@ -953,7 +954,8 @@ static inline bool is_pv_64bit_vcpu(const struct vcpu
>>> *v)
>>> #endif
>>> static inline bool is_hvm_domain(const struct domain *d)
>>> {
>>> - return IS_ENABLED(CONFIG_HVM) ? d->guest_type == guest_type_hvm :
>>> false;
>>> + return IS_ENABLED(CONFIG_HVM)
>>> + ? evaluate_nospec(d->guest_type == guest_type_hvm) : false;
>>> }
>> Unfortunately, this breaks the livepatch build, in a rather insidious
>> way. The asm inside evaluate_nospec() forces these out-of-line.
>>
>> Duplicate symbol 'domctl.c#is_pv_domain.isra.0' (ffff82d080270390 !=
>> ffff82d08036a005)
>>
>> On forcing these to be always_inline, the next bit of breakage occurs at:
>>
>> Duplicate symbol 'domctl.c#is_pv_32bit_domain' (ffff82d080270270 !=
>> ffff82d08036a000)
>>
>> which means we now have a goose chase of throwing always_inline through
>> the code.
>>
>> I'm going to have to revert this in XenServer, especially seeing as
>> OSSTest is currently out of action, which AFAICT, makes XenServer's
>> testing the only automatic testing which staging is getting.
>>
>> I also need to see about upstreaming out local patch which causes the
>> build to properly fail in cases which we know will break the ability to
>> build livepatches.
> I see, thanks for the pointer. I guess the build only fails in case you
> patch some function that needs one of the two? In case you have
> something that reproduces the problem, and can be shared, we might have
> a look. Thanks!
The presence of duplicate symbols breaks the binary diffing logic in the
livepatch build process itself.
For runtime patch application, you are correct that it is fine in
practice so long as you don't care about patching Is_pv_domain() itself.
One part of livepatching work which appears not to have been upstreamed
(and I'm trying to fix this right now), is the bit which causes a hard
failure of the hypervisor build when you select CONFIG_LIVEPATCH and
duplicate symbols are detected.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
