linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix off-by-one in range_is_memory
@ 2021-07-28 15:32 David Brazdil
  2021-07-28 15:32 ` [PATCH 1/2] KVM: arm64: " David Brazdil
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: David Brazdil @ 2021-07-28 15:32 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose,
	Catalin Marinas, Will Deacon, Quentin Perret, linux-arm-kernel,
	linux-kernel, David Brazdil

Hi, here is an off-by-one bug fix and a very minor improvement for
the range_is_memory function in hyp.

David Brazdil (2):
  KVM: arm64: Fix off-by-one in range_is_memory
  KVM: arm64: Minor optimization of range_is_memory

 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] KVM: arm64: Fix off-by-one in range_is_memory
  2021-07-28 15:32 [PATCH 0/2] Fix off-by-one in range_is_memory David Brazdil
@ 2021-07-28 15:32 ` David Brazdil
  2021-07-29 16:52   ` Quentin Perret
  2021-07-28 15:32 ` [PATCH 2/2] KVM: arm64: Minor optimization of range_is_memory David Brazdil
  2021-08-20 11:05 ` (subset) [PATCH 0/2] Fix off-by-one in range_is_memory Marc Zyngier
  2 siblings, 1 reply; 6+ messages in thread
From: David Brazdil @ 2021-07-28 15:32 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose,
	Catalin Marinas, Will Deacon, Quentin Perret, linux-arm-kernel,
	linux-kernel, David Brazdil

Hyp checks whether an address range only covers RAM by checking the
start/endpoints against a list of memblock_region structs. However,
the endpoint here is exclusive but internally is treated as inclusive.
Fix the off-by-one error that caused valid address ranges to be
rejected.

Cc: Quentin Perret <qperret@google.com>
Fixes: 90134ac9cabb6 ("KVM: arm64: Protect the .hyp sections from the host")
Signed-off-by: David Brazdil <dbrazdil@google.com>
---
 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index d938ce95d3bd..a6ce991b1467 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -193,7 +193,7 @@ static bool range_is_memory(u64 start, u64 end)
 {
 	struct kvm_mem_range r1, r2;
 
-	if (!find_mem_range(start, &r1) || !find_mem_range(end, &r2))
+	if (!find_mem_range(start, &r1) || !find_mem_range(end - 1, &r2))
 		return false;
 	if (r1.start != r2.start)
 		return false;
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] KVM: arm64: Minor optimization of range_is_memory
  2021-07-28 15:32 [PATCH 0/2] Fix off-by-one in range_is_memory David Brazdil
  2021-07-28 15:32 ` [PATCH 1/2] KVM: arm64: " David Brazdil
@ 2021-07-28 15:32 ` David Brazdil
  2021-07-29 17:00   ` Quentin Perret
  2021-08-20 11:05 ` (subset) [PATCH 0/2] Fix off-by-one in range_is_memory Marc Zyngier
  2 siblings, 1 reply; 6+ messages in thread
From: David Brazdil @ 2021-07-28 15:32 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, James Morse, Alexandru Elisei, Suzuki K Poulose,
	Catalin Marinas, Will Deacon, Quentin Perret, linux-arm-kernel,
	linux-kernel, David Brazdil

Currently range_is_memory finds the corresponding struct memblock_region
for both the lower and upper bounds of the given address range with two
rounds of binary search, and then checks that the two memblocks are the
same. Simplify this by only doing binary search on the lower bound and
then checking that the upper bound is in the same memblock.

Signed-off-by: David Brazdil <dbrazdil@google.com>
---
 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index a6ce991b1467..37d73af69634 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -189,13 +189,18 @@ static bool find_mem_range(phys_addr_t addr, struct kvm_mem_range *range)
 	return false;
 }
 
+static bool is_in_mem_range(phys_addr_t addr, struct kvm_mem_range *range)
+{
+	return range->start <= addr && addr < range->end;
+}
+
 static bool range_is_memory(u64 start, u64 end)
 {
-	struct kvm_mem_range r1, r2;
+	struct kvm_mem_range r;
 
-	if (!find_mem_range(start, &r1) || !find_mem_range(end - 1, &r2))
+	if (!find_mem_range(start, &r))
 		return false;
-	if (r1.start != r2.start)
+	if (!is_in_mem_range(end - 1, &r))
 		return false;
 
 	return true;
-- 
2.32.0.432.gabb21c7263-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] KVM: arm64: Fix off-by-one in range_is_memory
  2021-07-28 15:32 ` [PATCH 1/2] KVM: arm64: " David Brazdil
