| Message ID | 20221117010804.9384-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | XSA-409 fixes | expand |
On 17.11.2022 02:08, Andrew Cooper wrote: > Exercise some basic functionality of the new > xc_{get,set}_paging_mempool_size() hypercalls. > > This passes on x86, but fails currently on ARM. ARM will be fixed up in > future patches. > > This is part of XSA-409 / CVE-2022-33747. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com> (if this counts anything, since as it stands the new stuff all falls under tool stack maintainership) > --- /dev/null > +++ b/tools/tests/paging-mempool/test-paging-mempool.c > @@ -0,0 +1,181 @@ > +#include <err.h> > +#include <errno.h> > +#include <inttypes.h> > +#include <stdio.h> > +#include <string.h> > +#include <sys/mman.h> > + > +#include <xenctrl.h> > +#include <xenforeignmemory.h> > +#include <xengnttab.h> > +#include <xen-tools/libs.h> > + > +static unsigned int nr_failures; > +#define fail(fmt, ...) \ > +({ \ > + nr_failures++; \ > + (void)printf(fmt, ##__VA_ARGS__); \ > +}) > + > +static xc_interface *xch; > +static uint32_t domid; > + > +static struct xen_domctl_createdomain create = { > + .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap, I understand that it is accepted that this test will thus fail when run on HAP-incapable hardware (including when run with Xen itself running on top of another hypervisor not surfacing HAP capabilities)? Oh, I notice you're actually translating EINVAL and EOPNOTSUPP failures into "skip". That'll probably do, albeit personally I consider skipping when EINVAL (which we use all over the place) as a overly relaxed. > +static void run_tests(void) > +{ > + xen_pfn_t physmap[] = { 0 }; I have to admit that I'm uncertain whether Arm (or other architectures that Xen is being planned to be ported to) have constraints which may cause populating of GFN 0 to fail. > + uint64_t size_bytes, old_size_bytes; > + int rc; > + > + printf("Test default mempool size\n"); > + > + rc = xc_get_paging_mempool_size(xch, domid, &size_bytes); > + if ( rc ) > + return fail(" Fail: get mempool size: %d - %s\n", > + errno, strerror(errno)); > + > + printf("mempool size %"PRIu64" bytes (%"PRIu64"kB, %"PRIu64"MB)\n", > + size_bytes, size_bytes >> 10, size_bytes >> 20); > + > + > + /* > + * Check that the domain has the expected default allocation size. This > + * will fail if the logic in Xen is altered without an equivelent Nit: equivalent > + * adjustment here. > + */ > + if ( size_bytes != default_mempool_size_bytes ) > + return fail(" Fail: size %"PRIu64" != expected size %"PRIu64"\n", > + size_bytes, default_mempool_size_bytes); > + > + > + printf("Test that allocate doesn't alter pool size\n"); > + > + /* > + * Populate the domain with some RAM. This will cause more of the mempool > + * to be used. > + */ > + old_size_bytes = size_bytes; > + > + rc = xc_domain_setmaxmem(xch, domid, -1); > + if ( rc ) > + return fail(" Fail: setmaxmem: : %d - %s\n", > + errno, strerror(errno)); > + > + rc = xc_domain_populate_physmap_exact(xch, domid, 1, 0, 0, physmap); > + if ( rc ) > + return fail(" Fail: populate physmap: %d - %s\n", > + errno, strerror(errno)); > + > + /* > + * Re-get the p2m size. Should not have changed as a consequence of > + * populate physmap. > + */ > + rc = xc_get_paging_mempool_size(xch, domid, &size_bytes); > + if ( rc ) > + return fail(" Fail: get mempool size: %d - %s\n", > + errno, strerror(errno)); > + > + if ( old_size_bytes != size_bytes ) > + return fail(" Fail: mempool size changed %"PRIu64" => %"PRIu64"\n", > + old_size_bytes, size_bytes); > + > + > + > + printf("Test bad set size\n"); > + > + /* > + * Check that setting a non-page size results in failure. > + */ > + rc = xc_set_paging_mempool_size(xch, domid, size_bytes + 1); > + if ( rc != -1 || errno != EINVAL ) > + return fail(" Fail: Bad set size: expected -1/EINVAL, got %d/%d - %s\n", > + rc, errno, strerror(errno)); > + > + > + printf("Test very large set size\n"); Maybe drop "very", as 64M isn't all that much (and would, in particular, not expose any 32-bit truncation issues)? Jan
On Thu, Nov 17, 2022 at 01:08:02AM +0000, Andrew Cooper wrote: > Exercise some basic functionality of the new > xc_{get,set}_paging_mempool_size() hypercalls. > > This passes on x86, but fails currently on ARM. ARM will be fixed up in > future patches. > > This is part of XSA-409 / CVE-2022-33747. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Release-acked-by: Henry Wang <Henry.Wang@arm.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
On 17/11/2022 10:39, Jan Beulich wrote: > On 17.11.2022 02:08, Andrew Cooper wrote: >> Exercise some basic functionality of the new >> xc_{get,set}_paging_mempool_size() hypercalls. >> >> This passes on x86, but fails currently on ARM. ARM will be fixed up in >> future patches. >> >> This is part of XSA-409 / CVE-2022-33747. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Release-acked-by: Henry Wang <Henry.Wang@arm.com> > Acked-by: Jan Beulich <jbeulich@suse.com> Thanks. > (if this counts anything, since as it stands the new stuff all falls > under tool stack maintainership) I do intend to give it its own section in due course, but this falls very much into "The Rest" at the moment. >> --- /dev/null >> +++ b/tools/tests/paging-mempool/test-paging-mempool.c >> @@ -0,0 +1,181 @@ >> +#include <err.h> >> +#include <errno.h> >> +#include <inttypes.h> >> +#include <stdio.h> >> +#include <string.h> >> +#include <sys/mman.h> >> + >> +#include <xenctrl.h> >> +#include <xenforeignmemory.h> >> +#include <xengnttab.h> >> +#include <xen-tools/libs.h> >> + >> +static unsigned int nr_failures; >> +#define fail(fmt, ...) \ >> +({ \ >> + nr_failures++; \ >> + (void)printf(fmt, ##__VA_ARGS__); \ >> +}) >> + >> +static xc_interface *xch; >> +static uint32_t domid; >> + >> +static struct xen_domctl_createdomain create = { >> + .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap, > I understand that it is accepted that this test will thus fail when run > on HAP-incapable hardware (including when run with Xen itself running on > top of another hypervisor not surfacing HAP capabilities)? Oh, I notice > you're actually translating EINVAL and EOPNOTSUPP failures into "skip". > That'll probably do, albeit personally I consider skipping when EINVAL > (which we use all over the place) as a overly relaxed. Checking capabilities needs to happen anyway to get PV and HVM Shadow support working. But this will do for now. >> +static void run_tests(void) >> +{ >> + xen_pfn_t physmap[] = { 0 }; > I have to admit that I'm uncertain whether Arm (or other architectures > that Xen is being planned to be ported to) have constraints which may > cause populating of GFN 0 to fail. Mechanically, no. x86 PV is an entirely arbitrary mapping, while all HVM modes (x86 and ARM) are just filling in a guest physical -> "some RAM" mapping in a pagetable structure. Logically, I hope that CDF_directmap on ARM checks for an alias to a real RAM block, but this capability isn't even exposed to the toolstack. And now I've looked at this, ewwww. We've got: d->options which holds config->flags, DOMCTL_CDF_* d->cdf which holds the local 'flags' parameter, CFD_* d->is_privileged which holds 'flags & CDF_privileged' and was never cleaned up when d->cdf was introduced. This is unnecessarily confusing to follow. Yet more for the cleanup pile. >> + uint64_t size_bytes, old_size_bytes; >> + int rc; >> + >> + printf("Test default mempool size\n"); >> + >> + rc = xc_get_paging_mempool_size(xch, domid, &size_bytes); >> + if ( rc ) >> + return fail(" Fail: get mempool size: %d - %s\n", >> + errno, strerror(errno)); >> + >> + printf("mempool size %"PRIu64" bytes (%"PRIu64"kB, %"PRIu64"MB)\n", >> + size_bytes, size_bytes >> 10, size_bytes >> 20); >> + >> + >> + /* >> + * Check that the domain has the expected default allocation size. This >> + * will fail if the logic in Xen is altered without an equivelent > Nit: equivalent Fixed. > >> + * adjustment here. >> + */ >> + if ( size_bytes != default_mempool_size_bytes ) >> + return fail(" Fail: size %"PRIu64" != expected size %"PRIu64"\n", >> + size_bytes, default_mempool_size_bytes); >> + >> + >> + printf("Test that allocate doesn't alter pool size\n"); >> + >> + /* >> + * Populate the domain with some RAM. This will cause more of the mempool >> + * to be used. >> + */ >> + old_size_bytes = size_bytes; >> + >> + rc = xc_domain_setmaxmem(xch, domid, -1); >> + if ( rc ) >> + return fail(" Fail: setmaxmem: : %d - %s\n", >> + errno, strerror(errno)); >> + >> + rc = xc_domain_populate_physmap_exact(xch, domid, 1, 0, 0, physmap); >> + if ( rc ) >> + return fail(" Fail: populate physmap: %d - %s\n", >> + errno, strerror(errno)); >> + >> + /* >> + * Re-get the p2m size. Should not have changed as a consequence of >> + * populate physmap. >> + */ >> + rc = xc_get_paging_mempool_size(xch, domid, &size_bytes); >> + if ( rc ) >> + return fail(" Fail: get mempool size: %d - %s\n", >> + errno, strerror(errno)); >> + >> + if ( old_size_bytes != size_bytes ) >> + return fail(" Fail: mempool size changed %"PRIu64" => %"PRIu64"\n", >> + old_size_bytes, size_bytes); >> + >> + >> + >> + printf("Test bad set size\n"); >> + >> + /* >> + * Check that setting a non-page size results in failure. >> + */ >> + rc = xc_set_paging_mempool_size(xch, domid, size_bytes + 1); >> + if ( rc != -1 || errno != EINVAL ) >> + return fail(" Fail: Bad set size: expected -1/EINVAL, got %d/%d - %s\n", >> + rc, errno, strerror(errno)); >> + >> + >> + printf("Test very large set size\n"); > Maybe drop "very", as 64M isn't all that much (and would, in particular, > not expose any 32-bit truncation issues)? Hmm yeah. That was rather stale. I've switched to "Test set continuation\n" seeing as that's the purpose of the test. 64M is an internal detail which ought to be small enough to work reliably everywhere, but large enough to cause a continuation. ~Andrew
diff --git a/tools/tests/Makefile b/tools/tests/Makefile index d99146d56a64..1319c3a9d88c 100644 --- a/tools/tests/Makefile +++ b/tools/tests/Makefile @@ -11,6 +11,7 @@ endif SUBDIRS-y += xenstore SUBDIRS-y += depriv SUBDIRS-y += vpci +SUBDIRS-y += paging-mempool .PHONY: all clean install distclean uninstall all clean distclean install uninstall: %: subdirs-% diff --git a/tools/tests/paging-mempool/.gitignore b/tools/tests/paging-mempool/.gitignore new file mode 100644 index 000000000000..2f9305b7cc07 --- /dev/null +++ b/tools/tests/paging-mempool/.gitignore @@ -0,0 +1 @@ +test-paging-mempool diff --git a/tools/tests/paging-mempool/Makefile b/tools/tests/paging-mempool/Makefile new file mode 100644 index 000000000000..5d49497710e0 --- /dev/null +++ b/tools/tests/paging-mempool/Makefile @@ -0,0 +1,42 @@ +XEN_ROOT = $(CURDIR)/../../.. +include $(XEN_ROOT)/tools/Rules.mk + +TARGET := test-paging-mempool + +.PHONY: all +all: $(TARGET) + +.PHONY: clean +clean: + $(RM) -- *.o $(TARGET) $(DEPS_RM) + +.PHONY: distclean +distclean: clean + $(RM) -- *~ + +.PHONY: install +install: all + $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN) + $(INSTALL_PROG) $(TARGET) $(DESTDIR)$(LIBEXEC_BIN) + +.PHONY: uninstall +uninstall: + $(RM) -- $(DESTDIR)$(LIBEXEC_BIN)/$(TARGET) + +CFLAGS += $(CFLAGS_xeninclude) +CFLAGS += $(CFLAGS_libxenctrl) +CFLAGS += $(CFLAGS_libxenforeignmemory) +CFLAGS += $(CFLAGS_libxengnttab) +CFLAGS += $(APPEND_CFLAGS) + +LDFLAGS += $(LDLIBS_libxenctrl) +LDFLAGS += $(LDLIBS_libxenforeignmemory) +LDFLAGS += $(LDLIBS_libxengnttab) +LDFLAGS += $(APPEND_LDFLAGS) + +%.o: Makefile + +$(TARGET): test-paging-mempool.o + $(CC) -o $@ $< $(LDFLAGS) + +-include $(DEPS_INCLUDE) diff --git a/tools/tests/paging-mempool/test-paging-mempool.c b/tools/tests/paging-mempool/test-paging-mempool.c new file mode 100644 index 000000000000..942a2fde19c7 --- /dev/null +++ b/tools/tests/paging-mempool/test-paging-mempool.c @@ -0,0 +1,181 @@ +#include <err.h> +#include <errno.h> +#include <inttypes.h> +#include <stdio.h> +#include <string.h> +#include <sys/mman.h> + +#include <xenctrl.h> +#include <xenforeignmemory.h> +#include <xengnttab.h> +#include <xen-tools/libs.h> + +static unsigned int nr_failures; +#define fail(fmt, ...) \ +({ \ + nr_failures++; \ + (void)printf(fmt, ##__VA_ARGS__); \ +}) + +static xc_interface *xch; +static uint32_t domid; + +static struct xen_domctl_createdomain create = { + .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap, + .max_vcpus = 1, + .max_grant_frames = 1, + .grant_opts = XEN_DOMCTL_GRANT_version(1), + + .arch = { +#if defined(__x86_64__) || defined(__i386__) + .emulation_flags = XEN_X86_EMU_LAPIC, +#endif + }, +}; + +static uint64_t default_mempool_size_bytes = +#if defined(__x86_64__) || defined(__i386__) + 256 << 12; /* Only x86 HAP for now. x86 Shadow needs more work. */ +#elif defined (__arm__) || defined(__aarch64__) + 16 << 12; +#endif + +static void run_tests(void) +{ + xen_pfn_t physmap[] = { 0 }; + uint64_t size_bytes, old_size_bytes; + int rc; + + printf("Test default mempool size\n"); + + rc = xc_get_paging_mempool_size(xch, domid, &size_bytes); + if ( rc ) + return fail(" Fail: get mempool size: %d - %s\n", + errno, strerror(errno)); + + printf("mempool size %"PRIu64" bytes (%"PRIu64"kB, %"PRIu64"MB)\n", + size_bytes, size_bytes >> 10, size_bytes >> 20); + + + /* + * Check that the domain has the expected default allocation size. This + * will fail if the logic in Xen is altered without an equivelent + * adjustment here. + */ + if ( size_bytes != default_mempool_size_bytes ) + return fail(" Fail: size %"PRIu64" != expected size %"PRIu64"\n", + size_bytes, default_mempool_size_bytes); + + + printf("Test that allocate doesn't alter pool size\n"); + + /* + * Populate the domain with some RAM. This will cause more of the mempool + * to be used. + */ + old_size_bytes = size_bytes; + + rc = xc_domain_setmaxmem(xch, domid, -1); + if ( rc ) + return fail(" Fail: setmaxmem: : %d - %s\n", + errno, strerror(errno)); + + rc = xc_domain_populate_physmap_exact(xch, domid, 1, 0, 0, physmap); + if ( rc ) + return fail(" Fail: populate physmap: %d - %s\n", + errno, strerror(errno)); + + /* + * Re-get the p2m size. Should not have changed as a consequence of + * populate physmap. + */ + rc = xc_get_paging_mempool_size(xch, domid, &size_bytes); + if ( rc ) + return fail(" Fail: get mempool size: %d - %s\n", + errno, strerror(errno)); + + if ( old_size_bytes != size_bytes ) + return fail(" Fail: mempool size changed %"PRIu64" => %"PRIu64"\n", + old_size_bytes, size_bytes); + + + + printf("Test bad set size\n"); + + /* + * Check that setting a non-page size results in failure. + */ + rc = xc_set_paging_mempool_size(xch, domid, size_bytes + 1); + if ( rc != -1 || errno != EINVAL ) + return fail(" Fail: Bad set size: expected -1/EINVAL, got %d/%d - %s\n", + rc, errno, strerror(errno)); + + + printf("Test very large set size\n"); + + /* + * Check that setting a large P2M size succeeds. This is expecting to + * trigger continuations. + */ + rc = xc_set_paging_mempool_size(xch, domid, 64 << 20); + if ( rc ) + return fail(" Fail: Set size 64MB: %d - %s\n", + errno, strerror(errno)); + + + /* + * Check that the reported size matches what set consumed. + */ + rc = xc_get_paging_mempool_size(xch, domid, &size_bytes); + if ( rc ) + return fail(" Fail: get p2m mempool size: %d - %s\n", + errno, strerror(errno)); + + if ( size_bytes != 64 << 20 ) + return fail(" Fail: expected mempool size %u, got %"PRIu64"\n", + 64 << 20, size_bytes); +} + +int main(int argc, char **argv) +{ + int rc; + + printf("Paging mempool tests\n"); + + xch = xc_interface_open(NULL, NULL, 0); + + if ( !xch ) + err(1, "xc_interface_open"); + + rc = xc_domain_create(xch, &domid, &create); + if ( rc ) + { + if ( errno == EINVAL || errno == EOPNOTSUPP ) + printf(" Skip: %d - %s\n", errno, strerror(errno)); + else + fail(" Domain create failure: %d - %s\n", + errno, strerror(errno)); + goto out; + } + + printf(" Created d%u\n", domid); + + run_tests(); + + rc = xc_domain_destroy(xch, domid); + if ( rc ) + fail(" Failed to destroy domain: %d - %s\n", + errno, strerror(errno)); + out: + return !!nr_failures; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */
Exercise some basic functionality of the new xc_{get,set}_paging_mempool_size() hypercalls. This passes on x86, but fails currently on ARM. ARM will be fixed up in future patches. This is part of XSA-409 / CVE-2022-33747. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien@xen.org> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> CC: Bertrand Marquis <bertrand.marquis@arm.com> CC: Henry Wang <Henry.Wang@arm.com> CC: Anthony PERARD <anthony.perard@citrix.com> x86 Shadow is complicated because of how it behaves for PV guests, and because of how it forms a simultaneous equation with tot_pages. This will require more work to untangle. v2: * s/p2m/paging/ * Fix CFLAGS_libxenforeginmemory typo --- tools/tests/Makefile | 1 + tools/tests/paging-mempool/.gitignore | 1 + tools/tests/paging-mempool/Makefile | 42 ++++++ tools/tests/paging-mempool/test-paging-mempool.c | 181 +++++++++++++++++++++++ 4 files changed, 225 insertions(+) create mode 100644 tools/tests/paging-mempool/.gitignore create mode 100644 tools/tests/paging-mempool/Makefile create mode 100644 tools/tests/paging-mempool/test-paging-mempool.c