| Message ID | 20170814070849.20986-49-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/14/2017 03:08 AM, Juergen Gross wrote: > Add a sysctl hypercall to support setting parameters similar to > command line parameters, but at runtime. The parameters to set are > specified as a string, just like the boot parameters. Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote: > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -1096,6 +1096,23 @@ 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. > + */ > + > +#define XEN_SET_PARAMETER_MAX_SIZE 1023 Does this really need to be in the public interface, i.e. isn't this limit an implementation detail? > +struct xen_sysctl_set_parameter { > + XEN_GUEST_HANDLE_64(char) params; /* IN: pointer to parameters. */ > + uint16_t size; /* IN: size of parameters. Max. > + XEN_SET_PARAMETER_MAX_SIZE. */ You could even allow querying the size by passing in a null handle and returning the value in the size field. > + uint16_t pad[3]; /* IN: MUST be zero. */ But you don't check the field is zero afaics. Jan
On 15/08/17 17:39, Jan Beulich wrote: >>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote: >> --- a/xen/include/public/sysctl.h >> +++ b/xen/include/public/sysctl.h >> @@ -1096,6 +1096,23 @@ 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. >> + */ >> + >> +#define XEN_SET_PARAMETER_MAX_SIZE 1023 > > Does this really need to be in the public interface, i.e. isn't this limit > an implementation detail? Hmm, yes. > >> +struct xen_sysctl_set_parameter { >> + XEN_GUEST_HANDLE_64(char) params; /* IN: pointer to parameters. */ >> + uint16_t size; /* IN: size of parameters. Max. >> + XEN_SET_PARAMETER_MAX_SIZE. */ > > You could even allow querying the size by passing in a null handle > and returning the value in the size field. Would this help in any way? Maybe just returning E2BIG would be enough? > >> + uint16_t pad[3]; /* IN: MUST be zero. */ > > But you don't check the field is zero afaics. Right, I should do this. Juergen
>>> On 15.08.17 at 17:57, <jgross@suse.com> wrote: > On 15/08/17 17:39, Jan Beulich wrote: >>>>> On 14.08.17 at 09:08, <jgross@suse.com> wrote: >>> +struct xen_sysctl_set_parameter { >>> + XEN_GUEST_HANDLE_64(char) params; /* IN: pointer to parameters. */ >>> + uint16_t size; /* IN: size of parameters. Max. >>> + XEN_SET_PARAMETER_MAX_SIZE. */ >> >> You could even allow querying the size by passing in a null handle >> and returning the value in the size field. > > Would this help in any way? Since the caller won't want to dimension the buffer dynamically, perhaps not much. The only use I could see would be to give a better error message for too long strings than the one resulting from strerror(). I.e. ... > Maybe just returning E2BIG would be enough? ... maybe yes. Jan
diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te index d0a4d91ac0..338caaf41e 100644 --- a/tools/flask/policy/modules/dom0.te +++ b/tools/flask/policy/modules/dom0.te @@ -16,7 +16,7 @@ allow dom0_t xen_t:xen { allow dom0_t xen_t:xen2 { resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol get_cpu_levelling_caps get_cpu_featureset livepatch_op - gcov_op + gcov_op set_parameter }; # Allow dom0 to use all XENVER_ subops that have checks. diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c index ae58a0f650..a3237fe9be 100644 --- a/xen/common/sysctl.c +++ b/xen/common/sysctl.c @@ -467,6 +467,35 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) copyback = 1; break; + case XEN_SYSCTL_set_parameter: + { + char *params; + + if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE ) + { + ret = -EINVAL; + 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) ) + ret = -EFAULT; + else + { + params[op->u.set_parameter.size] = 0; + ret = runtime_parse(params); + } + + xfree(params); + + break; + } + default: ret = arch_do_sysctl(op, u_sysctl); copyback = 0; diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index 9e51af61e1..43b18bdb9b 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -1096,6 +1096,23 @@ 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. + */ + +#define XEN_SET_PARAMETER_MAX_SIZE 1023 +struct xen_sysctl_set_parameter { + XEN_GUEST_HANDLE_64(char) params; /* IN: pointer to parameters. */ + uint16_t size; /* IN: size of parameters. Max. + XEN_SET_PARAMETER_MAX_SIZE. */ + uint16_t pad[3]; /* IN: MUST be zero. */ +}; +typedef struct xen_sysctl_set_parameter xen_sysctl_set_parameter_t; +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_set_parameter_t); + struct xen_sysctl { uint32_t cmd; #define XEN_SYSCTL_readconsole 1 @@ -1124,6 +1141,7 @@ struct xen_sysctl { #define XEN_SYSCTL_get_cpu_levelling_caps 25 #define XEN_SYSCTL_get_cpu_featureset 26 #define XEN_SYSCTL_livepatch_op 27 +#define XEN_SYSCTL_set_parameter 28 uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */ union { struct xen_sysctl_readconsole readconsole; @@ -1152,6 +1170,7 @@ struct xen_sysctl { struct xen_sysctl_cpu_levelling_caps cpu_levelling_caps; struct xen_sysctl_cpu_featureset cpu_featureset; struct xen_sysctl_livepatch_op livepatch; + struct xen_sysctl_set_parameter set_parameter; uint8_t pad[128]; } u; }; diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index fd84ac0f09..c9c275bf3b 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -825,6 +825,9 @@ static int flask_sysctl(int cmd) case XEN_SYSCTL_gcov_op: return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2, XEN2__GCOV_OP, NULL); + case XEN_SYSCTL_set_parameter: + return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2, + XEN2__SET_PARAMETER, NULL); default: return avc_unknown_permission("sysctl", cmd); diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors index 1f7eb35fc8..b80fca1ec0 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -101,6 +101,8 @@ class xen2 livepatch_op # XEN_SYSCTL_gcov_op gcov_op +# XEN_SYSCTL_set_parameter + set_parameter } # Classes domain and domain2 consist of operations that a domain performs on
Add a sysctl hypercall to support setting parameters similar to command line parameters, but at runtime. The parameters to set are specified as a string, just like the boot parameters. Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - corrected XSM test (Daniel De Graaf) --- tools/flask/policy/modules/dom0.te | 2 +- xen/common/sysctl.c | 29 +++++++++++++++++++++++++++++ xen/include/public/sysctl.h | 19 +++++++++++++++++++ xen/xsm/flask/hooks.c | 3 +++ xen/xsm/flask/policy/access_vectors | 2 ++ 5 files changed, 54 insertions(+), 1 deletion(-)