linux-kernel - RE: [PATCH V4 2/3] x86/efi: Add efi page fault handler to recover from page faults caused by the firmware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Sep 2018 18:29:07 +0000
From:   "Prakhya, Sai Praneeth" <sai.praneeth.prakhya@...el.com>
To:     Peter Zijlstra <peterz@...radead.org>
CC:     "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "Neri, Ricardo" <ricardo.neri@...el.com>,
        "matt@...eblueprint.co.uk" <matt@...eblueprint.co.uk>,
        Al Stone <astone@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Ingo Molnar <mingo@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Bhupesh Sharma <bhsharma@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: RE: [PATCH V4 2/3] x86/efi: Add efi page fault handler to recover
 from page faults caused by the firmware

> > +	if (efi_rts_work.efi_rts_id == RESET_SYSTEM) {
> > +		pr_info("efi_reset_system() buggy! Reboot through BIOS\n");
> > +		machine_real_restart(MRR_BIOS);
> > +		return 0;
> > +	}
> > +
> > +	/* Firmware has caused page fault, hence, freeze efi_rts_wq. */
> > +	set_current_state(TASK_UNINTERRUPTIBLE);
> 
> This doesn't freeze it, as such, it just sets the state.

True! Thanks for pointing it out. I will update the comment.

> > +
> > +	/*
> > +	 * Before calling EFI Runtime Service, the kernel has switched the
> > +	 * calling process to efi_mm. Hence, switch back to task_mm.
> > +	 */
> > +	arch_efi_call_virt_teardown();
> > +
> > +	/* Signal error status to the efi caller process */
> > +	efi_rts_work.status = EFI_ABORTED;
> > +	complete(&efi_rts_work.efi_rts_comp);
> > +
> > +	clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
> > +	pr_info("Froze efi_rts_wq and disabled EFI Runtime Services\n");
> 
> > +	schedule();
> 
> So what happens when we get a spurious wakeup and return from this?
> 
> Quite possibly you want something like:
> 
> 	for (;;) {
> 		set_current_state(TASK_IDLE);
> 		schedule();
> 	}
> 
> here. The TASK_UNINTERRUPTIBLE thing will cause the load-avg to spike; is that
> what you want?

Yes, makes sense. TASK_IDLE seems more appropriate. I will change it.

Regards,
Sai