[bug report] KVM: arm64: Always invalidate TLB for stage-2 permission faults

[bug report] KVM: arm64: Always invalidate TLB for stage-2 permission faults

[Dan Carpenter]

Hello Oliver Upton,

The patch be097997a273: "KVM: arm64: Always invalidate TLB for
stage-2 permission faults" from Sep 22, 2023 (linux-next), leads to
the following Smatch static checker warning:

	arch/arm64/kvm/hyp/nvhe/../pgtable.c:1342 kvm_pgtable_stage2_relax_perms()
	error: uninitialized symbol 'level'.

arch/arm64/kvm/hyp/nvhe/../pgtable.c
    1319 int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
    1320                                    enum kvm_pgtable_prot prot)
    1321 {
    1322         int ret;
    1323         s8 level;
    1324         kvm_pte_t set = 0, clr = 0;
    1325 
    1326         if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
    1327                 return -EINVAL;
    1328 
    1329         if (prot & KVM_PGTABLE_PROT_R)
    1330                 set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
    1331 
    1332         if (prot & KVM_PGTABLE_PROT_W)
    1333                 set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
    1334 
    1335         if (prot & KVM_PGTABLE_PROT_X)
    1336                 clr |= KVM_PTE_LEAF_ATTR_HI_S2_XN;
    1337 
    1338         ret = stage2_update_leaf_attrs(pgt, addr, 1, set, clr, NULL, &level,
    1339                                        KVM_PGTABLE_WALK_HANDLE_FAULT |
    1340                                        KVM_PGTABLE_WALK_SHARED);
    1341         if (!ret || ret == -EAGAIN)
--> 1342                 kvm_call_hyp(__kvm_tlb_flush_vmid_ipa_nsh, pgt->mmu, addr, level);
                                                                                    ^^^^^
level is uninitialized if ret is -EAGAIN

    1343         return ret;
    1344 }

regards,
dan carpenter

Re: [bug report] KVM: arm64: Always invalidate TLB for stage-2 permission faults

[Oliver Upton]

On Thu, Feb 01, 2024 at 01:17:15PM +0300, Dan Carpenter wrote:
> Hello Oliver Upton,
> 
> The patch be097997a273: "KVM: arm64: Always invalidate TLB for
> stage-2 permission faults" from Sep 22, 2023 (linux-next), leads to
> the following Smatch static checker warning:
> 
> 	arch/arm64/kvm/hyp/nvhe/../pgtable.c:1342 kvm_pgtable_stage2_relax_perms()
> 	error: uninitialized symbol 'level'.
> 
> arch/arm64/kvm/hyp/nvhe/../pgtable.c
>     1319 int kvm_pgtable_stage2_relax_perms(struct kvm_pgtable *pgt, u64 addr,
>     1320                                    enum kvm_pgtable_prot prot)
>     1321 {
>     1322         int ret;
>     1323         s8 level;
>     1324         kvm_pte_t set = 0, clr = 0;
>     1325 
>     1326         if (prot & KVM_PTE_LEAF_ATTR_HI_SW)
>     1327                 return -EINVAL;
>     1328 
>     1329         if (prot & KVM_PGTABLE_PROT_R)
>     1330                 set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_R;
>     1331 
>     1332         if (prot & KVM_PGTABLE_PROT_W)
>     1333                 set |= KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W;
>     1334 
>     1335         if (prot & KVM_PGTABLE_PROT_X)
>     1336                 clr |= KVM_PTE_LEAF_ATTR_HI_S2_XN;
>     1337 
>     1338         ret = stage2_update_leaf_attrs(pgt, addr, 1, set, clr, NULL, &level,
>     1339                                        KVM_PGTABLE_WALK_HANDLE_FAULT |
>     1340                                        KVM_PGTABLE_WALK_SHARED);
>     1341         if (!ret || ret == -EAGAIN)
> --> 1342                 kvm_call_hyp(__kvm_tlb_flush_vmid_ipa_nsh, pgt->mmu, addr, level);
>                                                                                     ^^^^^
> level is uninitialized if ret is -EAGAIN

Hmm, race detection can happen at any level in the walk and that may not
be a leaf level. We can initialize level to -1 which would provide no
TTL hint for this case.

-- 
Thanks,
Oliver