On 06/16/2014 09:08 AM, Michal Privoznik wrote:
> The new API is exposed under 'freepages' command.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> tools/virsh-host.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/virsh.pod | 8 +++
> 2 files changed, 175 insertions(+)
>
I'm working on a followup patch to fix several bugs...
> + {.name = "pagesize",
> + .type = VSH_OT_INT,
> + .help = N_("page size (in kibibites)")
s/bites/bytes/
> +static bool
> +cmdFreepages(vshControl *ctl, const vshCmd *cmd)
> +{
> + bool ret = false;
> + unsigned int npages;
> + unsigned int *pagesize = NULL;
> + int cell;
> + unsigned long long *counts = NULL;
> + size_t i, j;
> + xmlNodePtr *nodes = NULL;
> + int nodes_cnt;
pagesize is an int...
> +
> + nodes_cnt = virXPathNodeSet("/capabilities/host/cpu/pages", ctxt, &nodes);
> +
> + if (nodes_cnt <= 0) {
> + vshError(ctl, "%s", _("could not get information about "
> + "supported page sizes"));
> + goto cleanup;
> + }
> +
> + pagesize = vshMalloc(ctl, nodes_cnt * sizeof(*pagesize));
Risks multiplication overflow (probably unlikely in practice, but in
theory a super-large number of /capabilities/host/cpu/pages can
overflow). You're not the first culprit; we've got lots of abuse of
vshMalloc(, a * b) which should instead be using vshCalloc or VIR_ALLOC_N.
> +
> + pagesize = vshMalloc(ctl, sizeof(*pagesize));
...so this allocates only 4 bytes...
> + if (vshCommandOptScaledInt(cmd, "pagesize", (unsigned long long *) pagesize,
> + 1, UINT_MAX) < 0) {
...but this pointer cast causes a store through 8 bytes. Absolute
no-no. Clang caught it, and so will valgrind.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list