Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <bc070ef7-8168-f1fc-f5ec-aeac204f2ef6@amd.com>
Date:   Wed, 16 Nov 2022 10:58:33 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH V4] virt: sev: Prevent IV reuse in SNP guest driver
Content-Language: en-US
To:     Peter Gonda <pgonda@google.com>, Borislav Petkov <bp@suse.de>
Cc:     Dionna Glaze <dionnaglaze@google.com>,
        Michael Roth <michael.roth@amd.com>,
        Haowen Bai <baihaowen@meizu.com>,
        Yang Yingliang <yangyingliang@huawei.com>,
        Marc Orr <marcorr@google.com>,
        David Rientjes <rientjes@google.com>,
        Ashish Kalra <Ashish.Kalra@amd.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
References: <20221103152318.88354-1-pgonda@google.com>
 <Y258U+8oF/eo14U+@zn.tnic>
 <CAMkAt6o-jcG7u1=zw4jJp5evrO4sFJR-iG_ApF7LhT+7c55_Wg@mail.gmail.com>
 <Y3TVcJnQ/Ym6dGz2@zn.tnic>
 <CAMkAt6qQmkufbuotzMA4bMJaA4uBFMdk8w7a3X+OH3JaOdFepA@mail.gmail.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <CAMkAt6qQmkufbuotzMA4bMJaA4uBFMdk8w7a3X+OH3JaOdFepA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZklaS3Zra0h1R0Zwa0dnRDlFMlo3RTZycVNBRDhETFp3bElEVTFTRDhKSzBS?=
 =?utf-8?B?bk11S1pDRFY2bnVpZmFzUkI1VHFNcHNpZHJTNTN3c3VsbVZvbUNCTnlsSUFZ?=
 =?utf-8?B?eUFUMWxmRDRFQjlPUEF3S1cyN1JMd2x6aE5scjNFNDNINUE4d2hNN1U0Smpr?=
 =?utf-8?B?SGs0V1VDOFJOV3oxcGpmUWJnem13alUyTVpLRWEvWVRYVWlSeUNyUXlmcEVG?=
 =?utf-8?B?UkQyZWRiUWFOVVhObnY0WkRCTlJORnpmWmpPY2VreE9ndDBvbXhmdHJReHVS?=
 =?utf-8?B?SEF2ZjBzbGd0b0tkalZob2g1bC9KQmYrSVZRcFN1MzdUeGdLZDQ2TkZ4RTMz?=
 =?utf-8?B?Q2crQnE0eC9QUmtyYWJSRFlrZDA4emhYcDArZTdKWE9nd2VadGRCQW95eTR2?=
 =?utf-8?B?Zk9GTHhtY2xOdG1IdHZCaTFjS0F6YW1oRzNtOStSZDd5anI0T0ZJK1UxazY5?=
 =?utf-8?B?WndGMnBzRGlvL1E5MzdCWlAyS3c2eWVKUitUOHRGeEx4RDB6dFFsS1cwR2xK?=
 =?utf-8?B?bi9WYms1aXZtYWRyNWgwYUIxVmw3ZkV1NXRFWHVhRTlQMVF1ZXhUU2twQUFo?=
 =?utf-8?B?VVpBdDl0R2IwUk5lK0ZQc25FTDJJWU1oVmRGKzN2WVphcHB2NVA1K0cwWWp5?=
 =?utf-8?B?cXdicXk3R3VmbXo5Vm92TEkvWTRmMmF5MWgyNXptbG5pT1lqS0UyVlZ6Sk14?=
 =?utf-8?B?YjdwOWNFQXJSMnFWamlIRTdWVzUwR0g5dkxldXk1aTl2aEk4cDdGZGRaV1Zn?=
 =?utf-8?B?SEZhdUorZC8ySHh2UHpOYUEvdjhxamdIRGVXbnBXRkkwa3VHaHhpUUhCZElK?=
 =?utf-8?B?YlFrVGQzaEVnR1Rqa1d0RHlpNmRKZGZsNkRIZHRyY3RLcEx5WnE5NGg1U3Yr?=
 =?utf-8?B?UysxRzlZbjBXQmo2SjRpOGZTYitReUJIYzVHTENuTDRhTzd2MHQ0VlV1NWRX?=
 =?utf-8?B?ck5FK0xsZ0NhTVJBNDk4NldwM3J2eDNMVzdmc1VzSnFRVlhFZHBQTytPSEV4?=
 =?utf-8?B?QXYwdUIwaU9XVnVjay9aa3hHOVFKZzRIU20ra2ZZOGFJRTZBMjJLNVFqczFl?=
 =?utf-8?B?Q3dyZWUwM3h3U0grbUpTWjVVL2RweXVLU2xZczY4ak1pU1BEV0FKa1VsUktl?=
 =?utf-8?B?ZEhYeDBMcVV0THhOVU9PK3dDcm0yVXRLcVJGNWQwUUJWOVoyZWxYVVRIazZD?=
 =?utf-8?B?VnFxcUJQOEMybGxhZWY3VEZMZVpxMXNialAzdlZjNERYQ2kyY2NDclBvQ3Uz?=
 =?utf-8?B?TGcyZEhFb0NzNGxZQjdVeDI4R0JzZmMrN00wblBCTXdncDg1QlhMenA4MmNE?=
 =?utf-8?B?aHBWM0NLK3hMZm5vWWxFRnpHMFphVS83bVF0UHdSTjh2MnlVd0xEa3k1cjN3?=
 =?utf-8?B?cmZHN3Jkb0w3NTBsZjVyYWtqWm50RnF5S2l3RjBUWHcrYWFyQmR6ZW1DTC9z?=
 =?utf-8?B?eGh3TjRCWWoxaU1wNWZqVS85bnVNcGRBcVh3ZFpTNitSVEJIV3pudW1HcFpY?=
 =?utf-8?B?KzRsV3hrMWZuV3lIUlcyK0ZXZXN2d0lLK1IzNGRHbUdTMU5ETmV5TVpqVjFp?=
 =?utf-8?B?V1c4eEM2VGd6ekJLYng2eVdaTkdlNElHQlJlYkk5NDhWMUpXZmtkWjd6NHll?=
 =?utf-8?B?dlpOa29HNWphejJpd3dXK2wvTExpeUxZYTJhdTFMOEhEL0hINHAremF0THBw?=
 =?utf-8?B?N2cxVzJyUmh5clpha3hxeFJjWGxST1V0V1pia2ZaMFBOY1hxTCtva0JXZzNR?=
 =?utf-8?B?cVYrcUJlZFRKLzVRTnd2dXJJNklDbEliT3VuYmZST21JcGM0T2lua08zU09W?=
 =?utf-8?B?YlMxQmppa25PS1RPUjh0Q2w4b0Q3MnAxVTl0VXhaR0ZRZDB1eGY2NEptcjVk?=
 =?utf-8?B?V1hvbmozeSs4WldlT3lxamJyRmlsWDZ3N2U0WXZDTFRwRFZaN0pTYllDcThn?=
 =?utf-8?B?ZGt6U1NqMjZBSGN3cHJNRE9rWUIwTTVzY2Q0cFJGOXNHSDNua0ZDYk43RjFO?=
 =?utf-8?B?WC9NcXJhVHhVdnc4RFFiWTFoOHF2eXI3Qy9KWDlsTXBkY0pzZS9uWXpOanhq?=
 =?utf-8?B?dE8rU052THZvZGRkdzNlSlhtZnhUaFRlRERnMDM5M3ExRlk0a2dwRHlNcmp1?=
 =?utf-8?Q?hHnrc+lhkTncftkTLO2zjZ0lk?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ff5ec1d7-eb8f-46a6-72ad-08dac7f3cce4
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Nov 2022 16:58:35.5787
 (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: 3zifmhqgocaysaRNBo4rp56CPnlu1kjrjBZb1xqgoHBc3iDhc6I6lSpznmDiNuNSAv8+8gUcV79qz21ApEwlmg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB6191
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/16/22 10:23, Peter Gonda wrote:
> On Wed, Nov 16, 2022 at 5:20 AM Borislav Petkov <bp@suse.de> wrote:
>>
>> On Tue, Nov 15, 2022 at 02:47:31PM -0700, Peter Gonda wrote:
>>>>> +      * certificate data buffer retry the same guest request without the
>>>>> +      * extended data request.
>>>>> +      */
>>>>> +     if (exit_code == SVM_VMGEXIT_EXT_GUEST_REQUEST &&
>>>>> +         err == SNP_GUEST_REQ_INVALID_LEN) {
>>>>> +             const unsigned int certs_npages = snp_dev->input.data_npages;
>>>>> +
>>>>> +             exit_code = SVM_VMGEXIT_GUEST_REQUEST;
>>>>> +             rc = snp_issue_guest_request(exit_code, &snp_dev->input, &err);
>>>>> +
>>>>> +             err = SNP_GUEST_REQ_INVALID_LEN;
>>>>
>>>> Huh, why are we overwriting err here?
>>>
>>> I have added a comment for the next revision.
>>>
>>> We are overwriting err here so that userspace is alerted that they
>>> supplied a buffer too small.
>>
>> Sure but you're not checking rc either. What if that reissue fails for
>> whatever other reason? -EIO for example...
> 
> If we get any error here we have to wipe the VMPCK here so I thought

More accurate to say that you will wipe the VMPCK, since the value of rc 
is checked a bit further down in the code and the -EIO (or other non-zero) 
will be result in a call to snp_disable_vmpck() and rc being propagated 
back to the user as an ioctl() return code.

Might be worth a comment above that second snp_issue_guest_request() 
explaining that.

> this always override @err was OK.
> 
> I can update this to only override @err if after the secondary
> SVM_VMGEXIT_GUEST_REQUEST rc and err are OK. Thoughts?

I think it's ok to set it no matter what, but I don't have a strong 
opinion either way.

Thanks,
Tom

> 
>>
>> --
>> Regards/Gruss,
>>      Boris.
>>
>> SUSE Software Solutions Germany GmbH
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
>> (HRB 36809, AG Nürnberg)