The addresses in the SIE control block of the host should not be forwarded to the guest. They are only meaningful to the host, and moreover it would be a clear security issue.
Subsequent patches will actually put the right values in the guest SIE control block.
Fixes: a3508fbe9dc6d ("KVM: s390: vsie: initial support for nested virtualization") Cc: stable@vger.kernel.org Signed-off-by: Claudio Imbrenda imbrenda@linux.ibm.com --- arch/s390/kvm/vsie.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index 4f3cbf6003a9..ada49583e530 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -416,11 +416,6 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) memcpy((void *)((u64)scb_o + 0xc0), (void *)((u64)scb_s + 0xc0), 0xf0 - 0xc0); break; - case ICPT_PARTEXEC: - /* MVPG only */ - memcpy((void *)((u64)scb_o + 0xc0), - (void *)((u64)scb_s + 0xc0), 0xd0 - 0xc0); - break; }
if (scb_s->ihcpu != 0xffffU)
On 18.12.20 15:18, Claudio Imbrenda wrote:
The addresses in the SIE control block of the host should not be forwarded to the guest. They are only meaningful to the host, and moreover it would be a clear security issue.
It's really almost impossible for someone without access to documentation to understand what we leak. I assume we're leaking the g1 address of a page table (entry), used for translation of g2->g3 to g1. Can you try making that clearer?
In that case, it's pretty much a random number (of a random page used as a leave page table) and does not let g1 identify locations of symbols etc. If so, I don't think this is a "clear security issue" and suggest squashing this into the actual fix (#p4 I assume).
@Christian, @Janosch? Am I missing something?
Subsequent patches will actually put the right values in the guest SIE control block.
Fixes: a3508fbe9dc6d ("KVM: s390: vsie: initial support for nested virtualization") Cc: stable@vger.kernel.org Signed-off-by: Claudio Imbrenda imbrenda@linux.ibm.com
arch/s390/kvm/vsie.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index 4f3cbf6003a9..ada49583e530 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -416,11 +416,6 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) memcpy((void *)((u64)scb_o + 0xc0), (void *)((u64)scb_s + 0xc0), 0xf0 - 0xc0); break;
- case ICPT_PARTEXEC:
/* MVPG only */
memcpy((void *)((u64)scb_o + 0xc0),
(void *)((u64)scb_s + 0xc0), 0xd0 - 0xc0);
}break;
if (scb_s->ihcpu != 0xffffU)
On Sun, 20 Dec 2020 10:44:56 +0100 David Hildenbrand david@redhat.com wrote:
On 18.12.20 15:18, Claudio Imbrenda wrote:
The addresses in the SIE control block of the host should not be forwarded to the guest. They are only meaningful to the host, and moreover it would be a clear security issue.
It's really almost impossible for someone without access to documentation to understand what we leak. I assume we're leaking the g1 address of a page table (entry), used for translation of g2->g3 to g1. Can you try making that clearer?
this is correct.
I guess I can improve the text of the commit
In that case, it's pretty much a random number (of a random page used as a leave page table) and does not let g1 identify locations of symbols etc. If so, I don't think this is a "clear security issue" and suggest squashing this into the actual fix (#p4 I assume).
yeah __maybe__ I overstated the importance ;)
But I would still like to keep it as a separate patch, looks more straightforward to me
@Christian, @Janosch? Am I missing something?
Subsequent patches will actually put the right values in the guest SIE control block.
Fixes: a3508fbe9dc6d ("KVM: s390: vsie: initial support for nested virtualization") Cc: stable@vger.kernel.org Signed-off-by: Claudio Imbrenda imbrenda@linux.ibm.com
arch/s390/kvm/vsie.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index 4f3cbf6003a9..ada49583e530 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -416,11 +416,6 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) memcpy((void *)((u64)scb_o + 0xc0), (void *)((u64)scb_s + 0xc0), 0xf0 - 0xc0); break;
- case ICPT_PARTEXEC:
/* MVPG only */
memcpy((void *)((u64)scb_o + 0xc0),
(void *)((u64)scb_s + 0xc0), 0xd0 - 0xc0);
}break;
if (scb_s->ihcpu != 0xffffU)
In that case, it's pretty much a random number (of a random page used as a leave page table) and does not let g1 identify locations of symbols etc. If so, I don't think this is a "clear security issue" and suggest squashing this into the actual fix (#p4 I assume).
yeah __maybe__ I overstated the importance ;)
But I would still like to keep it as a separate patch, looks more straightforward to me
I don't see a need to split this up. Just fix it right away.
On 12/18/20 3:18 PM, Claudio Imbrenda wrote:
The addresses in the SIE control block of the host should not be forwarded to the guest. They are only meaningful to the host, and moreover it would be a clear security issue.
Subsequent patches will actually put the right values in the guest SIE control block.
Fixes: a3508fbe9dc6d ("KVM: s390: vsie: initial support for nested virtualization") Cc: stable@vger.kernel.org Signed-off-by: Claudio Imbrenda imbrenda@linux.ibm.com
Looks reasonable. Give me some time to have a deeper look it's a lot of architecture to read.
arch/s390/kvm/vsie.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c index 4f3cbf6003a9..ada49583e530 100644 --- a/arch/s390/kvm/vsie.c +++ b/arch/s390/kvm/vsie.c @@ -416,11 +416,6 @@ static void unshadow_scb(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page) memcpy((void *)((u64)scb_o + 0xc0), (void *)((u64)scb_s + 0xc0), 0xf0 - 0xc0); break;
- case ICPT_PARTEXEC:
/* MVPG only */
memcpy((void *)((u64)scb_o + 0xc0),
(void *)((u64)scb_s + 0xc0), 0xd0 - 0xc0);
}break;
if (scb_s->ihcpu != 0xffffU)
linux-stable-mirror@lists.linaro.org