Re: [Xen-devel] [PATCH v3 48/52] xen: add hypercall for setting parameters at runtime

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.17 at 14:52, <jgross@xxxxxxxx> wrote:
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -467,6 +467,42 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
> u_sysctl)
>              copyback = 1;
>          break;
>  
> +    case XEN_SYSCTL_set_parameter:
> +    {
> +#define XEN_SET_PARAMETER_MAX_SIZE 1023
> +        char *params;
> +
> +        if ( op->u.set_parameter.pad[0] || op->u.set_parameter.pad[1] ||
> +             op->u.set_parameter.pad[2] )
> +        {
> +            ret = -EINVAL;
> +            break;
> +        }
> +        if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
> +        {
> +            ret = -E2BIG;
> +            break;
> +        }
> +        params = xmalloc_bytes(op->u.set_parameter.size + 1);
> +        if ( !params )
> +        {
> +            ret = -ENOMEM;
> +            break;
> +        }
> +        if ( __copy_from_guest(params, op->u.set_parameter.params,
> +                               op->u.set_parameter.size) )

You didn't verify the handle earlier, so I think this needs to be
copy_from_guest().

> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1096,6 +1096,21 @@ struct xen_sysctl_livepatch_op {
>  typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
>  
> +/*
> + * XEN_SYSCTL_set_parameter
> + *
> + * Change hypervisor parameters at runtime.
> + * The input string is parsed similar to the boot parameters.
> + */
> +
> +struct xen_sysctl_set_parameter {
> +    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
> +    uint16_t size;                          /* IN: size of parameters. */

The combination of length and zero terminator is always a little
ambiguous: I think it should be clarified in the comment what
behavior to expect, unless you want to either disallow
embedded NULs or drop the size field.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel