Hello Jim Mattson,
The patch f1b9dd5eb86c: "kvm: Disallow wraparound in
kvm_gfn_to_hva_cache_init" from Dec 17, 2018, leads to the following
static checker warning:
arch/x86/kvm/../../../virt/kvm/kvm_main.c:2024 __kvm_gfn_to_hva_cache_init()
error: uninitialized symbol 'nr_pages_avail'.
arch/x86/kvm/../../../virt/kvm/kvm_main.c
1998 static int __kvm_gfn_to_hva_cache_init(struct kvm_memslots *slots,
1999 struct gfn_to_hva_cache *ghc,
2000 gpa_t gpa, unsigned long len)
2001 {
2002 int offset = offset_in_page(gpa);
2003 gfn_t start_gfn = gpa >> PAGE_SHIFT;
2004 gfn_t end_gfn = (gpa + len - 1) >> PAGE_SHIFT;
2005 gfn_t nr_pages_needed = end_gfn - start_gfn + 1;
2006 gfn_t nr_pages_avail;
2007 int r = start_gfn <= end_gfn ? 0 : -EINVAL;
2008
2009 ghc->gpa = gpa;
2010 ghc->generation = slots->generation;
2011 ghc->len = len;
2012 ghc->hva = KVM_HVA_ERR_BAD;
2013
2014 /*
2015 * If the requested region crosses two memslots, we still
2016 * verify that the entire region is valid here.
2017 */
2018 while (!r && start_gfn <= end_gfn) {
2019 ghc->memslot = __gfn_to_memslot(slots, start_gfn);
2020 ghc->hva = gfn_to_hva_many(ghc->memslot, start_gfn,
2021 &nr_pages_avail);
2022 if (kvm_is_error_hva(ghc->hva))
2023 r = -EFAULT;
--> 2024 start_gfn += nr_pages_avail;
Setting start_gfn to garbage is harmless, but adding a break statement
would make the static checker happy.
2025 }
2026
2027 /* Use the slow path for cross page reads and writes. */
2028 if (!r && nr_pages_needed == 1)
2029 ghc->hva += offset;
2030 else
2031 ghc->memslot = NULL;
This chunk is really confusing. It would be more clear to write it
like this:
if (r) {
ghc->memslot = NULL;
return r;
}
if (end_gfn == start_gfn) {
ghc->hva += offset;
return 0;
}
ghc->memslot = NULL;
return 0;
But even though the code is now clear, I'm still a bit confused why
we're setting ->memslot to NULL on the success path.
2032
2033 return r;
2034 }
regards,
dan carpenter
