On Thu, Dec 08, 2022, Vitaly Kuznetsov wrote:
> In kvm_hv_flush_tlb(), 'data_offset' and 'consumed_xmm_halves' variables
> are used in a mutually exclusive way: in 'hc->fast' we count in 'XMM
> halves' and increase 'data_offset' otherwise. Coverity discovered, that in
> one case both variables are incremented unconditionally. This doesn't seem
> to cause any issues as the only user of 'data_offset'/'consumed_xmm_halves'
> data is kvm_hv_get_tlb_flush_entries() -> kvm_hv_get_hc_data() which also
> takes into account 'hc->fast' but is still worth fixing.
>
> To make things explicit, put 'data_offset' and 'consumed_xmm_halves' to
> 'struct kvm_hv_hcall' as a union and use at call sites. This allows to
> remove explicit 'data_offset'/'consumed_xmm_halves' parameters from
> kvm_hv_get_hc_data()/kvm_get_sparse_vp_set()/kvm_hv_get_tlb_flush_entries()
> helpers.
>
> Note: 'struct kvm_hv_hcall' is allocated on stack in kvm_hv_hypercall() and
> is not zeroed, consumers are supposed to initialize the appropriate field
> if needed.
>
> Reported-by: coverity-bot <keescook+coverity-bot@xxxxxxxxxxxx>
> Addresses-Coverity-ID: 1527764 ("Uninitialized variables")
> Fixes: 260970862c88 ("KVM: x86: hyper-v: Handle HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST{,EX} calls gently")
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
Reviewed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> Changes since v1:
> - Shove 'data_offset'/'consumed_xmm_halves' into a union in 'struct
> kvm_hv_hcall' to make things more explicit. [Sean]
> ---
> arch/x86/kvm/hyperv.c | 63 ++++++++++++++++++++++++-------------------
> 1 file changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 2c7f2a26421e..e8296942a868 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -1769,6 +1769,7 @@ static bool hv_is_vp_in_sparse_set(u32 vp_id, u64 valid_bank_mask, u64 sparse_ba
> }
>
> struct kvm_hv_hcall {
> + /* Hypercall input data */
> u64 param;
> u64 ingpa;
> u64 outgpa;
> @@ -1779,12 +1780,21 @@ struct kvm_hv_hcall {
> bool fast;
> bool rep;
> sse128_t xmm[HV_HYPERCALL_MAX_XMM_REGISTERS];
Uber nit, might want an
/* End hypercall input data */
or so to cleary demarcate the input vs. the scratch area.
