Hey Sean,
On Tue, Feb 18, 2020 at 11:07:29AM -0800, Sean Christopherson wrote:
> On Tue, Feb 18, 2020 at 10:47:56AM -0800, Oliver Upton wrote:
> > Particularly draconian compilers warn of a possible uninitialized use of
> > the nr_pages_avail variable. Silence this warning by initializing it to
> > zero.
>
> Can you check if the warning still exists with commit 6ad1e29fe0ab ("KVM:
> Clean up __kvm_gfn_to_hva_cache_init() and its callers")? I'm guessing
> (hoping?) the suppression is no longer necessary.
Hmm. I rebased this patch right before sending out + it seems that it is
required (at least for me) to silence the compiler warning. For good
measure, I ran git branch --contains to ensure I had your change. Looks
like my topic branch did in fact have your fix.
--
Oliver
> commit 6ad1e29fe0aba843dfffc714fced0ef6a2e19502
> Author: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Date: Thu Jan 9 14:58:55 2020 -0500
>
> KVM: Clean up __kvm_gfn_to_hva_cache_init() and its callers
>
> Barret reported a (technically benign) bug where nr_pages_avail can be
> accessed without being initialized if gfn_to_hva_many() fails.
>
> virt/kvm/kvm_main.c:2193:13: warning: 'nr_pages_avail' may be
> used uninitialized in this function [-Wmaybe-uninitialized]
>
> Rather than simply squashing the warning by initializing nr_pages_avail,
> fix the underlying issues by reworking __kvm_gfn_to_hva_cache_init() to
> return immediately instead of continuing on. Now that all callers check
> the result and/or bail immediately on a bad hva, there's no need to
> explicitly nullify the memslot on error.
>
> Reported-by: Barret Rhoden <brho@xxxxxxxxxx>
> Fixes: f1b9dd5eb86c ("kvm: Disallow wraparound in kvm_gfn_to_hva_cache_init")
> Cc: Jim Mattson <jmattson@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>
>
> > Signed-off-by: Oliver Upton <oupton@xxxxxxxxxx>
> > ---
> > virt/kvm/kvm_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 70f03ce0e5c1..dc8a67ad082d 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2219,7 +2219,7 @@ static int __kvm_gfn_to_hva_cache_init(struct kvm_memslots *slots,
> > gfn_t start_gfn = gpa >> PAGE_SHIFT;
> > gfn_t end_gfn = (gpa + len - 1) >> PAGE_SHIFT;
> > gfn_t nr_pages_needed = end_gfn - start_gfn + 1;
> > - gfn_t nr_pages_avail;
> > + gfn_t nr_pages_avail = 0;
> >
> > /* Update ghc->generation before performing any error checks. */
> > ghc->generation = slots->generation;
> > --
> > 2.25.0.265.gbab2e86ba0-goog
> >
