|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [livepatch: independ. modules 1/3] livepatch: Always check hypervisor build ID upon hotpatch upload
On Tue, Apr 16, 2019 at 12:58:30PM +0000, Pawel Wieczorkiewicz wrote:
> This change is part of a independant stacked hotpatch modules
> feature. This feature allows to bypass dependencies between modules
> upon loading, but still verifies Xen build ID matching.
OK, so build_id_dep would not be called for those?
I see patch #2 doing this. Cool.
>
> In order to prevent (up)loading any hotpatches built for different
> hypervisor version as indicated by the Xen Build ID, add checking for
> the payload's vs Xen's build id match.
>
> To achieve that embed into every hotpatch another section with a
> dedicated hypervisor build id in it. After the payload is loaded and
> the .livepatch.xen_depends section becomes available, perform the
> check and reject the payload if there is no match.
>
> Signed-off-by: Pawel Wieczorkiewicz <wipawel@xxxxxxxxx>
> Reviewed-by: Andra-Irina Paraschiv <andraprs@xxxxxxxxxx>
> Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxx>
> Reviewed-by: Eslam Elnikety <elnikety@xxxxxxxxx>
> Reviewed-by: Martin Pohlack <mpohlack@xxxxxxxxx>
> ---
> xen/common/livepatch.c | 47
> +++++++++++++++++++++++++++++++++++++++++++++
> xen/include/xen/livepatch.h | 7 ++++---
Can you please include:
- Changes to the docs/misc/livepatch.markdown describing this change.
- Include a test-case exercising this.
- Fix the test-cases as I think they will now all fail? (As they don't have
this section?)
Thank you.
> 2 files changed, 51 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index d6eaae6d3b..6a4af6ce57 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -74,6 +74,7 @@ struct payload {
> unsigned int nsyms; /* Nr of entries in .strtab and
> symbols. */
> struct livepatch_build_id id; /* ELFNOTE_DESC(.note.gnu.build-id)
> of the payload. */
> struct livepatch_build_id dep; /*
> ELFNOTE_DESC(.livepatch.depends). */
> + struct livepatch_build_id xen_dep; /*
> ELFNOTE_DESC(.livepatch.xen_depends). */
> livepatch_loadcall_t *const *load_funcs; /* The array of funcs to call
> after */
> livepatch_unloadcall_t *const *unload_funcs;/* load and unload of the
> payload. */
> unsigned int n_load_funcs; /* Nr of the funcs to load and
> execute. */
> @@ -476,11 +477,34 @@ static bool section_ok(const struct livepatch_elf *elf,
> return true;
> }
>
> +static int check_xen_build_id(const struct payload *payload)
> +{
> + const void *id = NULL;
> + unsigned int len = 0;
> + int rc;
> +
> + ASSERT(payload->xen_dep.len);
> + ASSERT(payload->xen_dep.p);
> +
> + rc = xen_build_id(&id, &len);
> + if ( rc )
> + return rc;
> +
> + if ( payload->xen_dep.len != len || memcmp(id, payload->xen_dep.p, len)
> ) {
> + dprintk(XENLOG_ERR, "%s%s: check against hypervisor build-id
> failed!\n",
> + LIVEPATCH, payload->name);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int check_special_sections(const struct livepatch_elf *elf)
> {
> unsigned int i;
> static const char *const names[] = { ELF_LIVEPATCH_FUNC,
> ELF_LIVEPATCH_DEPENDS,
> + ELF_LIVEPATCH_XEN_DEPENDS,
> ELF_BUILD_ID_NOTE};
> DECLARE_BITMAP(found, ARRAY_SIZE(names)) = { 0 };
>
> @@ -632,6 +656,22 @@ static int prepare_payload(struct payload *payload,
> return -EINVAL;
> }
>
> + sec = livepatch_elf_sec_by_name(elf, ELF_LIVEPATCH_XEN_DEPENDS);
> + if ( sec )
> + {
> + n = sec->load_addr;
> +
> + if ( sec->sec->sh_size <= sizeof(*n) )
> + return -EINVAL;
> +
> + if ( xen_build_id_check(n, sec->sec->sh_size,
> + &payload->xen_dep.p, &payload->xen_dep.len) )
> + return -EINVAL;
> +
> + if ( !payload->xen_dep.len || !payload->xen_dep.p )
> + return -EINVAL;
> + }
> +
> /* Setup the virtual region with proper data. */
> region = &payload->region;
>
> @@ -882,6 +922,10 @@ static int load_payload_data(struct payload *payload,
> void *raw, size_t len)
> if ( rc )
> goto out;
>
> + rc = check_xen_build_id(payload);
> + if ( rc )
> + goto out;
> +
> rc = build_symbol_table(payload, &elf);
> if ( rc )
> goto out;
> @@ -1655,6 +1699,9 @@ static void livepatch_printall(unsigned char key)
>
> if ( data->dep.len )
> printk("depend-on=%*phN\n", data->dep.len, data->dep.p);
> +
> + if ( data->xen_dep.len )
> + printk("depend-on-xen=%*phN\n", data->xen_dep.len,
> data->xen_dep.p);
> }
>
> spin_unlock(&payload_lock);
> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
> index 1b1817ca0d..ed997aa4cc 100644
> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -29,9 +29,10 @@ struct xen_sysctl_livepatch_op;
> /* Convenience define for printk. */
> #define LIVEPATCH "livepatch: "
> /* ELF payload special section names. */
> -#define ELF_LIVEPATCH_FUNC ".livepatch.funcs"
> -#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends"
> -#define ELF_BUILD_ID_NOTE ".note.gnu.build-id"
> +#define ELF_LIVEPATCH_FUNC ".livepatch.funcs"
> +#define ELF_LIVEPATCH_DEPENDS ".livepatch.depends"
> +#define ELF_LIVEPATCH_XEN_DEPENDS ".livepatch.xen_depends"
> +#define ELF_BUILD_ID_NOTE ".note.gnu.build-id"
> /* Arbitrary limit for payload size and .bss section size. */
> #define LIVEPATCH_MAX_SIZE MB(2)
>
> --
> 2.16.5
>
>
>
>
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
> Ust-ID: DE 289 237 879
> Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |