[v20210701,11/40] tools: use sr_is_known_page_type

Commit Message

Olaf Hering July 1, 2021, 9:56 a.m. UTC

Verify pfn type on sending side, also verify incoming batch of pfns.

Signed-off-by: Olaf Hering <[e-mail]>
Reviewed-by: Juergen Gross <[e-mail]>

v02:
- use sr_is_known_page_type instead of xc_is_known_page_type
---
 tools/libs/saverestore/restore.c | 3 +--
 tools/libs/saverestore/save.c    | 6 ++++++
 2 files changed, 7 insertions(+), 2 deletions(-)

Comments

Andrew Cooper July 2, 2021, 7:27 p.m. UTC | #1

On 01/07/2021 10:56, Olaf Hering wrote:
> Verify pfn type on sending side, also verify incoming batch of pfns.
>
> Signed-off-by: Olaf Hering <[e-mail]>
> Reviewed-by: Juergen Gross <[e-mail]>

Any reason this isn't folded into the previous patch, like your
subsequent two page type helper patches are?

> diff --git a/tools/libs/saverestore/save.c b/tools/libs/saverestore/save.c
> index ae3e8797d0..6f820ea432 100644
> --- a/tools/libs/saverestore/save.c
> +++ b/tools/libs/saverestore/save.c
> @@ -147,6 +147,12 @@ static int write_batch(struct xc_sr_context *ctx)
>  
>      for ( i = 0; i < nr_pfns; ++i )
>      {
> +        if ( sr_is_known_page_type(types[i]) == false )
> +        {
> +            ERROR("Wrong type %#"PRIpfn" for pfn %#"PRIpfn, types[i], mfns[i]);

"Unknown type" would be more accurate.

~Andrew

Olaf Hering July 5, 2021, 8:25 a.m. UTC | #2

Am Fri, 2 Jul 2021 20:27:21 +0100
schrieb Andrew Cooper <[e-mail]>:

> Any reason this isn't folded into the previous patch, like your
> subsequent two page type helper patches are?

I think I wanted to separate this for simpler review, but I forgot to split the followup change as well.

> > +            ERROR("Wrong type %#"PRIpfn" for pfn %#"PRIpfn, types[i], mfns[i]);  
> "Unknown type" would be more accurate.

Yes, this is better. Thanks.

Olaf

Andrew Cooper July 5, 2021, 9:53 a.m. UTC | #3

On 05/07/2021 09:25, Olaf Hering wrote:
> Am Fri, 2 Jul 2021 20:27:21 +0100
> schrieb Andrew Cooper <[e-mail]>:
>
>> Any reason this isn't folded into the previous patch, like your
>> subsequent two page type helper patches are?
> I think I wanted to separate this for simpler review, but I forgot to split the followup change as well.

All patches are largely mechanical changes.  It's easier to review
together, rather than split, because you can only judge the correctness
of the new helper in terms of the code it is replacing.

~Andrew

Patch

diff --git a/tools/libs/saverestore/restore.c b/tools/libs/saverestore/restore.c
index be259a1c6b..324b9050e2 100644
--- a/tools/libs/saverestore/restore.c
+++ b/tools/libs/saverestore/restore.c
@@ -406,8 +406,7 @@  static int handle_page_data(struct xc_sr_context *ctx, struct xc_sr_record *rec)
         }
 
         type = (pages->pfn[i] & PAGE_DATA_TYPE_MASK) >> 32;
-        if ( ((type >> XEN_DOMCTL_PFINFO_LTAB_SHIFT) >= 5) &&
-             ((type >> XEN_DOMCTL_PFINFO_LTAB_SHIFT) <= 8) )
+        if ( sr_is_known_page_type(type) == false )
         {
             ERROR("Invalid type %#"PRIx32" for pfn %#"PRIpfn" (index %u)",
                   type, pfn, i);
diff --git a/tools/libs/saverestore/save.c b/tools/libs/saverestore/save.c
index ae3e8797d0..6f820ea432 100644
--- a/tools/libs/saverestore/save.c
+++ b/tools/libs/saverestore/save.c
@@ -147,6 +147,12 @@  static int write_batch(struct xc_sr_context *ctx)
 
     for ( i = 0; i < nr_pfns; ++i )
     {
+        if ( sr_is_known_page_type(types[i]) == false )
+        {
+            ERROR("Wrong type %#"PRIpfn" for pfn %#"PRIpfn, types[i], mfns[i]);
+            goto err;
+        }
+
         switch ( types[i] )
         {
         case XEN_DOMCTL_PFINFO_BROKEN: