linux-kernel - Re: [PATCH 4/8] x86/traps: Demand-populate PASID MSR via #GP

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:   Tue, 28 Sep 2021 23:50:37 +0000
From:   Fenghua Yu <fenghua.yu@...el.com>
To:     "Luck, Tony" <tony.luck@...el.com>
Cc:     Dave Hansen <dave.hansen@...el.com>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Joerg Roedel <joro@...tes.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Jacob Jun Pan <jacob.jun.pan@...el.com>,
        Raj Ashok <ashok.raj@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        iommu@...ts.linux-foundation.org,
        the arch/x86 maintainers <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/8] x86/traps: Demand-populate PASID MSR via #GP

Hi, Tony,

On Tue, Sep 28, 2021 at 04:10:39PM -0700, Luck, Tony wrote:
> Moving beyond pseudo-code and into compiles-but-probably-broken-code.
> 
> 
> The intent of the functions below is that Fenghua should be able to
> do:
> 
> void fpu__pasid_write(u32 pasid)
> {
> 	u64 msr_val = pasid | MSR_IA32_PASID_VALID;
> 	struct ia32_pasid_state *addr;
> 
> 	addr = begin_update_one_xsave_feature(current, XFEATURE_PASID, true);
> 	addr->pasid = msr_val;
> 	finish_update_one_xsave_feature(current);
> }
> 
> So here's the two new functions that would be added to
> arch/x86/kernel/fpu/xstate.c
> 
> ----
> 
> void *begin_update_one_xsave_feature(struct task_struct *tsk,
>                                      enum xfeature xfeature, bool full)
> {
>         struct xregs_state *xsave = &tsk->thread.fpu.state.xsave;
>         struct xregs_state *xinit = &init_fpstate.xsave;
>         u64 fmask = 1ull << xfeature;
>         void *addr;
> 
>         BUG_ON(!(xsave->header.xcomp_bv & fmask));
> 
>         fpregs_lock();
> 
>         addr = __raw_xsave_addr(xsave, xfeature);
> 
>         if (full || tsk != current) {
>                 memcpy(addr, __raw_xsave_addr(xinit, xfeature), xstate_sizes[xfeature]);
>                 goto out;
>         }
> 
> 	/* could optimize some cases where xsaves() isn't fastest option */
>         if (!(xsave->header.xfeatures & fmask))
>                 xsaves(xsave, fmask);

If xfeatures's feature bit is 0, xsaves will not write its init value to the
memory due to init optimization. So the xsaves will do nothing and the
state is not initialized and may have random data.

> 
> out:
>         xsave->header.xfeatures |= fmask;
>         return addr;
> }
> 
> void finish_update_one_xsave_feature(struct task_struct *tsk)
> {
>         set_ti_thread_flag(task_thread_info(tsk), TIF_NEED_FPU_LOAD);

Setting TIF_NEED_FPU_LOAD cannot guaranteed to execute XRSTORS on exiting
to user. In fpregs_restore_userregs():
	if (!fpregs_state_valid(fpu, cpu)) {
		...
                __restore_fpregs_from_fpstate(&fpu->state, mask);
		...
	}

fpregs state should be invalid to get the XRSTROS executed.

So setting TIF_NEED_FPU_LOAD may get the FPU register unchanged on exiting
to user.


>         fpregs_unlock();
> }

Thanks.

-Fenghua