Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC PATCH] arch/x86: Optionally flush L1D on context switch
Date:   Sun, 22 Mar 2020 08:10:29 -0700
Message-Id: <A38682A4-62B2-4849-ADEA-196DFF4D36C9@amacapital.net>
References: <99ef5eec8502a7b53eee362063b9b2252a5a47da.camel@amazon.com>
Cc:     "tglx@linutronix.de" <tglx@linutronix.de>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Singh, Balbir" <sblbir@amazon.com>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "x86@kernel.org" <x86@kernel.org>
In-Reply-To: <99ef5eec8502a7b53eee362063b9b2252a5a47da.camel@amazon.com>
To:     "Herrenschmidt, Benjamin" <benh@amazon.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Mar 21, 2020, at 10:08 PM, Herrenschmidt, Benjamin <benh@amazon.com> wr=
ote:
>=20
> =EF=BB=BFOn Fri, 2020-03-20 at 12:49 +0100, Thomas Gleixner wrote:
>> Balbir,
>>=20
>> "Singh, Balbir" <sblbir@amazon.com> writes:
>>>> On Thu, 2020-03-19 at 01:38 +0100, Thomas Gleixner wrote:
>>>>> What's the point? The attack surface is the L1D content of the schedul=
ed
>>>>> out task. If the malicious task schedules out, then why would you care=
?
>>>>>=20
>>>>> I might be missing something, but AFAICT this is beyond paranoia.
>>>>>=20
>>>=20
>>> I think there are two cases
>>>=20
>>> 1. Task with important data schedules out
>>> 2. Malicious task schedules in
>>>=20
>>> These patches address 1, but call out case #2
>>=20
>> The point is if the victim task schedules out, then there is no reason
>> to flush L1D immediately in context switch. If that just schedules a
>> kernel thread and then goes back to the task, then there is no point
>> unless you do not even trust the kernel thread.
>=20
> A switch to a kernel thread will not call switch_mm, will it ? At least it=
 used not to...
>=20
>>>>> 3. There is a fallback software L1D load, similar to what L1TF does, b=
ut
>>>>>    we don't prefetch the TLB, is that sufficient?
>>>>=20
>>>> If we go there, then the KVM L1D flush code wants to move into general
>>>> x86 code.
>>>=20
>>> OK.. we can definitely consider reusing code, but I think the KVM bits r=
equire
>>> tlb prefetching, IIUC before cache flush to negate any bad translations
>>> associated with an L1TF fault, but the code/comments are not clear on th=
e need
>>> to do so.
>>=20
>> I forgot the gory details by now, but having two entry points or a
>> conditional and share the rest (page allocation etc.) is definitely
>> better than two slightly different implementation which basically do the s=
ame thing.
>>=20
>>>>> +void enable_l1d_flush_for_task(struct task_struct *tsk)
>>>>> +{
>>>>> +     struct page *page;
>>>>> +
>>>>> +     if (static_cpu_has(X86_FEATURE_FLUSH_L1D))
>>>>> +             goto done;
>>>>> +
>>>>> +     mutex_lock(&l1d_flush_mutex);
>>>>> +     if (l1d_flush_pages)
>>>>> +             goto done;
>>>>> +     /*
>>>>> +      * These pages are never freed, we use the same
>>>>> +      * set of pages across multiple processes/contexts
>>>>> +      */
>>>>> +     page =3D alloc_pages(GFP_KERNEL | __GFP_ZERO, L1D_CACHE_ORDER);
>>>>> +     if (!page)
>>>>> +             return;
>>>>> +
>>>>> +     l1d_flush_pages =3D page_address(page);
>>>>> +     /* I don't think we need to worry about KSM */
>>>>=20
>>>> Why not? Even if it wouldn't be necessary why would we care as this is a=

>>>> once per boot operation in fully preemptible code.
>>>=20
>>> Not sure I understand your question, I was stating that even if KSM was
>>> running, it would not impact us (with dedup), as we'd still be writing o=
ut 0s
>>> to the cache line in the fallback case.
>>=20
>> I probably confused myself vs. the comment in the VMX code, but that
>> mentions nested virt. Needs at least some consideration when we reuse
>> that code.
>>=20
>>>>>  void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>>>>>              struct task_struct *tsk)
>>>>>  {
>>>>> @@ -433,6 +519,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, st=
ruct
>>>>> mm_struct *next,
>>>>>               trace_tlb_flush_rcuidle(TLB_FLUSH_ON_TASK_SWITCH, 0);
>>>>>       }
>>>>>=20
>>>>> +     l1d_flush(next, tsk);
>>>>=20
>>>> This is really the wrong place. You want to do that:
>>>>=20
>>>>   1) Just before return to user space
>>>>   2) When entering a guest
>>>>=20
>>>> and only when the previously running user space task was the one which
>>>> requested this massive protection.
>>>>=20
>>>=20
>>> Cases 1 and 2 are handled via
>>>=20
>>> 1. SWAPGS fixes/work arounds (unless I misunderstood your suggestion)
>>=20
>> How so? SWAPGS mitigation does not flush L1D. It merily serializes SWAPGS=
.
>=20
>>> 2. L1TF fault handling
>>>=20
>>> This mechanism allows for flushing not restricted to 1 or 2, the idea is=
 to
