> On 12 Apr 2021, at 12:03, Jan Beulich <[email protected]> wrote:
>
> On 12.04.2021 12:52, Luca Fancellu wrote:
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -1022,6 +1022,9 @@ static always_inline bool is_hardware_domain(const
>> struct domain *d)
>> if ( IS_ENABLED(CONFIG_PV_SHIM_EXCLUSIVE) )
>> return false;
>>
>> + if ( !d )
>> + return false;
>> +
>> return evaluate_nospec(d == hardware_domain);
>> }
>
> On v2 I did say on the respective code that was here (and my
> suggestion of this alternative adjustment): "Can you point out
> code paths where d may actually be NULL, and where [...] would
> not behave as intended (i.e. where bad speculation would
> result)?"
>
> Since you've taken the suggestion as-is, and since the commit
> message says nothing in either direction here, did you actually
> verify that there's no abuse of speculation possible with this
> extra return path? And did you find any caller at all which may
> pass NULL into here?
Hi Jan,
I’ve analysed the code and seems that there are no path that calls
Is_hardware_domain() with a NULL domain, however I found this
function in xen/arch/arm/irq.c:
bool irq_type_set_by_domain(const struct domain *d)
{
return is_hardware_domain(d);
}
That is calling directly is_hardware_domain and it is global.
It can be the case that a future code can call irq_type_set_by_domain
potentially with a null domain...
I introduced a check for the domain for that reason, do you think that
maybe it’s better to put that check on the irq_type_set_by_domain instead?
Thank you for your feedback.
Cheers,
Luca
>
> Jan
