linux-kernel - Re: [PATCH v12 2/3] x86/sev: Change snp_guest_issue_request's fw

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:   Mon, 23 Jan 2023 13:22:07 -0800
From:   Dionna Amalie Glaze <dionnaglaze@...gle.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Tom Lendacky <Thomas.Lendacky@....com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Joerg Roedel <jroedel@...e.de>,
        Peter Gonda <pgonda@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Venu Busireddy <venu.busireddy@...cle.com>,
        Michael Roth <michael.roth@....com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Michael Sterritt <sterritt@...gle.com>
Subject: Re: [PATCH v12 2/3] x86/sev: Change snp_guest_issue_request's fw_err

>
> Anyway, that's a lot of changes for a fix which needs to go to stable. I don't
> mind them but not in a minimal fix.
>

> So how is this for a minimal fix to go in now, ontop of your first patch? The
> cleanups can then go later...
>
> ---
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 4ec4174e05a3..20b560a45bc1 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -322,7 +322,7 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code, in
>                                 u8 type, void *req_buf, size_t req_sz, void *resp_buf,
>                                 u32 resp_sz, __u64 *fw_err)
>  {
> -       unsigned long err;
> +       unsigned long err = SEV_RET_NO_FW_CALL;
>         u64 seqno;
>         int rc;
>

This isn't the primary problem that needs fixing, although it is part
of it. The problem is that the host can provide a throttling error and
the guest will need to continue trying the exact same request or else
end up locking themself out of the vmpck due to the IV reuse patch
Peter sent.

I think Sean's request to keep throttling a host problem in user space
is not the right one in this case. That would avoid scheduling the
whole vCPU, but the guest code I'm proposing can do other useful work
while waiting. There will be no other code that depends on that
particular control flow.

If we call this one-liner the fix, then the VM guest still has the IV
reuse problem unless it can guarantee that globally no other thread
than the one requesting an attestation report will continue to retry
upon a throttling error in the exitinfo2 value. I wouldn't call that a
fix.

-- 
-Dionna Glaze, PhD (she/her)