Re: [Xen-devel] [PATCH v3 08/11] libxl: Kill QEMU by uid when possible

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

George Dunlap writes ("[PATCH v3 08/11] libxl: Kill QEMU by uid when possible"):
> The privcmd fd that a dm_restrict'ed QEMU has gives it permission to
> one specific domain ID.  This domain ID will probably eventually be
> used again.  It is therefore necessary to make absolutely sure that a
> rogue QEMU process cannot hang around after its domain has exited.
...
> +static int kill_device_model_uid_child(libxl__destroy_devicemodel_state 
> *ddms,

> +    /*
> +     * And kill everyone but me.
> +     *
> +     * NB that it's not clear from either POSIX or the Linux man page
> +     * that ESRCH would be returned with a pid value of -1, but it
> +     * doesn't hurt to check.
> +     */
> +    r = kill(-1, 9);
> +    if (r && errno != ESRCH) {
> +        LOGED(ERROR, domid, "kill(-1,9)");
> +        rc = ERROR_FAIL;
> +    }

Missing `goto out', there.

> +
> +    rc = 0;
> +
> +out:
> +    return rc;
> +}

> +static void kill_device_model_uid_cb(libxl__egc *egc,
> +                                    libxl__ev_child *destroyer,
> +                                    pid_t pid, int status)
> +{
> +    libxl__destroy_devicemodel_state *ddms = CONTAINER_OF(destroyer, *ddms, 
> destroyer);
> +    STATE_AO_GC(ddms->ao);
> +
> +    if (status) {
> +        int rc = ERROR_FAIL;
> +
> +        if (WIFEXITED(status))
> +            rc = -WEXITSTATUS(status);

But WEXITSTATUS might be something weird.  See my mail about possible
system-invented exit statuses.  I suggest you tolerate only the status
values you intend to generate, 1..125.  (And 0 for success.)

> +/* 
> + * A macro to help retain the first failure in "do as much as you can"
> + * situations.  Note the hard-coded use of the variable name `rc`.
> + */
> +#define ACCUMULATE_RC(rc_acc) ((rc_acc) = (rc_acc) ?: rc)
> +    

Good, although you have trailing whitespace in the new blank line.


Everything else LGTM.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel