* Re: [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised
2020-09-30 10:24 [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised Will Deacon
@ 2020-09-30 10:30 ` Will Deacon
2020-09-30 10:47 ` Alexandru Elisei
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Will Deacon @ 2020-09-30 10:30 UTC (permalink / raw)
To: maz; +Cc: kernel test robot, Will Deacon, kernel-team, kvmarm, Dan Carpenter
On Wed, Sep 30, 2020 at 11:24:42AM +0100, Will Deacon wrote:
> From: Will Deacon <willdeacon@google.com>
Bugger, sorry, committed this in the wrong tree and got the wrong address on
it. Patch is still fine, but the author should be my kernel.org address, to
match the SoB.
Will
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised
2020-09-30 10:24 [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised Will Deacon
2020-09-30 10:30 ` Will Deacon
@ 2020-09-30 10:47 ` Alexandru Elisei
2020-10-01 0:59 ` Gavin Shan
2020-10-02 8:20 ` Marc Zyngier
3 siblings, 0 replies; 7+ messages in thread
From: Alexandru Elisei @ 2020-09-30 10:47 UTC (permalink / raw)
To: Will Deacon, maz
Cc: kernel test robot, Will Deacon, kernel-team, kvmarm, Dan Carpenter
Hi Will,
On 9/30/20 11:24 AM, Will Deacon wrote:
> From: Will Deacon <willdeacon@google.com>
>
> If a change in the MMU notifier sequence number forces user_mem_abort()
> to return early when attempting to handle a stage-2 fault, we return
> uninitialised stack to kvm_handle_guest_abort(), which could potentially
> result in the injection of an external abort into the guest or a spurious
> return to userspace. Neither or these are what we want to do.
>
> Initialise 'ret' to 0 in user_mem_abort() so that bailing due to a
> change in the MMU notrifier sequence number is treated as though the
> fault was handled.
>
> Cc: Gavin Shan <gshan@redhat.com>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>
> Applies on top of kvmarm/next.
>
> arch/arm64/kvm/mmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index c5c26a9cb85b..a816cb8e619b 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -742,7 +742,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> struct kvm_memory_slot *memslot, unsigned long hva,
> unsigned long fault_status)
> {
> - int ret;
> + int ret = 0;
> bool write_fault, writable, force_pte = false;
> bool exec_fault;
> bool device = false;
This matches the current behavior of user_mem_abort(), where ret = 0 from the call
to kvm_mmu_topup_memory_cache(), which was made conditional by the EL2 page table
rewrite. It makes sense to me - we return to the guest and take the fault again
until the changes to the translation tables have been executed (mmu_notifier_seq
remains the same and mmu_notifier_count == 0):
Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised
2020-09-30 10:24 [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised Will Deacon
2020-09-30 10:30 ` Will Deacon
2020-09-30 10:47 ` Alexandru Elisei
@ 2020-10-01 0:59 ` Gavin Shan
2020-10-02 8:20 ` Marc Zyngier
3 siblings, 0 replies; 7+ messages in thread
From: Gavin Shan @ 2020-10-01 0:59 UTC (permalink / raw)
To: Will Deacon, maz
Cc: kernel test robot, Will Deacon, kernel-team, kvmarm, Dan Carpenter
On 9/30/20 8:24 PM, Will Deacon wrote:
> From: Will Deacon <willdeacon@google.com>
>
> If a change in the MMU notifier sequence number forces user_mem_abort()
> to return early when attempting to handle a stage-2 fault, we return
> uninitialised stack to kvm_handle_guest_abort(), which could potentially
> result in the injection of an external abort into the guest or a spurious
> return to userspace. Neither or these are what we want to do.
>
> Initialise 'ret' to 0 in user_mem_abort() so that bailing due to a
> change in the MMU notrifier sequence number is treated as though the
> fault was handled.
>
> Cc: Gavin Shan <gshan@redhat.com>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>
Reviewed-by: Gavin Shan <gshan@redhat.com>
> Applies on top of kvmarm/next.
>
> arch/arm64/kvm/mmu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index c5c26a9cb85b..a816cb8e619b 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -742,7 +742,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
> struct kvm_memory_slot *memslot, unsigned long hva,
> unsigned long fault_status)
> {
> - int ret;
> + int ret = 0;
> bool write_fault, writable, force_pte = false;
> bool exec_fault;
> bool device = false;
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised
2020-09-30 10:24 [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised Will Deacon
2020-09-30 10:30 ` Will Deacon
@ 2020-10-02 8:20 ` Marc Zyngier
2020-10-01 0:59 ` Gavin Shan
2020-10-02 8:20 ` Marc Zyngier
3 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2020-10-02 8:20 UTC (permalink / raw)
To: kvmarm, David Brazdil, Will Deacon
Cc: linux-arm-kernel, linux-kernel, Dennis Zhou, Catalin Marinas,
Tejun Heo, kernel-team, Christoph Lameter, kernel test robot,
Will Deacon, Dan Carpenter
On Wed, 30 Sep 2020 11:24:42 +0100, Will Deacon wrote:
> If a change in the MMU notifier sequence number forces user_mem_abort()
> to return early when attempting to handle a stage-2 fault, we return
> uninitialised stack to kvm_handle_guest_abort(), which could potentially
> result in the injection of an external abort into the guest or a spurious
> return to userspace. Neither or these are what we want to do.
>
> Initialise 'ret' to 0 in user_mem_abort() so that bailing due to a
> change in the MMU notrifier sequence number is treated as though the
> fault was handled.
Applied to next, thanks!
[1/1] KVM: arm64: Ensure user_mem_abort() return value is initialised
commit: 84cd7df693f07df94d617049773f7c757a2b7847
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised
@ 2020-10-02 8:20 ` Marc Zyngier
0 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2020-10-02 8:20 UTC (permalink / raw)
To: kvmarm, David Brazdil, Will Deacon
Cc: kernel test robot, Will Deacon, Catalin Marinas, linux-kernel,
Dan Carpenter, Tejun Heo, Dennis Zhou, Christoph Lameter,
kernel-team, linux-arm-kernel
On Wed, 30 Sep 2020 11:24:42 +0100, Will Deacon wrote:
> If a change in the MMU notifier sequence number forces user_mem_abort()
> to return early when attempting to handle a stage-2 fault, we return
> uninitialised stack to kvm_handle_guest_abort(), which could potentially
> result in the injection of an external abort into the guest or a spurious
> return to userspace. Neither or these are what we want to do.
>
> Initialise 'ret' to 0 in user_mem_abort() so that bailing due to a
> change in the MMU notrifier sequence number is treated as though the
> fault was handled.
Applied to next, thanks!
[1/1] KVM: arm64: Ensure user_mem_abort() return value is initialised
commit: 84cd7df693f07df94d617049773f7c757a2b7847
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] KVM: arm64: Ensure user_mem_abort() return value is initialised
@ 2020-10-02 8:20 ` Marc Zyngier
0 siblings, 0 replies; 7+ messages in thread
From: Marc Zyngier @ 2020-10-02 8:20 UTC (permalink / raw)
To: kvmarm, David Brazdil, Will Deacon
Cc: kernel test robot, Will Deacon, Catalin Marinas, linux-kernel,
Dan Carpenter, Tejun Heo, Dennis Zhou, Christoph Lameter,
kernel-team, linux-arm-kernel
On Wed, 30 Sep 2020 11:24:42 +0100, Will Deacon wrote:
> If a change in the MMU notifier sequence number forces user_mem_abort()
> to return early when attempting to handle a stage-2 fault, we return
> uninitialised stack to kvm_handle_guest_abort(), which could potentially
> result in the injection of an external abort into the guest or a spurious
> return to userspace. Neither or these are what we want to do.
>
> Initialise 'ret' to 0 in user_mem_abort() so that bailing due to a
> change in the MMU notrifier sequence number is treated as though the
> fault was handled.
Applied to next, thanks!
[1/1] KVM: arm64: Ensure user_mem_abort() return value is initialised
commit: 84cd7df693f07df94d617049773f7c757a2b7847
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 7+ messages in thread