[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/4] xen/arm: Handle cases when hardware_domain is NULL
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Luca Fancellu <luca.fancellu@xxxxxxx>
- Date: Mon, 12 Apr 2021 14:58:58 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=rAeEwcj23krVwC3C/RIwtZr+s+XvL2+19gc2q6ein7g=; b=QetbBT9eUj1d7z6db3QSS+V3pW2JnYKjplGxNdRnYXlWUzp0inTAgADjQKzrrrOGJF9bSOmj6XBMzfzEFnHMpWMaxRE7416dfeVJ7nCY6MFNeq4ARf9y5euIh65Djf8n8A/2TIxN5M8jtsjoNZXPvhlZsSUcwn9vk58dtxsiluW408I4WRiGtETH3YkxKfCAHPeEyAvsbc5ZjYDovmQp9Vr5FjJWaxPATZmPyrPbp1GeXra4gXXEP9kQ90OGzPGqIK0aoB1l3LeItwTjugUIFxBVV1QsNy8ZZhxOitCJlSG+wJrFj4GMh5GFa8eqsGGP+/InMoHgbKu8WUhSr1LSnQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ugxoh04+c9y6E0o72rGBnE/ozrz8QPvAnQfZUBeV/lXCN7wnCfzXjao3uqf+QywQoi4sK9Lh4Wkzgh3X01/ugHdi6LOQ+Jqpuf2nyAYTTeqax5kT98cAjS288naeves8zPDVNIA9lyWShEcB4UNiLvucFNTJ+fA9uJedqrU8kLn/61TfUoppbnz4Z3jcK+Y+byFXBV5C0PAQ5N7rV0l1BYAk1iFfbl0TABjHcOXyUIaIuqqBV+KoH/YLRw8tli95sDfcGwbEZqcQPFM3o/J3GCtKfeMM2QW3wga1G+MBj0SJGbS8aNI1Upg1tEHO2HDsvNXVWNcHm0EsVeJf3Oznfw==
- Authentication-results-original: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
- Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>, wei.chen@xxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Mon, 12 Apr 2021 13:59:30 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
- Nodisclaimer: true
- Original-authentication-results: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=arm.com;
> On 12 Apr 2021, at 12:03, Jan Beulich <jbeulich@xxxxxxxx> 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
|