@ 2021-07-29 16:52   ` Quentin Perret
  0 siblings, 0 replies; 6+ messages in thread
From: Quentin Perret @ 2021-07-29 16:52 UTC (permalink / raw)
  To: David Brazdil
  Cc: kvmarm, Marc Zyngier, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Catalin Marinas, Will Deacon, linux-arm-kernel,
	linux-kernel

On Wednesday 28 Jul 2021 at 15:32:31 (+0000), David Brazdil wrote:
> Hyp checks whether an address range only covers RAM by checking the
> start/endpoints against a list of memblock_region structs. However,
> the endpoint here is exclusive but internally is treated as inclusive.
> Fix the off-by-one error that caused valid address ranges to be
> rejected.
> 
> Cc: Quentin Perret <qperret@google.com>
> Fixes: 90134ac9cabb6 ("KVM: arm64: Protect the .hyp sections from the host")
> Signed-off-by: David Brazdil <dbrazdil@google.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index d938ce95d3bd..a6ce991b1467 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> @@ -193,7 +193,7 @@ static bool range_is_memory(u64 start, u64 end)
>  {
>  	struct kvm_mem_range r1, r2;
>  
> -	if (!find_mem_range(start, &r1) || !find_mem_range(end, &r2))
> +	if (!find_mem_range(start, &r1) || !find_mem_range(end - 1, &r2))
>  		return false;
>  	if (r1.start != r2.start)
>  		return false;

Looks good to me:

Reviewed-by: Quentin Perret <qperret@google.com>

Thanks,
Quentin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] KVM: arm64: Minor optimization of range_is_memory
  2021-07-28 15:32 ` [PATCH 2/2] KVM: arm64: Minor optimization of range_is_memory David Brazdil
@ 2021-07-29 17:00   ` Quentin Perret
  0 siblings, 0 replies; 6+ messages in thread
From: Quentin Perret @ 2021-07-29 17:00 UTC (permalink / raw)
  To: David Brazdil
  Cc: kvmarm, Marc Zyngier, James Morse, Alexandru Elisei,
	Suzuki K Poulose, Catalin Marinas, Will Deacon, linux-arm-kernel,
	linux-kernel

On Wednesday 28 Jul 2021 at 15:32:32 (+0000), David Brazdil wrote:
> Currently range_is_memory finds the corresponding struct memblock_region
> for both the lower and upper bounds of the given address range with two
> rounds of binary search, and then checks that the two memblocks are the
> same. Simplify this by only doing binary search on the lower bound and
> then checking that the upper bound is in the same memblock.
> 
> Signed-off-by: David Brazdil <dbrazdil@google.com>
> ---
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index a6ce991b1467..37d73af69634 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> @@ -189,13 +189,18 @@ static bool find_mem_range(phys_addr_t addr, struct kvm_mem_range *range)
>  	return false;
>  }
>  
> +static bool is_in_mem_range(phys_addr_t addr, struct kvm_mem_range *range)
> +{

Nit: addr@ could be u64 for consistency -- struct kvm_mem_range holds
IPAs in general.

> +	return range->start <= addr && addr < range->end;
> +}
> +
>  static bool range_is_memory(u64 start, u64 end)
>  {
> -	struct kvm_mem_range r1, r2;
> +	struct kvm_mem_range r;
>  
> -	if (!find_mem_range(start, &r1) || !find_mem_range(end - 1, &r2))
> +	if (!find_mem_range(start, &r))
>  		return false;
> -	if (r1.start != r2.start)
> +	if (!is_in_mem_range(end - 1, &r))
>  		return false;
>  
>  	return true;

Nit: maybe drop the second if and simplify to:

	return is_in_mem_range(end - 1, &r);

With that:

Reviewed-by: Quentin Perret <qperret@google.com>

Thanks,
Quentin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: (subset) [PATCH 0/2] Fix off-by-one in range_is_memory
  2021-07-28 15:32 [PATCH 0/2] Fix off-by-one in range_is_memory David Brazdil
  2021-07-28 15:32 ` [PATCH 1/2] KVM: arm64: " David Brazdil
  2021-07-28 15:32 ` [PATCH 2/2] KVM: arm64: Minor optimization of range_is_memory David Brazdil
@ 2021-08-20 11:05 ` Marc Zyngier
  2 siblings, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2021-08-20 11:05 UTC (permalink / raw)
  To: kvmarm, David Brazdil
  Cc: linux-kernel, Will Deacon, Quentin Perret, Alexandru Elisei,
	linux-arm-kernel, James Morse, Suzuki K Poulose, Catalin Marinas

On Wed, 28 Jul 2021 15:32:30 +0000, David Brazdil wrote:
> the range_is_memory function in hyp.
> 
> David Brazdil (2):
>   KVM: arm64: Fix off-by-one in range_is_memory
>   KVM: arm64: Minor optimization of range_is_memory
> 
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)

Applied to kvm-arm64/mmu/el2-tracking, thanks!

[2/2] KVM: arm64: Minor optimization of range_is_memory
      commit: 14ecf075fe5be01860927fdf3aa11d7b18023ab2

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-08-20 11:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28 15:32 [PATCH 0/2] Fix off-by-one in range_is_memory David Brazdil
2021-07-28 15:32 ` [PATCH 1/2] KVM: arm64: " David Brazdil
2021-07-29 16:52   ` Quentin Perret
2021-07-28 15:32 ` [PATCH 2/2] KVM: arm64: Minor optimization of range_is_memory David Brazdil
2021-07-29 17:00   ` Quentin Perret
2021-08-20 11:05 ` (subset) [PATCH 0/2] Fix off-by-one in range_is_memory Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).