1. Patchew
  2. QEMU
  3. View series

[PATCH v2] riscv: Make sure an exception is raised if a pte is malformed

Alexandre Ghiti posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230419104756.71455-1-alexghiti@rivosinc.com
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
target/riscv/cpu_bits.h   |  1 +
target/riscv/cpu_helper.c | 15 +++++++++++----
2 files changed, 12 insertions(+), 4 deletions(-)
[PATCH v2] riscv: Make sure an exception is raised if a pte is malformed
Posted by Alexandre Ghiti 1 year ago 
As per the privileged specification, in 64-bit, if any of the pte reserved
bits 60-54 is set an exception should be triggered, and the same applies to
napot/pbmt bits if those extensions are not enabled
(see 4.4.1, "Addressing and Memory Protection").

Reported-by: Andrea Parri <andrea@rivosinc.com>
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
 target/riscv/cpu_bits.h   |  1 +
 target/riscv/cpu_helper.c | 15 +++++++++++----
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fca7ef0cef..8d9ba2ce11 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -640,6 +640,7 @@ typedef enum {
 #define PTE_SOFT            0x300 /* Reserved for Software */
 #define PTE_PBMT            0x6000000000000000ULL /* Page-based memory types */
 #define PTE_N               0x8000000000000000ULL /* NAPOT translation */
+#define PTE_RESERVED        0x1FC0000000000000ULL /* Reserved bits */
 #define PTE_ATTR            (PTE_N | PTE_PBMT) /* All attributes bits */
 
 /* Page table PPN shift amount */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f88c503cf4..8dc832d1bb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -946,13 +946,20 @@ restart:
 
         if (riscv_cpu_sxl(env) == MXL_RV32) {
             ppn = pte >> PTE_PPN_SHIFT;
-        } else if (pbmte || cpu->cfg.ext_svnapot) {
-            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
         } else {
-            ppn = pte >> PTE_PPN_SHIFT;
-            if ((pte & ~(target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT) {
+            if (pte & PTE_RESERVED) {
+                return TRANSLATE_FAIL;
+            }
+
+            if (!pbmte && (pte & PTE_PBMT)) {
                 return TRANSLATE_FAIL;
             }
+
+            if (!cpu->cfg.ext_svnapot && (pte & PTE_N)) {
+                return TRANSLATE_FAIL;
+            }
+
+            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
         }
 
         if (!(pte & PTE_V)) {
-- 
2.37.2
Re: [PATCH v2] riscv: Make sure an exception is raised if a pte is malformed
Posted by Alistair Francis 1 year ago 
On Wed, Apr 19, 2023 at 8:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> As per the privileged specification, in 64-bit, if any of the pte reserved
> bits 60-54 is set an exception should be triggered, and the same applies to
> napot/pbmt bits if those extensions are not enabled
> (see 4.4.1, "Addressing and Memory Protection").
>
> Reported-by: Andrea Parri <andrea@rivosinc.com>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks for the patch

Do you mind sending a v3 rebased on
https://github.com/alistair23/qemu/tree/riscv-to-apply.next ?

Alistair

> ---
>  target/riscv/cpu_bits.h   |  1 +
>  target/riscv/cpu_helper.c | 15 +++++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index fca7ef0cef..8d9ba2ce11 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -640,6 +640,7 @@ typedef enum {
>  #define PTE_SOFT            0x300 /* Reserved for Software */
>  #define PTE_PBMT            0x6000000000000000ULL /* Page-based memory types */
>  #define PTE_N               0x8000000000000000ULL /* NAPOT translation */
> +#define PTE_RESERVED        0x1FC0000000000000ULL /* Reserved bits */
>  #define PTE_ATTR            (PTE_N | PTE_PBMT) /* All attributes bits */
>
>  /* Page table PPN shift amount */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..8dc832d1bb 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -946,13 +946,20 @@ restart:
>
>          if (riscv_cpu_sxl(env) == MXL_RV32) {
>              ppn = pte >> PTE_PPN_SHIFT;
> -        } else if (pbmte || cpu->cfg.ext_svnapot) {
> -            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>          } else {
> -            ppn = pte >> PTE_PPN_SHIFT;
> -            if ((pte & ~(target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT) {
> +            if (pte & PTE_RESERVED) {
> +                return TRANSLATE_FAIL;
> +            }
> +
> +            if (!pbmte && (pte & PTE_PBMT)) {
>                  return TRANSLATE_FAIL;
>              }
> +
> +            if (!cpu->cfg.ext_svnapot && (pte & PTE_N)) {
> +                return TRANSLATE_FAIL;
> +            }
> +
> +            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>          }
>
>          if (!(pte & PTE_V)) {
> --
> 2.37.2
>
>
Re: [PATCH v2] riscv: Make sure an exception is raised if a pte is malformed
Posted by Alexandre Ghiti 1 year ago 
On Thu, Apr 20, 2023 at 1:31 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Apr 19, 2023 at 8:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
> >
> > As per the privileged specification, in 64-bit, if any of the pte reserved
> > bits 60-54 is set an exception should be triggered, and the same applies to
> > napot/pbmt bits if those extensions are not enabled
> > (see 4.4.1, "Addressing and Memory Protection").
> >
> > Reported-by: Andrea Parri <andrea@rivosinc.com>
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>
> Thanks for the patch
>
> Do you mind sending a v3 rebased on
> https://github.com/alistair23/qemu/tree/riscv-to-apply.next ?

Sure, I have just sent the v3.

Thanks for your quick review!

Alex

>
> Alistair
>
> > ---
> >  target/riscv/cpu_bits.h   |  1 +
> >  target/riscv/cpu_helper.c | 15 +++++++++++----
> >  2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index fca7ef0cef..8d9ba2ce11 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -640,6 +640,7 @@ typedef enum {
> >  #define PTE_SOFT            0x300 /* Reserved for Software */
> >  #define PTE_PBMT            0x6000000000000000ULL /* Page-based memory types */
> >  #define PTE_N               0x8000000000000000ULL /* NAPOT translation */
> > +#define PTE_RESERVED        0x1FC0000000000000ULL /* Reserved bits */
> >  #define PTE_ATTR            (PTE_N | PTE_PBMT) /* All attributes bits */
> >
> >  /* Page table PPN shift amount */
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index f88c503cf4..8dc832d1bb 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -946,13 +946,20 @@ restart:
> >
> >          if (riscv_cpu_sxl(env) == MXL_RV32) {
> >              ppn = pte >> PTE_PPN_SHIFT;
> > -        } else if (pbmte || cpu->cfg.ext_svnapot) {
> > -            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> >          } else {
> > -            ppn = pte >> PTE_PPN_SHIFT;
> > -            if ((pte & ~(target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT) {
> > +            if (pte & PTE_RESERVED) {
> > +                return TRANSLATE_FAIL;
> > +            }
> > +
> > +            if (!pbmte && (pte & PTE_PBMT)) {
> >                  return TRANSLATE_FAIL;
> >              }
> > +
> > +            if (!cpu->cfg.ext_svnapot && (pte & PTE_N)) {
> > +                return TRANSLATE_FAIL;
> > +            }
> > +
> > +            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> >          }
> >
> >          if (!(pte & PTE_V)) {
> > --
> > 2.37.2
> >
> >
Re: [PATCH v2] riscv: Make sure an exception is raised if a pte is malformed
Posted by Alistair Francis 1 year ago 
On Wed, Apr 19, 2023 at 8:48 PM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> As per the privileged specification, in 64-bit, if any of the pte reserved
> bits 60-54 is set an exception should be triggered, and the same applies to
> napot/pbmt bits if those extensions are not enabled
> (see 4.4.1, "Addressing and Memory Protection").
>
> Reported-by: Andrea Parri <andrea@rivosinc.com>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu_bits.h   |  1 +
>  target/riscv/cpu_helper.c | 15 +++++++++++----
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index fca7ef0cef..8d9ba2ce11 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -640,6 +640,7 @@ typedef enum {
>  #define PTE_SOFT            0x300 /* Reserved for Software */
>  #define PTE_PBMT            0x6000000000000000ULL /* Page-based memory types */
>  #define PTE_N               0x8000000000000000ULL /* NAPOT translation */
> +#define PTE_RESERVED        0x1FC0000000000000ULL /* Reserved bits */
>  #define PTE_ATTR            (PTE_N | PTE_PBMT) /* All attributes bits */
>
>  /* Page table PPN shift amount */
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..8dc832d1bb 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -946,13 +946,20 @@ restart:
>
>          if (riscv_cpu_sxl(env) == MXL_RV32) {
>              ppn = pte >> PTE_PPN_SHIFT;
> -        } else if (pbmte || cpu->cfg.ext_svnapot) {
> -            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>          } else {
> -            ppn = pte >> PTE_PPN_SHIFT;
> -            if ((pte & ~(target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT) {
> +            if (pte & PTE_RESERVED) {
> +                return TRANSLATE_FAIL;
> +            }
> +
> +            if (!pbmte && (pte & PTE_PBMT)) {
>                  return TRANSLATE_FAIL;
>              }
> +
> +            if (!cpu->cfg.ext_svnapot && (pte & PTE_N)) {
> +                return TRANSLATE_FAIL;
> +            }
> +
> +            ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>          }
>
>          if (!(pte & PTE_V)) {
> --
> 2.37.2
>
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.