On 18/12/2019 11:06, Jan Beulich wrote: > On 17.12.2019 16:46, Sergey Dyasli wrote: >> Hide the following information that can help identify the running Xen >> binary version: >> >> XENVER_extraversion >> XENVER_compile_info >> XENVER_capabilities > > What's wrong with exposing this one?
extraversion can help identify the exact running Xen binary (and I must say that at Citrix we add some additional version information there). compile_info will identify compiler and many speculative mitigations are provided by compilers these days. Not sure if it's worth hiding capabilities, but OTOH I don't see how guests could meaningfully use it. > >> XENVER_changeset >> XENVER_commandline >> XENVER_build_id >> >> Return a more customer friendly empty string instead of "<denied>" >> which would be shown in tools like dmidecode.> > I think "<denied>" is quite fine for many of the original purposes. > Maybe it would be better to filter for this when populating guest > DMI tables? I don't know how DMI tables are populated, but nothing stops a guest from using these hypercalls directly. > >> But allow guests to see this information in Debug builds of Xen. > > Behavior like this would imo better not differ between debug and > release builds, or else guest software may behave entirely > differently once you put it on a production build. I agree on this one, it's not much worth providing this information in debug builds. > >> --- a/xen/include/xsm/dummy.h >> +++ b/xen/include/xsm/dummy.h >> @@ -750,16 +750,21 @@ static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG >> uint32_t op) >> case XENVER_get_features: >> /* These sub-ops ignore the permission checks and return data. */ >> return 0; >> - case XENVER_extraversion: >> - case XENVER_compile_info: >> - case XENVER_capabilities: >> - case XENVER_changeset: >> case XENVER_pagesize: >> case XENVER_guest_handle: >> /* These MUST always be accessible to any guest by default. */ > > This comment, not the least because of its use of capitals, > suggests to me that there's further justification needed for > your change, including discussion of why there's no risk of > breaking existing guests. Not sure about this comment, maybe the author (Konrad) remembers? We had this change in production for very long time now and haven't seen any guest regressions. > >> return xsm_default_action(XSM_HOOK, current->domain, NULL); >> + >> + case XENVER_extraversion: >> + case XENVER_compile_info: >> + case XENVER_capabilities: >> + case XENVER_changeset: >> + case XENVER_commandline: >> + case XENVER_build_id: >> default: > > There's no need to add all of these next to the default case. > Note how commandline and build_id have been coming here already > (and there would need to be further justification for exposing > them on debug builds, should this questionable behavior - see > above - be retained). I find that explicitly listing all the cases is better for code readability, but I don't have a strong opinion here. -- Thanks, Sergey _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
