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
