Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Cc:     brijesh.singh@amd.com, x86@kernel.org,
        linux-kernel@vger.kernel.org, kvm list <kvm@vger.kernel.org>,
        linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
        linux-coco@lists.linux.dev, linux-mm@kvack.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Sergio Lopez <slp@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Dov Murik <dovmurik@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Borislav Petkov <bp@alien8.de>,
        Michael Roth <michael.roth@amd.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Andi Kleen <ak@linux.intel.com>, tony.luck@intel.com,
        Marc Orr <marcorr@google.com>,
        sathyanarayanan.kuppuswamy@linux.intel.com
Subject: Re: [PATCH Part1 v5 34/38] x86/sev: Add snp_msg_seqno() helper
To:     Peter Gonda <pgonda@google.com>
References: <20210820151933.22401-1-brijesh.singh@amd.com>
 <20210820151933.22401-35-brijesh.singh@amd.com>
 <CAMkAt6qQOgZVEMQdMXqvs2s8pELnAFV-Msgc2_MC5WOYf8oAiQ@mail.gmail.com>
 <4742dbfe-4e02-a7e3-6464-905ccc602e6c@amd.com>
 <CAMkAt6pT4vkgLxTN1Lj54ufaStyCHHitNaHAdZvEgDV8Nyrx-Q@mail.gmail.com>
 <f54f3fc3-14c3-5c36-2712-62eb625a958b@amd.com>
 <CAMkAt6oG0L4an-VgsADxz1xs6-wq3x4Qpxk9BdP=F2W3uT8U4g@mail.gmail.com>
