From mboxrd@z Thu Jan 1 00:00:00 1970 References: <3511af9f-f393-a226-0e41-a23d8c1577c8@siemens.com> <87ft0xfyj4.fsf@xenomai.org> <4f5118a4-d362-7f18-fdcf-da22aabb7344@siemens.com> <87czw0g5b5.fsf@xenomai.org> From: Philippe Gerum Subject: Re: [PATCH 5.4] x86: ipipe: Harden path between use_temporary_mm and unuse_temporary_mm In-reply-to: Date: Mon, 15 Mar 2021 10:47:23 +0100 Message-ID: <871rcgg34k.fsf@xenomai.org> MIME-Version: 1.0 Content-Type: text/plain List-Id: Discussions about the Xenomai project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: xenomai@xenomai.org Jan Kiszka writes: > On 15.03.21 10:00, Philippe Gerum wrote: >> >> Jan Kiszka writes: >> >>> On 14.03.21 18:14, Philippe Gerum wrote: >>>> >>>> Jan Kiszka via Xenomai writes: >>>> >>>>> From: Jan Kiszka >>>>> >>>>> This is only called during early init, e.g. for switching alternatives. >>>>> Still, switch_mm_irqs_off would complain without this, and we are better >>>>> safe than sorry. >>>>> >>>> >>>> The way this is done in Dovetail is fragile too, since the protection we >>>> have there still expects the pipeline entry code not to mess up on >>>> handling an interrupt, which defeats the purpose of such >>>> precaution. Besides, the temp_state should be snapshot under protection >>>> too. IOW, IRQs should be hard disabled fully while using the temporary >>>> mm. >>>> >>>> Upstreaming a similar patch for Dovetail. >>> >>> Just saw it: It's wrong as it left the hard_irq_save_full at the caller >>> site. Would raise a warning when debugging is enabled. Please fix. >>> >> >> I guess you mean local_irq_save_full(). After a second look, the hard >> irqs off section should cover the entire code poking new text into a >> live kernel. This is what switching to the _full() forms enforced >> already. So I see no point in changing the IRQ state in the use/unuse >> helpers eventually. > > Right, either way is needed. If the tlb flush should be fully protected, > the existing pattern is needed. But maybe more: > > Under I-pipe, playing with full disabling didn't work out well. Some > spinlock function called via get_locked_pte reenabled hard IRQs. Not > sure if dovetail is affected by that as well. > In the case at hand, the PTE of the poking address is locked and returned outside of the locked section, so this should not be a concern for live patching. -- Philippe.