[PATCH] tests/pdx: fix overflow from conversion from page index to address on 32bit

Roger Pau Monne posted 1 patch 3 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20260312080206.52648-1-roger.pau@citrix.com
tools/tests/pdx/test-pdx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] tests/pdx: fix overflow from conversion from page index to address on 32bit
Posted by Roger Pau Monne 3 weeks, 5 days ago
When building the PDX test harness as a 32bit executable the page shifts
done on unsigned long types can overflow.  Instead use pfn_to_paddr(),
which casts the values to paddr_t previous to doing the shift.

Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
Reported-by: Edwin Török <edwin.torok@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/tests/pdx/test-pdx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/tests/pdx/test-pdx.c b/tools/tests/pdx/test-pdx.c
index eefd54c76815..066bd3e7e569 100644
--- a/tools/tests/pdx/test-pdx.c
+++ b/tools/tests/pdx/test-pdx.c
@@ -208,8 +208,8 @@ int main(int argc, char **argv)
             if ( !tests[i].ranges[j].start && !tests[i].ranges[j].end )
                 break;
 
-            pfn_pdx_add_region(tests[i].ranges[j].start << PAGE_SHIFT,
-                               size << PAGE_SHIFT);
+            pfn_pdx_add_region(pfn_to_paddr(tests[i].ranges[j].start),
+                               pfn_to_paddr(size));
         }
 
         if ( pfn_pdx_compression_setup(0) != tests[i].compress )
@@ -233,8 +233,8 @@ int main(int argc, char **argv)
             if ( !start && !end )
                 break;
 
-            if ( !pdx_is_region_compressible(start << PAGE_SHIFT, 1) ||
-                 !pdx_is_region_compressible((end - 1) << PAGE_SHIFT, 1) )
+            if ( !pdx_is_region_compressible(pfn_to_paddr(start), 1) ||
+                 !pdx_is_region_compressible(pfn_to_paddr(end - 1), 1) )
             {
                 printf(
     "PFN compression invalid, pages %#lx and %#lx should be compressible\n",
-- 
2.51.0


Re: [PATCH] tests/pdx: fix overflow from conversion from page index to address on 32bit
Posted by Jan Beulich 3 weeks, 5 days ago
On 12.03.2026 09:02, Roger Pau Monne wrote:
> When building the PDX test harness as a 32bit executable the page shifts
> done on unsigned long types can overflow.  Instead use pfn_to_paddr(),
> which casts the values to paddr_t previous to doing the shift.
> 
> Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
> Reported-by: Edwin Török <edwin.torok@citrix.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> --- a/tools/tests/pdx/test-pdx.c
> +++ b/tools/tests/pdx/test-pdx.c
> @@ -208,8 +208,8 @@ int main(int argc, char **argv)
>              if ( !tests[i].ranges[j].start && !tests[i].ranges[j].end )
>                  break;
>  
> -            pfn_pdx_add_region(tests[i].ranges[j].start << PAGE_SHIFT,
> -                               size << PAGE_SHIFT);
> +            pfn_pdx_add_region(pfn_to_paddr(tests[i].ranges[j].start),
> +                               pfn_to_paddr(size));
>          }
>  
>          if ( pfn_pdx_compression_setup(0) != tests[i].compress )
> @@ -233,8 +233,8 @@ int main(int argc, char **argv)
>              if ( !start && !end )
>                  break;
>  
> -            if ( !pdx_is_region_compressible(start << PAGE_SHIFT, 1) ||
> -                 !pdx_is_region_compressible((end - 1) << PAGE_SHIFT, 1) )
> +            if ( !pdx_is_region_compressible(pfn_to_paddr(start), 1) ||
> +                 !pdx_is_region_compressible(pfn_to_paddr(end - 1), 1) )
>              {
>                  printf(
>      "PFN compression invalid, pages %#lx and %#lx should be compressible\n",

Largely unrelated remark, from going through all of the PAGE_SHIFT uses: Isn't
the __LP64__ conditional excluding quite a few too many array elements in
main()'s tests[]?

Jan

Re: [PATCH] tests/pdx: fix overflow from conversion from page index to address on 32bit
Posted by Roger Pau Monné 3 weeks, 5 days ago
On Thu, Mar 12, 2026 at 09:25:12AM +0100, Jan Beulich wrote:
> On 12.03.2026 09:02, Roger Pau Monne wrote:
> > When building the PDX test harness as a 32bit executable the page shifts
> > done on unsigned long types can overflow.  Instead use pfn_to_paddr(),
> > which casts the values to paddr_t previous to doing the shift.
> > 
> > Fixes: cb50e4033717 ("test/pdx: add PDX compression unit tests")
> > Reported-by: Edwin Török <edwin.torok@citrix.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> > --- a/tools/tests/pdx/test-pdx.c
> > +++ b/tools/tests/pdx/test-pdx.c
> > @@ -208,8 +208,8 @@ int main(int argc, char **argv)
> >              if ( !tests[i].ranges[j].start && !tests[i].ranges[j].end )
> >                  break;
> >  
> > -            pfn_pdx_add_region(tests[i].ranges[j].start << PAGE_SHIFT,
> > -                               size << PAGE_SHIFT);
> > +            pfn_pdx_add_region(pfn_to_paddr(tests[i].ranges[j].start),
> > +                               pfn_to_paddr(size));
> >          }
> >  
> >          if ( pfn_pdx_compression_setup(0) != tests[i].compress )
> > @@ -233,8 +233,8 @@ int main(int argc, char **argv)
> >              if ( !start && !end )
> >                  break;
> >  
> > -            if ( !pdx_is_region_compressible(start << PAGE_SHIFT, 1) ||
> > -                 !pdx_is_region_compressible((end - 1) << PAGE_SHIFT, 1) )
> > +            if ( !pdx_is_region_compressible(pfn_to_paddr(start), 1) ||
> > +                 !pdx_is_region_compressible(pfn_to_paddr(end - 1), 1) )
> >              {
> >                  printf(
> >      "PFN compression invalid, pages %#lx and %#lx should be compressible\n",
> 
> Largely unrelated remark, from going through all of the PAGE_SHIFT uses: Isn't
> the __LP64__ conditional excluding quite a few too many array elements in
> main()'s tests[]?

Hm, I think so.  Will send a separate fix for that however.

Thanks, Roger.