From:   Brijesh Singh <brijesh.singh@amd.com>
Message-ID: <7ddc78d4-745d-217e-11f1-77643863e6ee@amd.com>
Date:   Thu, 9 Sep 2021 14:26:49 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
In-Reply-To: <CAMkAt6oG0L4an-VgsADxz1xs6-wq3x4Qpxk9BdP=F2W3uT8U4g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Lzc5UGRKbXNkcnZGeFVxNjNjRkxhNXNiMDByU2JxNDhvSGVUKzFNdGNWUTU5?=
 =?utf-8?B?dklBVHZucXlSck90SzQ2NjF5ODUrVmUrMS85TTNhWWovSFYzNmUvbHdnWWdG?=
 =?utf-8?B?OUNINUYxanBPSzZWZ2Z3d3Z1MUw1b0NZN3RWc0U3eTVHL1V1UVdDVUlGeDg5?=
 =?utf-8?B?dEhIKzhoMTZIT2hJWEM5Uit5eU90ckxIakEycTJEQi9nUkR2ckdESDR4KzRi?=
 =?utf-8?B?WFZjYnJVZlVCTnI3NCtZZnR1Tko2VHBzTXFTWHc4cGFUNS84TFFySlJXajVC?=
 =?utf-8?B?VG9ZaG5oZGtVMS9xTG1NWUZLcVdOQ3JkRXFmL3Q4L08xS01xcjdRc0xYazJG?=
 =?utf-8?B?VmphbzBMSTJpOVhNQkN4K0RFWDYvczlpU3Y0L2ZrVkxlaUdXT2hYT3dJSUV5?=
 =?utf-8?B?TnVVUVU5elJXdEhrQlBoMmRqL01vMHB4VUQ0WjhUUU0xbmxjZlFaNG45bU54?=
 =?utf-8?B?WGJWcGU3V0RpZk1IcHJ6Z2hMVFNLV2NSVHJjR1FHaW9JMjlUcE94MFg3S1ZZ?=
 =?utf-8?B?Wk43VUJOa2pSKzNKc3plbGRKNkcxM3RoLzVjTXhpY3lDaThqWTZSY2Y1TlhC?=
 =?utf-8?B?bjdpTzNEdXNIR0k3ekhiWnh5N2NDZlF6WU41TjNBMEcvb2lvOUlMZlF3RlJB?=
 =?utf-8?B?MUJ5RXN0bFRpK0trdEJVVlFtTUd3c0RCVmRCc1F5eDBJODZYSTFoblp1bUs4?=
 =?utf-8?B?a29icnZBMnBhQkF2NlpSbkdwMXJuRnJsYnlIL1NKbWJxaEI1U2h2cGRSQlNs?=
 =?utf-8?B?SERjczI1QUVxd0NIZXN0NkxOVVdYNjEyTThVSk9QcThUWkJLcU01NzdtQk9E?=
 =?utf-8?B?U3I2Y0ZYQjZROWovbS96UGMxNVZqRGZ5V2VTT1c5dlorOWNHM2wzMFdqR21E?=
 =?utf-8?B?bktROUVpZFZqekNiUEZDVjBuMmdWbkRnU25sUXE5KyszVnNkSUN2SlRVbnR3?=
 =?utf-8?B?NmZFNjk2NjMvZmczeTQ0MEFhZnVBY1lpOHg4SHBsRVEyQWVsczlTUUxDUlZB?=
 =?utf-8?B?cHVibnRqRXhJSzUxUTdmZUpSNithYnU3MGZVeFh3N3V4MmhZL2FWbXNwMmtp?=
 =?utf-8?B?VGJLN2lISHZCV0xVNlo1T243ZDRLWTdJcUFpNGZEbm5RR3FZZWU3RWhWTHlM?=
 =?utf-8?B?TC9rY2EzOURzeFVxOS9KRzR2SGR2QkY2RzZHaUFhZzJ0aGJyZUVIdXpkelJB?=
 =?utf-8?B?OEJzd0JSZ0RkTmk5c1Z4ZzZ5dUs1QXNWUUdpbDB5bmphSUx1N2VqUTdXaXht?=
 =?utf-8?B?TDRwOFhFczVHU0ZYc1FtcjlJc0JiM29VM2M4RDFrRmlIQjRCMFM4eWZhNWlH?=
 =?utf-8?B?NHlqZExDbHFTcC9CZUxuaFFTbWJCWUN2RUNBTlRFTWZDWm5PdlorT2dmYmln?=
 =?utf-8?B?dlA0SjQxRmJyMGE0aXNyNW9iNjdsK2ZoVW9WMjRSYWx3QytqbGM3RmY5eXA5?=
 =?utf-8?B?N2JQeEVLT0dNdVNUR2ZJWFVsbzUwMFJlcFdCMVp0M0MyWUlRYXFMcU9wWlZ1?=
 =?utf-8?B?WEczeGJVWmcrK054UmllU2wwamdDL1FWWEJGcGU2a3NxYUlSWSswKzQ2MzB1?=
 =?utf-8?B?YXBPcjlMYThSY05pbFhLSFU5Zmx3RDFka1VUblZ1dUoxNmlyZHBzc1dxVldi?=
 =?utf-8?B?TkowYWVYMnd0N2tHMFpYYXhaU0kxNnJiU0xMaldXWGo3emZwbytKcHR5Qnk1?=
 =?utf-8?B?eDZPZzlEaEN4UmtTaUxGRlVHbDB1SnVzYzNYNTZ0VmJzOERidUV1cXU0QWE1?=
 =?utf-8?Q?O/2sxDGr0JBNio7Bzo9HihYuP/G1khHgaTgj9TM?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e0e94c00-63ae-416f-1e64-08d973c7c6cf
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Sep 2021 19:26:52.4762
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: meFqzL+zls7RCZAi32i3VJ1wW/jMTkV7iKeOyxaCTt3wbRx+fi+giHRW4bdvDiw09e1fPrR27QTAICj0mnx5aQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR12MB4431
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 9/9/21 11:21 AM, Peter Gonda wrote:
> On Thu, Sep 9, 2021 at 10:17 AM Brijesh Singh <brijesh.singh@amd.com> wrote:
>>
>>
>>
>> On 9/9/21 10:43 AM, Peter Gonda wrote:
>> ...
>>
>>>>
>>>> Does this address your concern?
>>>
>>> So the 'snp_msg_seqno()' call in 'enc_payload' will not increment the
>>> counter, its only incremented on 'snp_gen_msg_seqno()'? If thats
>>> correct, that addresses my first concern.
>>>
>>
>> Yes, that is goal.
>>
>>>>>
>>>>
>>>> So far, the only user for the snp_msg_seqno() is the attestation driver.
>>>> And the driver is designed to serialize the vmgexit request and thus we
>>>> should not run into concurrence issue.
>>>
>>> That seems a little dangerous as any module new code or out-of-tree
>>> module could use this function thus revealing this race condition
>>> right? Could we at least have a comment on these functions
>>> (snp_msg_seqno and snp_gen_msg_seqno) noting this?
>>>
>>
>> Yes, if the driver is not performing the serialization then we will get
>> into race condition.
>>
>> One way to avoid this requirement is to do all the crypto inside the
>> snp_issue_guest_request() and eliminate the need to export the
>> snp_msg_seqno().
>>
>> I will add the comment about it in the function.
> 
> Actually I forgot that the sequence number is the only component of
> the AES-GCM IV. Seen in 'enc_payload'. Given the AES-GCM spec requires
> uniqueness of the IV. I think we should try a little harder than a
> comment to guarantee we never expose 2 requests encrypted with the
> same sequence number / IV. It's more than just a DOS against the
> guest's PSP request ability but also could be a guest security issue,
> thoughts?
> 

Ah good point, we should avoid a request with same IV. May be move the 
sequence number increment and save in sevguest drv. Then driver can do 
the sequence get, vmgexit and increment with a protected lock.

thanks

> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnvlpubs.nist.gov%2Fnistpubs%2FLegacy%2FSP%2Fnistspecialpublication800-38d.pdf&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C46a05f4713834307706608d973ade616%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637668013461202204%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=KCsi5rTQX6L%2BqY07VdBtF8IH0TLNyHn6wTyidgWvXf4%3D&amp;reserved=0
> (Section 8 page 18)
> 
>>
>> thanks
>