Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The function misuses the shared per-CPU `hyperv_pcpu_input_arg` for an asynchronous hypercall, potentially corrupting the hypercall input. - [Medium] Un-ratelimited `pr_err` on the guest-triggerable hypercall failure path allows a malicious guest to flood the host kernel log. --
--- Patch [2]: [PATCH v3 2/7] mshv: add SEV-SNP PSP request hypercall --- commit 8b1f140f3a0780e521d7a96627211fe9e4967940 Author: Wei Liu <[email protected]> mshv: add SEV-SNP PSP request hypercall This commit adds a helper for HVCALL_ISSUE_SNP_PSP_GUEST_REQUEST so MSHV can issue asynchronous PSP guest requests on behalf of encrypted partitions. > diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c > index cb55d4d4be2e5..48abe338c06f8 100644 > --- a/drivers/hv/mshv_root_hv_call.c > +++ b/drivers/hv/mshv_root_hv_call.c > @@ -1009,6 +1009,45 @@ int hv_unmap_stats_page(enum hv_stats_object_type type, > return ret; > } > > +#ifdef HV_SUPPORTS_SEV_SNP_GUESTS > +int hv_call_issue_psp_guest_request(u64 partition_id, u64 req_pfn, > + u64 rsp_pfn, > + void (*completion_handler)(void *data, > + u64 *status), > + void *completion_data) > +{ > + struct hv_input_issue_psp_guest_request *input; > + unsigned long flags; > + u64 status; > + > + if (!completion_handler) { > + pr_err("%s: missing completion handler\n", __func__); > + return -EINVAL; > + } > + > + local_irq_save(flags); > + input = *this_cpu_ptr(hyperv_pcpu_input_arg); > + memset(input, 0, sizeof(*input)); > + input->partition_id = partition_id; > + input->request_page = req_pfn; > + input->response_page = rsp_pfn; > + status = hv_do_hypercall(HVCALL_ISSUE_SNP_PSP_GUEST_REQUEST, input, > + NULL); > + local_irq_restore(flags); > + > + if (hv_result(status) == HV_STATUS_CALL_PENDING) > + completion_handler(completion_data, &status); [Severity: High] Does this code risk corrupting the hypercall input data? The function uses the shared per-CPU hyperv_pcpu_input_arg for the hypercall. However, it restores interrupts and invokes completion_handler() which blocks while the hypercall is pending. During this time, another thread scheduled on the same CPU can issue a hypercall and overwrite the shared per-CPU page while the hypervisor is still reading the original parameters asynchronously. > + > + if (!hv_result_success(status)) { > + pr_err("%s: status=%s partition_id=%llu\n", __func__, > + hv_result_to_string(status), partition_id); [Severity: Medium] Could this unconditional error print be used to flood the host kernel log? If a malicious guest VM repeatedly issues invalid SEV-SNP PSP requests, the hypercall will fail and trigger this error print on the failure path. Since this can be triggered at will by the guest, it might be safer to rate-limit this message. > + return hv_result_to_errno(status); > + } > + > + return 0; > +} > +#endif -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