>>> immediately flush L1D for paranoid processes on mm switch.
>>=20
>> Why? To protect the victim task against the malicious kernel?
>=20
> Mostly malicious other tasks for us. As I said, I don't think switch_mm
> is called on switching to a kernel thread and is definitely a colder
> path than the return to userspace, so it felt like the right place to
> put this, but I don't mind if you prefer it elsewhere as long as it
> does the job which is to prevent task B to snoop task A data.
>=20
>> The L1D content of the victim is endangered in the following case:
>>=20
>>    victim out -> attacker in
>>=20
>> The attacker can either run in user space or in guest mode. So the flush
>> is only interesting when the attacker actually goes back to user space
>> or reenters the guest.
>>=20
>> The following is completely uninteresting:
>>=20
>>    victim out -> kernel thread in/out -> victim in
>=20
> Sure but will that call switch_mm to be called ?
>=20
>> Even this is uninteresting:
>>=20
>>    victim in -> attacker in (stays in kernel, e.g. waits for data) ->
>>    attacker out -> victim in
>=20
> I don't get this ... how do you get attacker_in without victim_out
> first ? In which case you have a victim_out -> attacker_in transition
> which is what we are trying to protect.
>=20
> I still think flushing the "high value" process L1D on switch_mm out is
> the way to go here...

Let me try to understand the issue. There is some high-value data, and that d=
ata is owned by a high-value process. At some point, the data ends up in L1D=
.  Later in, evil code runs and may attempt to exfiltrate  that data from L1=
D using a side channel. (The evil code is not necessarily in a malicious pro=
cess context. It could be kernel code targeted by LVI or similar. It could b=
e ordinary code that happens to contain a side channel gadget by accident.)

So the idea is to flush L1D after manipulating high-value data and before ru=
nning evil code.

The nasty part here is that we don=E2=80=99t have a good handle on when L1D i=
s filled and when the evil code runs. If the evil code is untrusted process u=
serspace and the fill is an interrupt, then switch_mm is useless and we want=
 to flush on kernel exit instead. If the fill and evil code are both userspa=
ce, then switch_mm is probably the right choice, but prepare_exit_from_userm=
ode works too. If SMT is on, we lose no matter what.  If the evil code is in=
 kernel context, then it=E2=80=99s not clear what to do. If the fill and the=
 evil code are both in kernel threads (hi, io_uring), then I=E2=80=99m not a=
t all sure what to do.

In summary, kernel exit seems stronger, but the right answer isn=E2=80=99t s=
o clear.

We could do an optimized variant where we flush at kernel exit but we *decid=
e* to flush in switch_mm.