| Message ID | 20221121143731.27545-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Even more XSA-409 fixes | expand |
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,
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
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; }
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(-)