From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 49D3CC433EF for ; Wed, 27 Oct 2021 21:15:42 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E8DB6610C7 for ; Wed, 27 Oct 2021 21:15:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E8DB6610C7 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 3C3926B0071; Wed, 27 Oct 2021 17:15:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 34C406B0072; Wed, 27 Oct 2021 17:15:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1C680940007; Wed, 27 Oct 2021 17:15:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0137.hostedemail.com [216.40.44.137]) by kanga.kvack.org (Postfix) with ESMTP id E4A136B0071 for ; Wed, 27 Oct 2021 17:15:40 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 759192DD6D for ; Wed, 27 Oct 2021 21:15:40 +0000 (UTC) X-FDA: 78743474040.17.3C64D80 Received: from mail-lf1-f44.google.com (mail-lf1-f44.google.com [209.85.167.44]) by imf16.hostedemail.com (Postfix) with ESMTP id DC402F00009A for ; Wed, 27 Oct 2021 21:15:34 +0000 (UTC) Received: by mail-lf1-f44.google.com with SMTP id y26so8903000lfa.11 for ; Wed, 27 Oct 2021 14:15:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ZUzj8OdRUyoCesvY9oYCJsi3ZMdPAltpXbUySPTyBuc=; b=ZIKZfwDyLiBXM4FTpiIdXUlr5R+dGI4KZkzsZUVmNpqPvYo82Eg9+mHUocDVdvsZRp bHC9ooluOVD3eOGj7LPilAH9no8kzTwRRuYP+1gemifBf9EmDbJHIc5OrlyAX3AErpYL rCUkbQ4oC/0/RC8tdZ8NSo7568axnml9rN/sSJ5mA2MGDa5CgiyJRlLuhtN1U3Q4WSCG AnMNeoEa0RpKePUuZlZiGhzIsuYPu5BLCjJhMGzARP7vYLxtIXT7UyovYQkTwrt82WqU 9RBNxTpyNJrocw3u98bizPwdzQxbEvMV670aQO7grbt5qAOhoDZmzAPqEDsUVxS/KD76 ZNrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ZUzj8OdRUyoCesvY9oYCJsi3ZMdPAltpXbUySPTyBuc=; b=NejPAoY+0eTUB1/nmxUIGdDPpsgp8BXr7MwUyrB4qQ9fudBLUs8pkrEDxYdc7iWoLl ulOffMavqPvmPuxQqkNfQo5ysmvx9CJYb45rlnrr+no0pPW9UU5LvygCbBhiJZN05fCw gKeg9pix/O6V2c61KuHprs/91qNW7VsySaoMcIIQ7SirhI8fIxLZ3n9A6tkCAR38Ia3W 13SpZYD45xtRayxrvVOdWxbT9oIUTdUnCAbpxX60hJHXiReyMCI+/a30CIrqM4AS60VZ G/VK62ziVBmoUBIYNZcXrf79Rq3fDI16bmMq+lcLYl7mDzbcSi/7s6p/LMPyMs3qJrau c+pA== X-Gm-Message-State: AOAM5315s5vzXwh3DTvSiTQgqL1NtaZojFE3OljpzwBXA8iGJ/8g3Oez Oc9qX8N14qJHkswLNxIhqQoTd/aH/T7k7pC+NKG4hLwMMpAIpg== X-Google-Smtp-Source: ABdhPJybiFy4oIGNhhlBmd8VY61nM798zr2vK1FyUgViR6EzKUCgaGgxcLv7n9sV9CUY4RPDWZNVBYEO2FFLs9wKsoY= X-Received: by 2002:a05:6512:39d1:: with SMTP id k17mr95057lfu.79.1635369338308; Wed, 27 Oct 2021 14:15:38 -0700 (PDT) MIME-Version: 1.0 References: <20211008180453.462291-1-brijesh.singh@amd.com> <20211008180453.462291-41-brijesh.singh@amd.com> <943a1b7d-d867-5daa-e2e7-f0d91de37103@amd.com> In-Reply-To: From: Peter Gonda Date: Wed, 27 Oct 2021 15:15:26 -0600 Message-ID: Subject: Re: [PATCH v6 40/42] virt: Add SEV-SNP guest driver To: Brijesh Singh Cc: x86@kernel.org, LKML , kvm list , linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org, linux-coco@lists.linux.dev, linux-mm@kvack.org, Thomas Gleixner , Ingo Molnar , Joerg Roedel , Tom Lendacky , "H. Peter Anvin" , Ard Biesheuvel , Paolo Bonzini , Sean Christopherson , Vitaly Kuznetsov , Jim Mattson , Andy Lutomirski , Dave Hansen , Sergio Lopez , Peter Zijlstra , Srinivas Pandruvada , David Rientjes , Dov Murik , Tobin Feldman-Fitzthum , Borislav Petkov , Michael Roth , Vlastimil Babka , "Kirill A . Shutemov" , Andi Kleen , "Dr . David Alan Gilbert" , tony.luck@intel.com, Marc Orr , sathyanarayanan.kuppuswamy@linux.intel.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: DC402F00009A Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=ZIKZfwDy; spf=pass (imf16.hostedemail.com: domain of pgonda@google.com designates 209.85.167.44 as permitted sender) smtp.mailfrom=pgonda@google.com; dmarc=pass (policy=reject) header.from=google.com X-Stat-Signature: dwpodupyjq4hcay4qff1ecuow1yo3oek X-Rspamd-Server: rspam06 X-HE-Tag: 1635369334-510257 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Oct 27, 2021 at 3:13 PM Brijesh Singh wrote: > > > > On 10/27/21 4:05 PM, Peter Gonda wrote: > .... > > >>>>> > >>>>> Thanks for updating this sequence number logic. But I still have some > >>>>> concerns. In verify_and_dec_payload() we check the encryption header > >>>>> but all these fields are accessible to the hypervisor, meaning it can > >>>>> change the header and cause this sequence number to not get > >>>>> incremented. We then will reuse the sequence number for the next > >>>>> command, which isn't great for AES GCM. It seems very hard to tell if > >>>>> the FW actually got our request and created a response there by > >>>>> incrementing the sequence number by 2, or if the hypervisor is acting > >>>>> in bad faith. It seems like to be safe we need to completely stop > >>>>> using this vmpck if we cannot confirm the PSP has gotten our request > >>>>> and created a response. Thoughts? > >>>>> > >>>> > >>>> Very good point, I think we can detect this condition by rearranging the > >>>> checks. The verify_and_dec_payload() is called only after the command is > >>>> succesful and does the following checks > >>>> > >>>> 1) Verifies the header > >>>> 2) Decrypts the payload > >>>> 3) Later we increment the sequence > >>>> > >>>> If we arrange to the below order then we can avoid this condition. > >>>> 1) Decrypt the payload > >>>> 2) Increment the sequence number > >>>> 3) Verify the header > >>>> > >>>> The descryption will succeed only if PSP constructed the payload. > >>>> > >>>> Does this make sense ? > >>> > >>> Either ordering seems fine to me. I don't think it changes much though > >>> since the header (bytes 30-50 according to the spec) are included in > >>> the authenticated data of the encryption. So any hypervisor modictions > >>> will lead to a decryption failure right? > >>> > >>> Either case if we do fail the decryption, what are your thoughts on > >>> not allowing further use of that VMPCK? > >>> > >> > >> We have limited number of VMPCK (total 3). I am not sure switching to > >> different will change much. HV can quickly exaust it. Once we have SVSM > >> in-place then its possible that SVSM may use of the VMPCK. If the > >> decryption failed, then maybe its safe to erase the key from the secrets > >> page (in other words guest OS cannot use that key for any further > >> communication). A guest can reload the driver will different VMPCK id > >> and try again. > > > > SNP cannot really cover DOS at all since the VMM could just never > > schedule the VM. In this case we know that the hypervisor is trying to > > mess with the guest, so my preference would be to stop sending guest > > messages to prevent that duplicated IV usage. If one caller gets an > > EBADMSG it knows its in this case but the rest of userspace has no > > idea. Maybe log an error? > > > >> > > Yap, we cannot protect against the DOS. This is why I was saying that we > zero the key from secrets page so that guest cannot use that key for any > future communication (whether its from rest of userspace or kexec > kernel). I can update the driver to log the message and ensure that > future messages will *not* use that key. The VMPCK ID is a module > params, so a guest can reload the driver to use different VMPCK. Duh! Sorry I thought you said we needed a VMPL0 SVSM to do that. That sounds great. > > > >> thanks