Re: [RFC PATCH v2 18/22] coco/sev-guest: Implement the guest support for SEV TIO

[Francesco Lavra <francescolavra.fl@gmail.com>]

On 2025-02-18 at 11:10, Alexey Kardashevskiy wrote:
> 
> +static int handle_tio_guest_request(struct snp_guest_dev *snp_dev,
> u8 type,
> +				   void *req_buf, size_t req_sz,
> void *resp_buf, u32 resp_sz,
> +				   void *pt, u64 *npages, u64 *bdfn,
> u64 *param, u64 *fw_err)
> +{
> +	struct snp_msg_desc *mdesc = snp_dev->msg_desc;
> +	struct snp_guest_req req = {
> +		.msg_version = TIO_MESSAGE_VERSION,
> +	};
> +	u64 exitinfo2 = 0;
> +	int ret;
> +
> +	req.msg_type = type;
> +	req.vmpck_id = mdesc->vmpck_id;
> +	req.req_buf = req_buf;
> +	req.req_sz = req_sz;
> +	req.resp_buf = resp_buf;
> +	req.resp_sz = resp_sz;
> +	req.exit_code = SVM_VMGEXIT_SEV_TIO_GUEST_REQUEST;
> +
> +	req.input.guest_rid = 0;
> +	req.input.param = 0;
> +
> +	if (pt && npages) {
> +		req.data = pt;
> +		req.input.data_npages = *npages;
> +	}
> +	if (bdfn)
> +		req.input.guest_rid = *bdfn;
> +	if (param)
> +		req.input.param = *param;
> +
> +	ret = snp_send_guest_request(mdesc, &req, &exitinfo2);
> +
> +	if (param)
> +		*param = req.input.param;
> +
> +	*fw_err = exitinfo2;
> +
> +	return ret;

The logic to update *npages is missing.

> 
> +}
> +
> +static int guest_request_tio_data(struct snp_guest_dev *snp_dev, u8
> type,
> +				   void *req_buf, size_t req_sz,
> void *resp_buf, u32 resp_sz,
> +				   u64 bdfn, enum tsm_tdisp_state
> *state,
> +				   struct tsm_blob **certs, struct
> tsm_blob **meas,
> +				   struct tsm_blob **report, u64
> *fw_err)
> +{
> +	u64 npages = SZ_32K >> PAGE_SHIFT, c1, param = 0;
> +	struct tio_blob_table_entry *pt;
> +	int rc;
> +
> +	pt = snp_alloc_shared_pages(npages << PAGE_SHIFT);
> +	if (!pt)
> +		return -ENOMEM;
> +
> +	c1 = npages;
> +	rc = handle_tio_guest_request(snp_dev, type, req_buf,
> req_sz, resp_buf, resp_sz,
> +				      pt, &c1, &bdfn, state ? &param
> : NULL, fw_err);
> +
> +	if (c1 > SZ_32K) {

c1 is supposed to be a number of pages, not a number of bytes.

> +static int tio_tdi_status(struct tsm_tdi *tdi, struct snp_guest_dev
> *snp_dev,
> +			  struct tsm_tdi_status *ts)
> +{
> +	struct snp_msg_desc *mdesc = snp_dev->msg_desc;
> +	size_t resp_len = sizeof(struct tio_msg_tdi_info_rsp) +
> mdesc->ctx->authsize;
> +	struct tio_msg_tdi_info_rsp *rsp = kzalloc(resp_len,
> GFP_KERNEL);
> +	struct tio_msg_tdi_info_req req = {
> +		.guest_device_id = pci_dev_id(tdi_to_pci_dev(tdi)),
> +	};
> +	u64 fw_err = 0;
> +	int rc;
> +	enum tsm_tdisp_state state = 0;
> +
> +	dev_notice(&tdi->dev, "TDI info");
> +	if (!rsp)
> +		return -ENOMEM;
> +
> +	rc = guest_request_tio_data(snp_dev, TIO_MSG_TDI_INFO_REQ,
> &req,
> +				     sizeof(req), rsp, resp_len,
> +				    
> pci_dev_id(tdi_to_pci_dev(tdi)), &state,
> +				     &tdi->tdev->certs, &tdi->tdev-
> >meas,
> +				     &tdi->report, &fw_err);
> +	if (rc)
> +		goto free_exit;
> +
> +	ts->meas_digest_valid = rsp->meas_digest_valid;
> +	ts->meas_digest_fresh = rsp->meas_digest_fresh;
> +	ts->no_fw_update = rsp->no_fw_update;
> +	ts->cache_line_size = rsp->cache_line_size == 0 ? 64 : 128;
> +	ts->lock_msix = rsp->lock_msix;
> +	ts->bind_p2p = rsp->bind_p2p;
> +	ts->all_request_redirect = rsp->all_request_redirect;
> +#define __ALGO(x, n, y) \
> +	((((x) & (0xFFUL << (n))) == TIO_SPDM_ALGOS_##y) ? \
> +	 (1ULL << TSM_SPDM_ALGOS_##y) : 0)
> +	ts->spdm_algos =
> +		__ALGO(rsp->spdm_algos, 0, DHE_SECP256R1) |
> +		__ALGO(rsp->spdm_algos, 0, DHE_SECP384R1) |
> +		__ALGO(rsp->spdm_algos, 8, AEAD_AES_128_GCM) |
> +		__ALGO(rsp->spdm_algos, 8, AEAD_AES_256_GCM) |
> +		__ALGO(rsp->spdm_algos, 16,
> ASYM_TPM_ALG_RSASSA_3072) |
> +		__ALGO(rsp->spdm_algos, 16,
> ASYM_TPM_ALG_ECDSA_ECC_NIST_P256) |
> +		__ALGO(rsp->spdm_algos, 16,
> ASYM_TPM_ALG_ECDSA_ECC_NIST_P384) |
> +		__ALGO(rsp->spdm_algos, 24, HASH_TPM_ALG_SHA_256) |
> +		__ALGO(rsp->spdm_algos, 24, HASH_TPM_ALG_SHA_384) |
> +		__ALGO(rsp->spdm_algos, 32,
> KEY_SCHED_SPDM_KEY_SCHEDULE);
> +#undef __ALGO
> +	memcpy(ts->certs_digest, rsp->certs_digest, sizeof(ts-
> >certs_digest));
> +	memcpy(ts->meas_digest, rsp->meas_digest, sizeof(ts-
> >meas_digest));
> +	memcpy(ts->interface_report_digest, rsp-
> >interface_report_digest,
> +	       sizeof(ts->interface_report_digest));
> +	ts->intf_report_counter = rsp->tdi_report_count;
> +
> +	ts->valid = true;
> +	ts->state = state;
> +	/* The response buffer contains the sensitive data,
> explicitly clear it. */
> +free_exit:
> +	memzero_explicit(&rsp, sizeof(resp_len));

The first argument should be rsp, not &rsp. This issue is also present
in the other memzero_explicit() calls in this patch.

> +static int sev_guest_tdi_validate(struct tsm_tdi *tdi, unsigned int
> featuremask,
> +				  bool invalidate, void
> *private_data)
> +{
> +	struct snp_guest_dev *snp_dev = private_data;
> +	struct tsm_tdi_status ts = { 0 };
> +	int ret;
> +
> +	if (!tdi->report) {
> +		ret = tio_tdi_status(tdi, snp_dev, &ts);
> +
> +		if (ret || !tdi->report) {
> +			dev_err(&tdi->dev, "No report available,
> ret=%d", ret);
> +			if (!ret && tdi->report)

This cannot happen, I think you meant (!ret && !tdi->report)
>