linux-kernel - Re: [PATCH v2 08/11] x86/sev: Add Secure TSC support for SNP guests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Apr 2023 11:14:47 -0600
From:   Peter Gonda <pgonda@...gle.com>
To:     Nikunj A Dadhania <nikunj@....com>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org, bp@...en8.de,
        thomas.lendacky@....com, dionnaglaze@...gle.com, seanjc@...gle.com,
        pbonzini@...hat.com, michael.roth@....com, ketanch@...k.ac.in
Subject: Re: [PATCH v2 08/11] x86/sev: Add Secure TSC support for SNP guests

> +
>  /* #VC handler runtime per-CPU data */
>  struct sev_es_runtime_data {
>         struct ghcb ghcb_page;
> @@ -1107,7 +1111,7 @@ static void *alloc_shared_pages(size_t sz)
>         return page_address(page);
>  }
>
> -static int snp_setup_psp_messaging(struct sev_guest_platform_data *pdata)
> +static int __init snp_setup_psp_messaging(struct sev_guest_platform_data *pdata)
>  {
>         u64 gpa;
>         int ret;
> @@ -1406,6 +1410,80 @@ bool snp_assign_vmpck(struct snp_guest_dev *dev, int vmpck_id)
>  }
>  EXPORT_SYMBOL_GPL(snp_assign_vmpck);
>
> +static int __init snp_get_tsc_info(void)
> +{
> +       u8 buf[SNP_TSC_INFO_REQ_SZ + AUTHTAG_LEN];
> +       struct snp_tsc_info_resp tsc_resp = {0};
> +       struct snp_tsc_info_req tsc_req;
> +       struct snp_guest_req req;
> +       struct snp_guest_dev dev;
> +       int rc, resp_len;
> +
> +       /*
> +        * The intermediate response buffer is used while decrypting the
> +        * response payload. Make sure that it has enough space to cover the
> +        * authtag.
> +        */
> +       resp_len = sizeof(tsc_resp) + AUTHTAG_LEN;
> +       if (sizeof(buf) < resp_len)
> +               return -EINVAL;
> +
> +       /* Zero the tsc_info_req */
> +       memzero_explicit(&tsc_req, sizeof(tsc_req));
> +       memzero_explicit(&req, sizeof(req));

Whats the guidance on when we should use memzero_explicit() vs just
something like: `snp_tsc_info_resp tsc_resp = {0};`?

> +
> +       dev.pdata = platform_data;
> +       if (!snp_assign_vmpck(&dev, 0))
> +               return -EINVAL;
> +
> +       req.msg_version = MSG_HDR_VER;
> +       req.msg_type = SNP_MSG_TSC_INFO_REQ;
> +       req.req_buf = &tsc_req;
> +       req.req_sz = sizeof(tsc_req);
> +       req.resp_buf = buf;
> +       req.resp_sz = resp_len;
> +       req.fw_err = NULL;

Why do we not want the FW error code?

> +       req.exit_code = SVM_VMGEXIT_GUEST_REQUEST;
> +       rc = snp_send_guest_request(&dev, &req);
> +       if (rc)
> +               goto err_req;
> +
> +       memcpy(&tsc_resp, buf, sizeof(tsc_resp));
> +       pr_debug("%s: Valid response status %x scale %llx offset %llx factor %llx\n",
> +                __func__, tsc_resp.status, tsc_resp.tsc_scale, tsc_resp.tsc_offset,
> +                tsc_resp.tsc_factor);
> +
> +       guest_tsc_scale = tsc_resp.tsc_scale;
> +       guest_tsc_offset = tsc_resp.tsc_offset;
> +
> +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));
> +
> +       return rc;
> +}
> +