[PATCH] kvm: fix to update memslots properly

[PATCH] kvm: fix to update memslots properly

[Tiejun Chen]

After commit, 0e60b0799fed, "kvm: change memslot sorting rule from size to
GFN" is introduced, we're missing but need to consider such a case,
(!new->base_gfn && !mslots[i - 1].base_gfn && !mslots[i - 1].npages), then
re-sort kvm_memslots wrong in next case to issue the following,

KVM internal error. Suberror: 1
emulation failure
EAX=000dee58 EBX=00000000 ECX=00000000 EDX=00000cfd
ESI=00000059 EDI=00000000 EBP=00000000 ESP=00006fc4
EIP=000f17f4 EFL=00010012 [----A--] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0010 00000000 ffffffff 00c09300 DPL=0 DS   [-WA]
CS =0008 00000000 ffffffff 00c09b00 DPL=0 CS32 [-RA]
SS =0010 00000000 ffffffff 00c09300 DPL=0 DS   [-WA]
DS =0010 00000000 ffffffff 00c09300 DPL=0 DS   [-WA]
FS =0010 00000000 ffffffff 00c09300 DPL=0 DS   [-WA]
GS =0010 00000000 ffffffff 00c09300 DPL=0 DS   [-WA]
LDT=0000 00000000 0000ffff 00008200 DPL=0 LDT
TR =0000 00000000 0000ffff 00008b00 DPL=0 TSS32-busy
GDT=     000f6c58 00000037
IDT=     000f6c96 00000000
CR0=60000011 CR2=00000000 CR3=00000000 CR4=00000000
DR0=0000000000000000 DR1=0000000000000000 DR2=0000000000000000
DR3=0000000000000000
DR6=00000000ffff0ff0 DR7=0000000000000400
EFER=0000000000000000
Code=e8 75 fc ff ff 89 f2 a8 10 89 d8 75 0a b9 74 17 ff ff ff d1 <5b>
5e c3 5b 5e e9 76 ff ff ff 57 56 53 8b 35 38 65 0f 00 85 f6 0f 88 be
00 00 00 0f b7 f6

And we also should set flag as 0 in case of (new->npages == 0) &&
(new->base_gfn == 0).

Reported-by: Jamie Heilman <jamie@audible.transient.net>
Tested-by: Jamie Heilman <jamie@audible.transient.net>
Reported-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---

