Re: [Xen-devel] [PATCH 1/2] xen: add interface for obtaining .config from hypervisor

[Juergen Gross <jgross@xxxxxxxx>]

On 17/01/2019 16:12, Wei Liu wrote:
> On Thu, Jan 17, 2019 at 03:57:21PM +0100, Juergen Gross wrote:
>> Add a sysctl interface for obtaining the .config file used to build
>> the hypervisor. The mechanism is inspired by the Linux kernel's one.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> ---
>>  .gitignore                          |  2 ++
>>  tools/flask/policy/modules/dom0.te  |  2 +-
>>  xen/common/Makefile                 |  7 +++++++
>>  xen/common/sysctl.c                 | 13 +++++++++++++
>>  xen/include/public/sysctl.h         | 16 ++++++++++++++++
>>  xen/include/xen/kernel.h            |  3 +++
>>  xen/tools/Makefile                  |  9 +++++++--
>>  xen/tools/bin2c.c                   | 28 ++++++++++++++++++++++++++++
>>  xen/xsm/flask/hooks.c               |  3 +++
>>  xen/xsm/flask/policy/access_vectors |  2 ++
>>  10 files changed, 82 insertions(+), 3 deletions(-)
>>  create mode 100644 xen/tools/bin2c.c
>>
>> diff --git a/.gitignore b/.gitignore
>> index 26bc583f74..549b57020f 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -309,6 +309,7 @@ xen/arch/*/efi/boot.c
>>  xen/arch/*/efi/compat.c
>>  xen/arch/*/efi/efi.h
>>  xen/arch/*/efi/runtime.c
>> +xen/common/config_data.c
>>  xen/include/headers*.chk
>>  xen/include/asm
>>  xen/include/asm-*/asm-offsets.h
>> @@ -328,6 +329,7 @@ xen/test/livepatch/xen_nop.livepatch
>>  xen/test/livepatch/xen_replace_world.livepatch
>>  xen/tools/kconfig/.tmp_gtkcheck
>>  xen/tools/kconfig/.tmp_qtcheck
>> +xen/tools/bin2c
> 
> Move this ahead before kconfig?

Sorry, of course!

> 
>>  xen/tools/symbols
>>  xen/xsm/flask/include/av_perm_to_string.h
>>  xen/xsm/flask/include/av_permissions.h
>> diff --git a/tools/flask/policy/modules/dom0.te 
>> b/tools/flask/policy/modules/dom0.te
>> index a347d664f8..b776e9f307 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_alloc pmu_ctrl get_symbol
>>      get_cpu_levelling_caps get_cpu_featureset livepatch_op
>> -    coverage_op set_parameter
>> +    coverage_op set_parameter get_config
>>  };
>>  
>>  # Allow dom0 to use all XENVER_ subops that have checks.
>> diff --git a/xen/common/Makefile b/xen/common/Makefile
>> index 56fc201b6b..b375a49ed7 100644
>> --- a/xen/common/Makefile
>> +++ b/xen/common/Makefile
>> @@ -1,5 +1,6 @@
>>  obj-y += bitmap.o
>>  obj-y += bsearch.o
>> +obj-y += config_data.o
>>  obj-$(CONFIG_CORE_PARKING) += core_parking.o
>>  obj-y += cpu.o
>>  obj-y += cpupool.o
>> @@ -83,3 +84,9 @@ subdir-$(CONFIG_UBSAN) += ubsan
>>  
>>  subdir-$(CONFIG_NEEDS_LIBELF) += libelf
>>  subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
>> +
>> +config_data.c: ../.config
>> +    ( echo "const char xen_config_data[] ="; \
>> +      cat $< | gzip | ../tools/bin2c; \
>> +      echo ";"; \
>> +      echo "unsigned int xen_config_data_sz = sizeof(xen_config_data) - 1;" 
>> ) > $@
>> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
>> index c0aa6bde4e..6b6608f67b 100644
>> --- a/xen/common/sysctl.c
>> +++ b/xen/common/sysctl.c
>> @@ -13,6 +13,7 @@
>>  #include <xen/domain.h>
>>  #include <xen/event.h>
>>  #include <xen/domain_page.h>
>> +#include <xen/kernel.h>
>>  #include <xen/tmem.h>
>>  #include <xen/trace.h>
>>  #include <xen/console.h>
>> @@ -502,6 +503,18 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
>> u_sysctl)
>>          break;
>>      }
>>  
>> +    case XEN_SYSCTL_get_config:
>> +    {
>> +        unsigned int size = min(op->u.get_config.size, xen_config_data_sz);
>> +
>> +        if ( size &&
>> +             copy_to_guest(op->u.get_config.buffer, xen_config_data, size) )
>> +            ret = -EFAULT;
> 
> What's the point of copying when user supplied buffer is not big enough?
> They can't continue from where they left off anyway.

Hmm, true. Will change.

> 
>> +        op->u.get_config.size = xen_config_data_sz;
>> +
>> +        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 c49b4dcc99..fb5d93a242 100644
>> --- a/xen/include/public/sysctl.h
>> +++ b/xen/include/public/sysctl.h
>> @@ -1100,6 +1100,20 @@ typedef struct xen_sysctl_cpu_policy 
>> xen_sysctl_cpu_policy_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpu_policy_t);
>>  #endif
>>  
>> +/*
>> + * XEN_SYSCTL_get_config
>> + *
>> + * Return gzip-ed .config file
>> + */
>> +struct xen_sysctl_get_config {
>> +    XEN_GUEST_HANDLE_64(char) buffer;   /* IN: pointer to buffer. */
>> +    uint32_t size;                      /* IN: size of buffer. */
>> +                                        /* OUT: size of config data. */
>> +    uint32_t pad;                       /* IN: MUST be zero. */
> 
> Please check pad is really zero in code.

Yes.


Juergen


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