[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 5/6] xen/riscv: introduce an implementation of macros from <asm/bug.h>
- To: Oleksii <oleksii.kurochko@xxxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Wed, 31 May 2023 14:55:59 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=eibG4ODoHhSIit3N/ADLUkJ1CVVZUjtrmS789JHSSV8=; b=cg34yxr/PLZQJZlxreICD6Y5U1Jc3cNpHX850BHgcmrQOK4y1Dn68JRdfu9SoZE5mM1MBLvpkJePr6ZP7EjD366u239blryB4EUAP8ZijKCMDBlNFitMdGg7Jh6SvmAs2jp0ou50ozL4f0jkDNCWmXS9sGkGrQlgJqTLVPwZo6d9R6kLQNeFvhJifqfQ/4yWNNAig2Js58LJ2cr5Ob7BYEJ0w2tqqap8cR6NRY7rBUkU7MubjcgnrMvUWIln+YPVaCkOpVsTieNhR+QoeLufW19VTXFPV6SknJLZ2xHeowYGMix+2ANOtjkSN+EzXVU8TTgt2qwe8DMIWjnoMFFyUA==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TZIfooylHcdMNY/Ys6N0MOoDVyJ+Pi3zIlsbgwWz0fVI4eDdzvXofGbIz2L/NqpV/qw3quySZkGoJyaJZL+CQVjunUpzShRBX+IlciLEEhTM/my/MJrjAI62EP+t9Wwis9BgMKjqg39Ic4Reizjsr+eKdML/7s1r00azsRkreTWHRz5MtYo8W1Idb5ZWpZVc049iiEGZRmXDAQekiVGUpbjHy72h89DzhM0zgRq97+3cDLraJAxx5jE2q/9XY7HTAgIKZLTo3edQEGRRR5erFAVZViXHVohW1pnpgm9iftYQ2oMnsnSXrdot9/TkddYsIEgfPbJcJFK9Uzo3Pumg9w==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: Julien Grall <julien@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bob Eshleman <bobbyeshleman@xxxxxxxxx>, Alistair Francis <alistair.francis@xxxxxxx>, Connor Davis <connojdavis@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- Delivery-date: Wed, 31 May 2023 12:56:45 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 31.05.2023 12:40, Oleksii wrote:
> On Tue, 2023-05-30 at 18:00 +0200, Jan Beulich wrote:
>> On 29.05.2023 14:13, Oleksii Kurochko wrote:
>>> +static uint32_t read_instr(unsigned long pc)
>>> +{
>>> + uint16_t instr16 = *(uint16_t *)pc;
>>> +
>>> + if ( GET_INSN_LENGTH(instr16) == 2 )
>>> + return (uint32_t)instr16;
>>> + else
>>> + return *(uint32_t *)pc;
>>> +}
>>
>> As long as this function is only used on Xen code, it's kind of okay.
>> There you/we control whether code can change behind our backs. But as
>> soon as you might use this on guest code, the double read is going to
>> be a problem (I think; I wonder how hardware is supposed to deal with
>> the situation: Maybe they indeed fetch in 16-bit quantities?).
> I'll check how the hardware fetches instructions.
>
> I am trying to figure out why the double-read can be a problem. It
> looks pretty safe to read 16 bits ( they will be available for any
> instruction length with the assumption that the minimal instruction
> length is 16 ), then check the length of the instruction, and if it is
> 32-bit instruction, read it as uint32_t.
Simply consider what happens if a buggy or malicious entity changes the
code between the two reads. And not just with the detection of "break"
in mind that you use it for here.
Jan
|
