Re: [bug report] KVM: Dynamically allocate "new" memslots from the get-go

[Sean Christopherson <seanjc@google.com>]

On Wed, Dec 15, 2021, Dan Carpenter wrote:
> Hello Sean Christopherson,
> 
> This is a semi-automatic email about new static checker warnings.

These are all ok, KVM is being clever and using implicit checks on pointers in
select flows, while using explicit checks in others.

> The patch 244893fa2859: "KVM: Dynamically allocate "new" memslots
> from the get-go" from Dec 6, 2021, leads to the following Smatch
> complaint:
> 
>     arch/x86/kvm/../../../virt/kvm/kvm_main.c:1526 kvm_prepare_memory_region()
>     warn: variable dereferenced before check 'new' (see line 1509)

@new is guaranteed to be non-NULL in the !KVM_MR_DELETE case.
 
> arch/x86/kvm/../../../virt/kvm/kvm_main.c
>   1508		if (change != KVM_MR_DELETE) {
>   1509			if (!(new->flags & KVM_MEM_LOG_DIRTY_PAGES))
>   1510				new->dirty_bitmap = NULL;
>   1511			else if (old && old->dirty_bitmap)
>   1512				new->dirty_bitmap = old->dirty_bitmap;
>   1513			else if (!kvm->dirty_ring_size) {
>   1514				r = kvm_alloc_dirty_bitmap(new);
>   1515				if (r)
>   1516					return r;
>   1517	
>   1518				if (kvm_dirty_log_manual_protect_and_init_set(kvm))
>   1519					bitmap_set(new->dirty_bitmap, 0, new->npages);
>                                                    ^^^^^
>   1520			}
>   1521		}
>   1522	
>   1523		r = kvm_arch_prepare_memory_region(kvm, old, new, change);
>                                                              ^^^
> Lots of unchecked dereferences

There's no true dereference, architectures are responsible for ensuring @old and
@new are non-NULL, either via explicit checks on the pointer or implicit checks
on @change.

>   1524	
>   1525		/* Free the bitmap on failure if it was allocated above. */
>   1526		if (r && new && new->dirty_bitmap && old && !old->dirty_bitmap)
>                          ^^^
> New check for NULL.  Can this be NULL?

Yes, when change == KVM_MR_DELETE.

>   1527			kvm_destroy_dirty_bitmap(new);
>   1528	
> 
> regards,
> dan carpenter