[PATCH v1 1/1] KVM: s390: VSIE: fix virtual/physical address in unpin_scb()

[PATCH v1 1/1] KVM: s390: VSIE: fix virtual/physical address in unpin_scb()

[Claudio Imbrenda]

In commit 77b533411595 ("KVM: s390: VSIE: sort out virtual/physical
address in pin_guest_page"), only pin_scb() has been updated. This
means that in unpin_scb() a virtual address was still used directly as
physical address without conversion. The resulting physical address is
obviously wrong and most of the time also invalid.

Since commit d0ef8d9fbebe ("KVM: s390: Use kvm_release_page_dirty() to
unpin "struct page" memory"), unpin_guest_page() will directly use
kvm_release_page_dirty(), instead of kvm_release_pfn_dirty(), which has
since been removed.

One of the checks that were performed by kvm_release_pfn_dirty() was to
verify whether the page was valid at all, and silently return
successfully without doing anything if the page was invalid.

When kvm_release_pfn_dirty() was still used, the invalid page was thus
silently ignored. Now the check is gone and the result is an Oops.
This also means that when running with a V!=R kernel, the page was not
released, causing a leak.

The solution is simply to add the missing virt_to_phys().

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Fixes: 77b533411595 ("KVM: s390: VSIE: sort out virtual/physical address in pin_guest_page")
---
 arch/s390/kvm/vsie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index 150b9387860a..a687695d8f68 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -854,7 +854,7 @@ static int pin_blocks(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 static void unpin_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
 		      gpa_t gpa)
 {
-	hpa_t hpa = (hpa_t) vsie_page->scb_o;
+	hpa_t hpa = virt_to_phys(vsie_page->scb_o);
 
 	if (hpa)
 		unpin_guest_page(vcpu->kvm, gpa, hpa);
-- 
2.47.1

Re: [PATCH v1 1/1] KVM: s390: VSIE: fix virtual/physical address in unpin_scb()

[Janosch Frank]

On 12/10/24 9:39 AM, Claudio Imbrenda wrote:
> In commit 77b533411595 ("KVM: s390: VSIE: sort out virtual/physical
> address in pin_guest_page"), only pin_scb() has been updated. This
> means that in unpin_scb() a virtual address was still used directly as
> physical address without conversion. The resulting physical address is
> obviously wrong and most of the time also invalid.
> 
> Since commit d0ef8d9fbebe ("KVM: s390: Use kvm_release_page_dirty() to
> unpin "struct page" memory"), unpin_guest_page() will directly use
> kvm_release_page_dirty(), instead of kvm_release_pfn_dirty(), which has
> since been removed.
> 
> One of the checks that were performed by kvm_release_pfn_dirty() was to
> verify whether the page was valid at all, and silently return
> successfully without doing anything if the page was invalid.
> 
> When kvm_release_pfn_dirty() was still used, the invalid page was thus
> silently ignored. Now the check is gone and the result is an Oops.
> This also means that when running with a V!=R kernel, the page was not
> released, causing a leak.
> 
> The solution is simply to add the missing virt_to_phys()

Please lower-case the "VSIE" in the subject.
I know that you're replicating the subject prefix from the patch you're 
fixing but this looks weird.

Thanks for fixing this so quickly.
Please push this for CI coverage if you haven't already.

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>

Re: [PATCH v1 1/1] KVM: s390: VSIE: fix virtual/physical address in unpin_scb()

[Nico Boehr]

On Tue Dec 10, 2024 at 9:39 AM CET, Claudio Imbrenda wrote:
> In commit 77b533411595 ("KVM: s390: VSIE: sort out virtual/physical
> address in pin_guest_page"), only pin_scb() has been updated. This
> means that in unpin_scb() a virtual address was still used directly as
> physical address without conversion. The resulting physical address is
> obviously wrong and most of the time also invalid.
>
> Since commit d0ef8d9fbebe ("KVM: s390: Use kvm_release_page_dirty() to
> unpin "struct page" memory"), unpin_guest_page() will directly use
> kvm_release_page_dirty(), instead of kvm_release_pfn_dirty(), which has
> since been removed.
>
> One of the checks that were performed by kvm_release_pfn_dirty() was to
> verify whether the page was valid at all, and silently return
> successfully without doing anything if the page was invalid.
>
> When kvm_release_pfn_dirty() was still used, the invalid page was thus
> silently ignored. Now the check is gone and the result is an Oops.
> This also means that when running with a V!=R kernel, the page was not
> released, causing a leak.
>
> The solution is simply to add the missing virt_to_phys().
>
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Fixes: 77b533411595 ("KVM: s390: VSIE: sort out virtual/physical address in pin_guest_page")

Reviewed-by: Nico Boehr <nrb@linux.ibm.com>