I test this both in Andy' case and Jamie's case.

 virt/kvm/kvm_main.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f528343..6e52f3f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -672,6 +672,7 @@ static void update_memslots(struct kvm_memslots *slots,
 	WARN_ON(mslots[i].id != id);
 	if (!new->npages) {
 		new->base_gfn = 0;
+		new->flags = 0;
 		if (mslots[i].npages)
 			slots->used_slots--;
 	} else {
@@ -688,7 +689,9 @@ static void update_memslots(struct kvm_memslots *slots,
 		i++;
 	}
 	while (i > 0 &&
-	       new->base_gfn > mslots[i - 1].base_gfn) {
+	       ((new->base_gfn > mslots[i - 1].base_gfn) ||
+	        (!new->base_gfn &&
+	         !mslots[i - 1].base_gfn && !mslots[i - 1].npages))) {
 		mslots[i] = mslots[i - 1];
 		slots->id_to_index[mslots[i].id] = i;
 		i--;
-- 
1.9.1

Re: [PATCH] kvm: fix to update memslots properly

[Paolo Bonzini]

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f528343..6e52f3f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -672,6 +672,7 @@ static void update_memslots(struct kvm_memslots *slots,
>  	WARN_ON(mslots[i].id != id);
>  	if (!new->npages) {
>  		new->base_gfn = 0;
> +		new->flags = 0;
>  		if (mslots[i].npages)
>  			slots->used_slots--;
>  	} else {

This should not be necessary.  The part of the mslots array that has 
base_gfn == npages == 0 is entirely unused, and such a slot can never 
be returned by search_memslots because this:

        if (gfn >= memslots[slot].base_gfn &&
            gfn < memslots[slot].base_gfn + memslots[slot].npages)

can never be true.

> @@ -688,7 +689,9 @@ static void update_memslots(struct kvm_memslots *slots,
>  		i++;
>  	}
>  	while (i > 0 &&
> -	       new->base_gfn > mslots[i - 1].base_gfn) {
> +	       ((new->base_gfn > mslots[i - 1].base_gfn) ||
> +	        (!new->base_gfn &&
> +	         !mslots[i - 1].base_gfn && !mslots[i - 1].npages))) {
>  		mslots[i] = mslots[i - 1];
>  		slots->id_to_index[mslots[i].id] = i;
>  		i--;
> 

You should have explained _why_ this fixes the bug, and what invariant 
is not being respected, something like this:

    kvm: fix sorting of memslots with base_gfn == 0
    
    Before commit 0e60b0799fed (kvm: change memslot sorting rule from size
    to GFN, 2014-12-01), the memslots' sorting key was npages, meaning
    that a valid memslot couldn't have its sorting key equal to zero.
    On the other hand, a valid memslot can have base_gfn == 0, and invalid
    memslots are identified by base_gfn == npages == 0.
    
    Because of this, commit 0e60b0799fed broke the invariant that invalid
    memslots are at the end of the mslots array.  When a memslot with
    base_gfn == 0 was created, any invalid memslot before it were left
    in place.
    
This suggests another fix.  We can change the insertion to use a ">="
comparison, as in your first patch.  Alone it is not correct, but we
only need to take some care and avoid breaking the case of deleting a
memslot.

It's enough to wrap the second loop (that you patched) with
"if (new->npages)".  In the new->npages == 0 case the first loop has
already set i to the right value, and moving i back would be wrong:

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f5283438ee05..050974c051b5 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -687,11 +687,23 @@ static void update_memslots(struct kvm_memslots *slots,
 		slots->id_to_index[mslots[i].id] = i;
 		i++;
 	}
-	while (i > 0 &&
-	       new->base_gfn > mslots[i - 1].base_gfn) {
-		mslots[i] = mslots[i - 1];
-		slots->id_to_index[mslots[i].id] = i;
-		i--;
+
+	/*
+	 * The ">=" is needed when creating a slot with base_gfn == 0,
+	 * so that it moves before all those with base_gfn == npages == 0.
+	 *
+	 * On the other hand, if new->npages is zero, the above loop has
+	 * already left i pointing to the beginning of the empty part of
+	 * mslots, and the ">=" would move the hole backwards in this
+	 * case---which is wrong.  So skip the loop when deleting a slot.
+	 */
+	if (new->npages) {
+		while (i > 0 &&
+		       new->base_gfn >= mslots[i - 1].base_gfn) {
+			mslots[i] = mslots[i - 1];
+			slots->id_to_index[mslots[i].id] = i;
+			i--;
+		}
 	}
 
 	mslots[i] = *new;

Paolo

Re: [PATCH] kvm: fix to update memslots properly

[Jamie Heilman]

Paolo Bonzini wrote:
> This suggests another fix.  We can change the insertion to use a ">="
> comparison, as in your first patch.  Alone it is not correct, but we
> only need to take some care and avoid breaking the case of deleting a
> memslot.
> 
> It's enough to wrap the second loop (that you patched) with
> "if (new->npages)".  In the new->npages == 0 case the first loop has
> already set i to the right value, and moving i back would be wrong:
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f5283438ee05..050974c051b5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -687,11 +687,23 @@ static void update_memslots(struct kvm_memslots *slots,
>  		slots->id_to_index[mslots[i].id] = i;
>  		i++;
>  	}
> -	while (i > 0 &&
> -	       new->base_gfn > mslots[i - 1].base_gfn) {
> -		mslots[i] = mslots[i - 1];
> -		slots->id_to_index[mslots[i].id] = i;
> -		i--;
> +
> +	/*
> +	 * The ">=" is needed when creating a slot with base_gfn == 0,
> +	 * so that it moves before all those with base_gfn == npages == 0.
> +	 *
> +	 * On the other hand, if new->npages is zero, the above loop has
> +	 * already left i pointing to the beginning of the empty part of
> +	 * mslots, and the ">=" would move the hole backwards in this
> +	 * case---which is wrong.  So skip the loop when deleting a slot.
> +	 */
> +	if (new->npages) {
> +		while (i > 0 &&
> +		       new->base_gfn >= mslots[i - 1].base_gfn) {
> +			mslots[i] = mslots[i - 1];
> +			slots->id_to_index[mslots[i].id] = i;
> +			i--;
> +		}
>  	}
>  
>  	mslots[i] = *new;

I gave this a try, and it works just fine for me too.

-- 
Jamie Heilman                     http://audible.transient.net/~jamie/

Re: [PATCH] kvm: fix to update memslots properly

[Chen, Tiejun]

On 2014/12/28 4:41, Paolo Bonzini wrote:
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index f528343..6e52f3f 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -672,6 +672,7 @@ static void update_memslots(struct kvm_memslots *slots,
>>   	WARN_ON(mslots[i].id != id);
>>   	if (!new->npages) {
>>   		new->base_gfn = 0;
>> +		new->flags = 0;
>>   		if (mslots[i].npages)
>>   			slots->used_slots--;
>>   	} else {
>
> This should not be necessary.  The part of the mslots array that has
> base_gfn == npages == 0 is entirely unused, and such a slot can never
> be returned by search_memslots because this:
>
>          if (gfn >= memslots[slot].base_gfn &&
>              gfn < memslots[slot].base_gfn + memslots[slot].npages)
>
> can never be true.

Yeah, but its really a little ugly to see some slots, 
base_gfn:npages:falgs = 0:0:(!0), to resort again when debug something 
inside of update_memslots().

>
>> @@ -688,7 +689,9 @@ static void update_memslots(struct kvm_memslots *slots,
>>   		i++;
>>   	}
>>   	while (i > 0 &&
>> -	       new->base_gfn > mslots[i - 1].base_gfn) {
>> +	       ((new->base_gfn > mslots[i - 1].base_gfn) ||
>> +	        (!new->base_gfn &&
>> +	         !mslots[i - 1].base_gfn && !mslots[i - 1].npages))) {
>>   		mslots[i] = mslots[i - 1];
>>   		slots->id_to_index[mslots[i].id] = i;
>>   		i--;
>>
>
> You should have explained _why_ this fixes the bug, and what invariant

Yeah.

> is not being respected, something like this:
>
>      kvm: fix sorting of memslots with base_gfn == 0
>
>      Before commit 0e60b0799fed (kvm: change memslot sorting rule from size
>      to GFN, 2014-12-01), the memslots' sorting key was npages, meaning
>      that a valid memslot couldn't have its sorting key equal to zero.
>      On the other hand, a valid memslot can have base_gfn == 0, and invalid
>      memslots are identified by base_gfn == npages == 0.
>
>      Because of this, commit 0e60b0799fed broke the invariant that invalid
>      memslots are at the end of the mslots array.  When a memslot with
>      base_gfn == 0 was created, any invalid memslot before it were left
>      in place.
>
> This suggests another fix.  We can change the insertion to use a ">="
> comparison, as in your first patch.  Alone it is not correct, but we
> only need to take some care and avoid breaking the case of deleting a
> memslot.
>
> It's enough to wrap the second loop (that you patched) with
> "if (new->npages)".  In the new->npages == 0 case the first loop has
> already set i to the right value, and moving i back would be wrong:
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f5283438ee05..050974c051b5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -687,11 +687,23 @@ static void update_memslots(struct kvm_memslots *slots,
>   		slots->id_to_index[mslots[i].id] = i;
>   		i++;
>   	}
> -	while (i > 0 &&
> -	       new->base_gfn > mslots[i - 1].base_gfn) {
> -		mslots[i] = mslots[i - 1];
> -		slots->id_to_index[mslots[i].id] = i;
> -		i--;
> +
> +	/*
> +	 * The ">=" is needed when creating a slot with base_gfn == 0,
> +	 * so that it moves before all those with base_gfn == npages == 0.
> +	 *
> +	 * On the other hand, if new->npages is zero, the above loop has
> +	 * already left i pointing to the beginning of the empty part of
> +	 * mslots, and the ">=" would move the hole backwards in this
> +	 * case---which is wrong.  So skip the loop when deleting a slot.
> +	 */
> +	if (new->npages) {
> +		while (i > 0 &&
> +		       new->base_gfn >= mslots[i - 1].base_gfn) {
> +			mslots[i] = mslots[i - 1];
> +			slots->id_to_index[mslots[i].id] = i;
> +			i--;
> +		}
>   	}
>
>   	mslots[i] = *new;
>

This looks better.

Tiejun

Re: [PATCH] kvm: fix to update memslots properly

[Marcelo Tosatti]

On Sat, Dec 27, 2014 at 09:41:45PM +0100, Paolo Bonzini wrote:
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index f528343..6e52f3f 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -672,6 +672,7 @@ static void update_memslots(struct kvm_memslots *slots,
> >  	WARN_ON(mslots[i].id != id);
> >  	if (!new->npages) {
> >  		new->base_gfn = 0;
> > +		new->flags = 0;
> >  		if (mslots[i].npages)
> >  			slots->used_slots--;
> >  	} else {
> 
> This should not be necessary.  The part of the mslots array that has 
> base_gfn == npages == 0 is entirely unused, and such a slot can never 
> be returned by search_memslots because this:
> 
>         if (gfn >= memslots[slot].base_gfn &&
>             gfn < memslots[slot].base_gfn + memslots[slot].npages)
> 
> can never be true.
> 
> > @@ -688,7 +689,9 @@ static void update_memslots(struct kvm_memslots *slots,
> >  		i++;
> >  	}
> >  	while (i > 0 &&
> > -	       new->base_gfn > mslots[i - 1].base_gfn) {
> > +	       ((new->base_gfn > mslots[i - 1].base_gfn) ||
> > +	        (!new->base_gfn &&
> > +	         !mslots[i - 1].base_gfn && !mslots[i - 1].npages))) {
> >  		mslots[i] = mslots[i - 1];
> >  		slots->id_to_index[mslots[i].id] = i;
> >  		i--;
> > 
> 
> You should have explained _why_ this fixes the bug, and what invariant 
> is not being respected, something like this:
> 
>     kvm: fix sorting of memslots with base_gfn == 0
>     
>     Before commit 0e60b0799fed (kvm: change memslot sorting rule from size
>     to GFN, 2014-12-01), the memslots' sorting key was npages, meaning
>     that a valid memslot couldn't have its sorting key equal to zero.
>     On the other hand, a valid memslot can have base_gfn == 0, and invalid
>     memslots are identified by base_gfn == npages == 0.
>     
>     Because of this, commit 0e60b0799fed broke the invariant that invalid
>     memslots are at the end of the mslots array.  When a memslot with
>     base_gfn == 0 was created, any invalid memslot before it were left
>     in place.
>     
> This suggests another fix.  We can change the insertion to use a ">="
> comparison, as in your first patch.  Alone it is not correct, but we
> only need to take some care and avoid breaking the case of deleting a
> memslot.
> 
> It's enough to wrap the second loop (that you patched) with
> "if (new->npages)".  In the new->npages == 0 case the first loop has
> already set i to the right value, and moving i back would be wrong:
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f5283438ee05..050974c051b5 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -687,11 +687,23 @@ static void update_memslots(struct kvm_memslots *slots,
>  		slots->id_to_index[mslots[i].id] = i;
>  		i++;
>  	}
> -	while (i > 0 &&
> -	       new->base_gfn > mslots[i - 1].base_gfn) {
> -		mslots[i] = mslots[i - 1];
> -		slots->id_to_index[mslots[i].id] = i;
> -		i--;
> +
> +	/*
> +	 * The ">=" is needed when creating a slot with base_gfn == 0,
> +	 * so that it moves before all those with base_gfn == npages == 0.
> +	 *
> +	 * On the other hand, if new->npages is zero, the above loop has
> +	 * already left i pointing to the beginning of the empty part of
> +	 * mslots, and the ">=" would move the hole backwards in this
> +	 * case---which is wrong.  So skip the loop when deleting a slot.
> +	 */
> +	if (new->npages) {
> +		while (i > 0 &&
> +		       new->base_gfn >= mslots[i - 1].base_gfn) {
> +			mslots[i] = mslots[i - 1];
> +			slots->id_to_index[mslots[i].id] = i;
> +			i--;
> +		}
>  	}
>  
>  	mslots[i] = *new;
> 
> Paolo

Paolo,

Can you include a proper changelog for this patch?

Re: [PATCH] kvm: fix to update memslots properly

[Chen, Tiejun]

On 2015/3/10 4:54, Marcelo Tosatti wrote:
> On Sat, Dec 27, 2014 at 09:41:45PM +0100, Paolo Bonzini wrote:
>>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>>> index f528343..6e52f3f 100644
>>> --- a/virt/kvm/kvm_main.c
>>> +++ b/virt/kvm/kvm_main.c
>>> @@ -672,6 +672,7 @@ static void update_memslots(struct kvm_memslots *slots,
>>>   	WARN_ON(mslots[i].id != id);
>>>   	if (!new->npages) {
>>>   		new->base_gfn = 0;
>>> +		new->flags = 0;
>>>   		if (mslots[i].npages)
>>>   			slots->used_slots--;
>>>   	} else {
>>
>> This should not be necessary.  The part of the mslots array that has
>> base_gfn == npages == 0 is entirely unused, and such a slot can never
>> be returned by search_memslots because this:
>>
>>          if (gfn >= memslots[slot].base_gfn &&
>>              gfn < memslots[slot].base_gfn + memslots[slot].npages)
>>
>> can never be true.
>>
>>> @@ -688,7 +689,9 @@ static void update_memslots(struct kvm_memslots *slots,
>>>   		i++;
>>>   	}
>>>   	while (i > 0 &&
>>> -	       new->base_gfn > mslots[i - 1].base_gfn) {
>>> +	       ((new->base_gfn > mslots[i - 1].base_gfn) ||
>>> +	        (!new->base_gfn &&
>>> +	         !mslots[i - 1].base_gfn && !mslots[i - 1].npages))) {
>>>   		mslots[i] = mslots[i - 1];
>>>   		slots->id_to_index[mslots[i].id] = i;
>>>   		i--;
>>>
>>
>> You should have explained _why_ this fixes the bug, and what invariant
>> is not being respected, something like this:
>>
>>      kvm: fix sorting of memslots with base_gfn == 0
>>
>>      Before commit 0e60b0799fed (kvm: change memslot sorting rule from size
>>      to GFN, 2014-12-01), the memslots' sorting key was npages, meaning
>>      that a valid memslot couldn't have its sorting key equal to zero.
>>      On the other hand, a valid memslot can have base_gfn == 0, and invalid
>>      memslots are identified by base_gfn == npages == 0.
>>
>>      Because of this, commit 0e60b0799fed broke the invariant that invalid
>>      memslots are at the end of the mslots array.  When a memslot with
>>      base_gfn == 0 was created, any invalid memslot before it were left
>>      in place.
>>
>> This suggests another fix.  We can change the insertion to use a ">="
>> comparison, as in your first patch.  Alone it is not correct, but we
>> only need to take some care and avoid breaking the case of deleting a
>> memslot.
>>
>> It's enough to wrap the second loop (that you patched) with
>> "if (new->npages)".  In the new->npages == 0 case the first loop has
>> already set i to the right value, and moving i back would be wrong:
>>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index f5283438ee05..050974c051b5 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -687,11 +687,23 @@ static void update_memslots(struct kvm_memslots *slots,
>>   		slots->id_to_index[mslots[i].id] = i;
>>   		i++;
>>   	}
>> -	while (i > 0 &&
>> -	       new->base_gfn > mslots[i - 1].base_gfn) {
>> -		mslots[i] = mslots[i - 1];
>> -		slots->id_to_index[mslots[i].id] = i;
>> -		i--;
>> +
>> +	/*
>> +	 * The ">=" is needed when creating a slot with base_gfn == 0,
>> +	 * so that it moves before all those with base_gfn == npages == 0.
>> +	 *
>> +	 * On the other hand, if new->npages is zero, the above loop has
>> +	 * already left i pointing to the beginning of the empty part of
>> +	 * mslots, and the ">=" would move the hole backwards in this
>> +	 * case---which is wrong.  So skip the loop when deleting a slot.
>> +	 */
>> +	if (new->npages) {
>> +		while (i > 0 &&
>> +		       new->base_gfn >= mslots[i - 1].base_gfn) {
>> +			mslots[i] = mslots[i - 1];
>> +			slots->id_to_index[mslots[i].id] = i;
>> +			i--;
>> +		}
>>   	}
>>
>>   	mslots[i] = *new;
>>
>> Paolo
>
> Paolo,
>
> Can you include a proper changelog for this patch?
>

But this is already applied long time ago...

Tiejun

Re: [PATCH] kvm: fix to update memslots properly

[Paolo Bonzini]

>>> This suggests another fix.  We can change the insertion to use a ">="
>>> comparison, as in your first patch.  Alone it is not correct, but we
>>> only need to take some care and avoid breaking the case of deleting a
>>> memslot.
>>>
>>> It's enough to wrap the second loop (that you patched) with
>>> "if (new->npages)".  In the new->npages == 0 case the first loop has
>>> already set i to the right value, and moving i back would be wrong:
>>>
>>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>>> index f5283438ee05..050974c051b5 100644
>>> --- a/virt/kvm/kvm_main.c
>>> +++ b/virt/kvm/kvm_main.c
>>> @@ -687,11 +687,23 @@ static void update_memslots(struct kvm_memslots
>>> *slots,
>>>           slots->id_to_index[mslots[i].id] = i;
>>>           i++;
>>>       }
>>> -    while (i > 0 &&
>>> -           new->base_gfn > mslots[i - 1].base_gfn) {
>>> -        mslots[i] = mslots[i - 1];
>>> -        slots->id_to_index[mslots[i].id] = i;
>>> -        i--;
>>> +
>>> +    /*
>>> +     * The ">=" is needed when creating a slot with base_gfn == 0,
>>> +     * so that it moves before all those with base_gfn == npages == 0.
>>> +     *
>>> +     * On the other hand, if new->npages is zero, the above loop has
>>> +     * already left i pointing to the beginning of the empty part of
>>> +     * mslots, and the ">=" would move the hole backwards in this
>>> +     * case---which is wrong.  So skip the loop when deleting a slot.
>>> +     */
>>> +    if (new->npages) {
>>> +        while (i > 0 &&
>>> +               new->base_gfn >= mslots[i - 1].base_gfn) {
>>> +            mslots[i] = mslots[i - 1];
>>> +            slots->id_to_index[mslots[i].id] = i;
>>> +            i--;
>>> +        }
>>>       }
>>>
>>>       mslots[i] = *new;
>>>
>>> Paolo
>>
>> Paolo,
>>
>> Can you include a proper changelog for this patch?
>>
> 
> But this is already applied long time ago...

Yes, this is commit efbeec7098eee2b3d2359d0cc24bbba0436e7f21.

Paolo