[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7c09a269-bce0-2533-1795-9410714716aa@amd.com>
Date: Tue, 22 Oct 2024 09:45:59 +0530
From: "Nikunj A. Dadhania" <nikunj@....com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>,
linux-kernel@...r.kernel.org, thomas.lendacky@....com, bp@...en8.de,
x86@...nel.org, kvm@...r.kernel.org
Cc: mingo@...hat.com, tglx@...utronix.de, dave.hansen@...ux.intel.com,
pgonda@...gle.com, seanjc@...gle.com, pbonzini@...hat.com
Subject: Re: [PATCH v13 03/13] x86/sev: Add Secure TSC support for SNP guests
On 10/21/2024 1:42 PM, Christophe JAILLET wrote:
> Le 21/10/2024 à 07:51, Nikunj A Dadhania a écrit :
>
> ..
>
>> +static int __init snp_get_tsc_info(void)
>> +{
>> + static u8 buf[SNP_TSC_INFO_RESP_SZ + AUTHTAG_LEN];
>> + struct snp_guest_request_ioctl rio;
>> + struct snp_tsc_info_resp tsc_resp;
>> + struct snp_tsc_info_req *tsc_req;
>> + struct snp_msg_desc *mdesc;
>> + struct snp_guest_req req;
>> + int rc;
>> +
>> + /*
>> + * The intermediate response buffer is used while decrypting the
>> + * response payload. Make sure that it has enough space to cover the
>> + * authtag.
>> + */
>> + BUILD_BUG_ON(sizeof(buf) < (sizeof(tsc_resp) + AUTHTAG_LEN));
>> +
>> + mdesc = snp_msg_alloc();
>> + if (IS_ERR_OR_NULL(mdesc))
>> + return -ENOMEM;
>> +
>> + rc = snp_msg_init(mdesc, snp_vmpl);
>> + if (rc)
>> + return rc;
>> +
>> + tsc_req = kzalloc(sizeof(struct snp_tsc_info_req), GFP_KERNEL);
>> + if (!tsc_req)
>> + return -ENOMEM;
>> +
>> + memset(&req, 0, sizeof(req));
>> + memset(&rio, 0, sizeof(rio));
>> + memset(buf, 0, sizeof(buf));
>> +
>> + req.msg_version = MSG_HDR_VER;
>> + req.msg_type = SNP_MSG_TSC_INFO_REQ;
>> + req.vmpck_id = snp_vmpl;
>> + req.req_buf = tsc_req;
>> + req.req_sz = sizeof(*tsc_req);
>> + req.resp_buf = buf;
>> + req.resp_sz = sizeof(tsc_resp) + AUTHTAG_LEN;
>> + req.exit_code = SVM_VMGEXIT_GUEST_REQUEST;
>> +
>> + rc = snp_send_guest_request(mdesc, &req, &rio);
>> + if (rc)
>> + goto err_req;
>> +
>> + memcpy(&tsc_resp, buf, sizeof(tsc_resp));
>> + pr_debug("%s: response status %x scale %llx offset %llx factor %x\n",
>> + __func__, tsc_resp.status, tsc_resp.tsc_scale, tsc_resp.tsc_offset,
>> + tsc_resp.tsc_factor);
>> +
>> + if (tsc_resp.status == 0) {
>> + snp_tsc_scale = tsc_resp.tsc_scale;
>> + snp_tsc_offset = tsc_resp.tsc_offset;
>> + } else {
>> + pr_err("Failed to get TSC info, response status %x\n", tsc_resp.status);
>> + rc = -EIO;
>> + }
>> +
>> +err_req:
>> + /* The response buffer contains the sensitive data, explicitly clear it. */
>> + memzero_explicit(buf, sizeof(buf));
>> + memzero_explicit(&tsc_resp, sizeof(tsc_resp));
>> + memzero_explicit(&req, sizeof(req));
>
> req does not seem to hold sensitive data.
That is correct, I will remove that.
> Is it needed, or maybe should it be tsc_req?
No, and tsc_req is zeroed by the caller and is not updated by AMD
security processor.
Regards
Nikunj
Powered by blists - more mailing lists