linux-kernel - Re: [PATCH v13 26/35] x86/fred: FRED entry/exit and dispatch code

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]

Message-ID: <4e41b658-f49e-424c-8a86-08c8ab8e384d@citrix.com>
Date:   Wed, 6 Dec 2023 14:11:46 +0000
From:   Andrew Cooper <andrew.cooper3@...rix.com>
To:     "Li, Xin3" <xin3.li@...el.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>
Cc:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        "Lutomirski, Andy" <luto@...nel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "Gross, Jurgen" <jgross@...e.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        "mhiramat@...nel.org" <mhiramat@...nel.org>,
        "jiangshanlai@...il.com" <jiangshanlai@...il.com>,
        "nik.borisov@...e.com" <nik.borisov@...e.com>,
        "Kang, Shan" <shan.kang@...el.com>
Subject: Re: [PATCH v13 26/35] x86/fred: FRED entry/exit and dispatch code

On 06/12/2023 7:45 am, Li, Xin3 wrote:
>>> +	case X86_TRAP_OF:
>>> +		exc_overflow(regs);
>>> +		return;
>>> +
>>> +	/* INT3 */
>>> +	case X86_TRAP_BP:
>>> +		exc_int3(regs);
>>> +		return;
>> ... neither OF nor BP will ever enter fred_intx() because they're type SWEXC not
>> SWINT.
> Per FRED spec 5.0, section 7.3 Software Interrupts and Related Instructions:
> INT n (opcode CD followed by an immediate byte): There are 256 such
> software interrupt instructions, one for each value n of the immediate
> byte (0–255).
>
> And appendix B Event Stack Levels:
> If the event is an execution of INT n (opcode CD n for 8-bit value n),
> the event stack level is 0. The event type is 4 (software interrupt)
> and the vector is n.
>
> So int $0x4 and int $0x3 (use asm(".byte 0xCD, 0x03")) get here.
>
> But into (0xCE) and int3 (0xCC) do use event type SWEXC. 
>
> BTW, into is NOT allowed in 64-bit mode but "int $0x4" is allowed.

There is certainly fun to be had with CD 03 and CD 04 byte patterns, but
if you meant to mean those here, then the comments are wrong.

Vectors 3 and 4 are installed with DPL3 because that is necessary to
make CC and CE function in userspace.  It also suggests that the SWINT
vs SWEXC distinction was retrofitted to architecture after the 286,
because exceptions don't check DPL and ICEBP delivers #DB from userspace
even when Vector 1 has a DPL of 0.

While CC is for most cases indistinguishable from CD 03, CE behaves
entirely differently to CD 04.  CD 04 doesn't #UD in 64bit mode, and
will trigger exc_overflow() irrespective of the state of EFLAGS.OF.


The SDM goes out of it's way to say not to use the CD 03 byte pattern
(and it does take effort to emit this byte pattern - e.g. GAS will
silently translate "int $3" to "int3"), and there's no plausible way
software is using CD 04 in place of CE.

So why do we care about containing to make mistakes of the IDT era work
in a FRED world?

Is there anything (other than perhaps the selftests) which would even
notice?

>>> +		instrumentation_end();
>>> +		irqentry_exit(regs, state);
>>> +	} else {
>>> +		common_interrupt(regs, vector);
>>> +	}
>>> +}
>>> +
>>> +static noinstr void fred_exception(struct pt_regs *regs, unsigned
>>> +long error_code) {
>>> +	/* Optimize for #PF. That's the only exception which matters performance
>> wise */
>>> +	if (likely(regs->fred_ss.vector == X86_TRAP_PF)) {
>>> +		exc_page_fault(regs, error_code);
>>> +		return;
>>> +	}
>>> +
>>> +	switch (regs->fred_ss.vector) {
>>> +	case X86_TRAP_DE: return exc_divide_error(regs);
>>> +	case X86_TRAP_DB: return fred_exc_debug(regs);
>>> +	case X86_TRAP_BP: return exc_int3(regs);
>>> +	case X86_TRAP_OF: return exc_overflow(regs);
>> Depending on what you want to do with BP/OF vs fred_intx(), this may need
>> adjusting.
>>
>> If you are cross-checking type and vector, then these should be rejected for not
>> being of type HWEXC.
> You're right, the event type needs to be SWEXC for into and int3.
>
> However, would it be overkilling?  Assuming hardware and VMM are sane.

You either care about cross checking, or not.  Right now, this patch is
a mix of the two approaches.

In my opinion, cross-checking is the better approach, because it means
that violations of the assumptions get noticed more quickly, and
hopefully by whomever is working on the new feature which alters the
assumptions.

~Andrew