The error message accidentally printed the bytes value as if it were kB.

Furthermore, both b_info.shadow_memkb and shadow_mem are uint64_t, meaning
there is a risk of overflow if the user specified a stupidly large value in
the vm.cfg file.  Check and reject such a condition.

Fixes: 7c3bbd940dd8 ("xen/arm, libxl: Revert XEN_DOMCTL_shadow_op; use p2m mempool hypercalls")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Henry Wang <Henry.Wang@arm.com>

v2:
 * Retain PRIu64
 * Check for overflow

For 4.17.  This is a low risk change, removes one overflow case, and makes an
error message accurate.
---
 tools/libs/light/libxl_dom.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index fa5c79e4f650..b454f988fbc5 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -1458,10 +1458,18 @@ int libxl__domain_set_paging_mempool_size(
     shadow_mem = d_config->b_info.shadow_memkb;
     shadow_mem <<= 10;
 
+    if ((shadow_mem >> 10) != d_config->b_info.shadow_memkb) {
+        LOGED(ERROR, domid,
+              "shadow_memkb value %"PRIu64"kB too large",
+              d_config->b_info.shadow_memkb);
+        return ERROR_FAIL;
+    }
+
     int r = xc_set_paging_mempool_size(CTX->xch, domid, shadow_mem);
     if (r) {
         LOGED(ERROR, domid,
-              "Failed to set paging mempool size to %"PRIu64"kB", shadow_mem);
+              "Failed to set paging mempool size to %"PRIu64"kB",
+              d_config->b_info.shadow_memkb);
         return ERROR_FAIL;
     }
 
-- 
2.11.0

On Mon, Nov 21, 2022 at 02:37:30PM +0000, Andrew Cooper wrote:
> The error message accidentally printed the bytes value as if it were kB.
> 
> Furthermore, both b_info.shadow_memkb and shadow_mem are uint64_t, meaning
> there is a risk of overflow if the user specified a stupidly large value in
> the vm.cfg file.  Check and reject such a condition.
> 
> Fixes: 7c3bbd940dd8 ("xen/arm, libxl: Revert XEN_DOMCTL_shadow_op; use p2m mempool hypercalls")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> 
> v2:
>  * Retain PRIu64
>  * Check for overflow
> 
> For 4.17.  This is a low risk change, removes one overflow case, and makes an
> error message accurate.

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

Hi Andrew,

> -----Original Message-----
> Subject: Re: [PATCH 1/2] tools/libxl: Fixes to
> libxl__domain_set_paging_mempool_size()
> 
> On Mon, Nov 21, 2022 at 02:37:30PM +0000, Andrew Cooper wrote:
> > The error message accidentally printed the bytes value as if it were kB.
> >
> > Furthermore, both b_info.shadow_memkb and shadow_mem are uint64_t,
> meaning
> > there is a risk of overflow if the user specified a stupidly large value in
> > the vm.cfg file.  Check and reject such a condition.
> >
> > Fixes: 7c3bbd940dd8 ("xen/arm, libxl: Revert XEN_DOMCTL_shadow_op;
> use p2m mempool hypercalls")
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> >
> > v2:
> >  * Retain PRIu64
> >  * Check for overflow
> >
> > For 4.17.  This is a low risk change, removes one overflow case, and makes
> an
> > error message accurate.
> 
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Release-acked-by: Henry Wang <Henry.Wang@arm.com>

Kind regards,
Henry