Message ID | 1458849640-22588-28-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
>>> On 24.03.16 at 21:00, <konrad.wilk@oracle.com> wrote: > Shadow variables are a piece of infrastructure to be used by xsplice > modules. They are used to attach a new piece of data to an existing > structure in memory. An already known question again: Out of recent XSAs, how many needed such? I.e. can#t we put this off for now? > Martin Pohlack also mentions that using the SHADOW_SLOTS of small > prime numbers has the advantage of having a simple hash function. This reads kind of backwards. > +void xsplice_shadow_free(const void *obj, const char *var) > +{ > + struct shadow_var *entry, *shadow = NULL; > + unsigned int slot; > + struct hlist_node *next; > + > + slot = (unsigned long)obj % SHADOW_SLOTS; > + > + spin_lock(&shadow_lock); > + hlist_for_each_entry(entry, next, &shadow_tbl[slot], list) > + { > + if ( entry->obj == obj && > + !strcmp(entry->var, var) ) > + { > + shadow = entry; > + break; > + } > + } > + if (shadow) Coding style. > + { > + hlist_del(&shadow->list); > + xfree(shadow->data); > + xfree(shadow); > + } > + spin_unlock(&shadow_lock); > +} Uniqueness not being guaranteed by the allocation function, how can you free the first hit here ... > +void *xsplice_shadow_get(const void *obj, const char *var) > +{ > + struct shadow_var *entry; > + unsigned int slot; > + struct hlist_node *next; > + void *ret = NULL; > + > + slot = (unsigned long)obj % SHADOW_SLOTS; > + > + spin_lock(&shadow_lock); > + hlist_for_each_entry(entry, next, &shadow_tbl[slot], list) > + { > + if ( entry->obj == obj && > + !strcmp(entry->var, var) ) > + { > + ret = entry->data; > + break; > + } > + } > + > + spin_unlock(&shadow_lock); > + return ret; > +} .. or return the first match here? > +static int __init xsplice_shadow_init(void) > +{ > + int i; unsigned int > --- a/xen/include/xen/xsplice_patch.h > +++ b/xen/include/xen/xsplice_patch.h > @@ -56,4 +56,40 @@ typedef void (*xsplice_unloadcall_t)(void); > #define XSPLICE_UNLOAD_HOOK(_fn) \ > xsplice_unloadcall_t __attribute__((weak)) xsplice_unload_data > __section(".xsplice.hooks.unload") = _fn; > > + > +/* > + * The following definitions are to be used in patches. They are taken > + * from kpatch. > + */ > + > +/* > + * xsplice shadow variables > + * > + * These functions can be used to add new "shadow" fields to existing data > + * structures. For example, to allocate a "newpid" variable associated with an > + * instance of task_struct, and assign it a value of 1000: > + * > + * struct task_struct *tsk = current; > + * int *newpid; > + * newpid = xsplice_shadow_alloc(tsk, "newpid", sizeof(int)); > + * if (newpid) > + * *newpid = 1000; > + * > + * To retrieve a pointer to the variable: > + * > + * struct task_struct *tsk = current; > + * int *newpid; > + * newpid = xsplice_shadow_get(tsk, "newpid"); > + * if (newpid) > + * printk("task newpid = %d\n", *newpid); // prints "task newpid = 1000" > + * > + * To free it: > + * > + * xsplice_shadow_free(tsk, "newpid"); > + */ While this indeed provides some basic understanding, I think this should be using a more Xen-affine example, and I fail to see how with particularly the example given this is going to be useful: How would the patch module sanely and within reasonable time get hold of all instances of a particular type? Jan
On Mon, Apr 04, 2016 at 09:18:34AM -0600, Jan Beulich wrote: > >>> On 24.03.16 at 21:00, <konrad.wilk@oracle.com> wrote: > > Shadow variables are a piece of infrastructure to be used by xsplice > > modules. They are used to attach a new piece of data to an existing > > structure in memory. > > An already known question again: Out of recent XSAs, how many > needed such? I.e. can#t we put this off for now? Yes I believe we can.
On 04/06/2016 03:26 AM, Konrad Rzeszutek Wilk wrote: > On Mon, Apr 04, 2016 at 09:18:34AM -0600, Jan Beulich wrote: >>>>> On 24.03.16 at 21:00, <konrad.wilk@oracle.com> wrote: >>> Shadow variables are a piece of infrastructure to be used by xsplice >>> modules. They are used to attach a new piece of data to an existing >>> structure in memory. >> >> An already known question again: Out of recent XSAs, how many >> needed such? I.e. can#t we put this off for now? > > Yes I believe we can. > I didn't really intend for this to be submitted now, especially since it can be included in the binary patch itself if needed. So I think dropping it is OK.
diff --git a/xen/common/Makefile b/xen/common/Makefile index afd84b6..8371e2c 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_XENOPROF) += xenoprof.o obj-y += xmalloc_tlsf.o obj-$(CONFIG_XSPLICE) += xsplice.o obj-$(CONFIG_XSPLICE) += xsplice_elf.o +obj-$(CONFIG_XSPLICE) += xsplice_shadow.o obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo unlz4 earlycpio,$(n).init.o) diff --git a/xen/common/xsplice_shadow.c b/xen/common/xsplice_shadow.c new file mode 100644 index 0000000..8c63d62 --- /dev/null +++ b/xen/common/xsplice_shadow.c @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2016 Citrix Systems R&D Ltd. + */ + +#include <xen/init.h> +#include <xen/kernel.h> +#include <xen/lib.h> +#include <xen/list.h> +#include <xen/spinlock.h> +#include <xen/xsplice_patch.h> + +#define SHADOW_SLOTS 257 +struct hlist_head shadow_tbl[SHADOW_SLOTS]; +static DEFINE_SPINLOCK(shadow_lock); + +struct shadow_var { + struct hlist_node list; /* Linked to 'shadow_tbl' */ + void *data; + const void *obj; + char var[16]; +}; + +void *xsplice_shadow_alloc(const void *obj, const char *var, size_t size) +{ + struct shadow_var *shadow; + unsigned int slot; + + shadow = xmalloc(struct shadow_var); + if ( !shadow ) + return NULL; + + shadow->obj = obj; + strlcpy(shadow->var, var, sizeof shadow->var); + shadow->data = xmalloc_bytes(size); + if ( !shadow->data ) + { + xfree(shadow); + return NULL; + } + + slot = (unsigned long)obj % SHADOW_SLOTS; + spin_lock(&shadow_lock); + hlist_add_head(&shadow->list, &shadow_tbl[slot]); + spin_unlock(&shadow_lock); + + return shadow->data; +} + +void xsplice_shadow_free(const void *obj, const char *var) +{ + struct shadow_var *entry, *shadow = NULL; + unsigned int slot; + struct hlist_node *next; + + slot = (unsigned long)obj % SHADOW_SLOTS; + + spin_lock(&shadow_lock); + hlist_for_each_entry(entry, next, &shadow_tbl[slot], list) + { + if ( entry->obj == obj && + !strcmp(entry->var, var) ) + { + shadow = entry; + break; + } + } + if (shadow) + { + hlist_del(&shadow->list); + xfree(shadow->data); + xfree(shadow); + } + spin_unlock(&shadow_lock); +} + +void *xsplice_shadow_get(const void *obj, const char *var) +{ + struct shadow_var *entry; + unsigned int slot; + struct hlist_node *next; + void *ret = NULL; + + slot = (unsigned long)obj % SHADOW_SLOTS; + + spin_lock(&shadow_lock); + hlist_for_each_entry(entry, next, &shadow_tbl[slot], list) + { + if ( entry->obj == obj && + !strcmp(entry->var, var) ) + { + ret = entry->data; + break; + } + } + + spin_unlock(&shadow_lock); + return ret; +} + +static int __init xsplice_shadow_init(void) +{ + int i; + + for ( i = 0; i < SHADOW_SLOTS; i++ ) + INIT_HLIST_HEAD(&shadow_tbl[i]); + + return 0; +} +__initcall(xsplice_shadow_init); diff --git a/xen/include/xen/xsplice_patch.h b/xen/include/xen/xsplice_patch.h index 19d3f76..e297fe1 100644 --- a/xen/include/xen/xsplice_patch.h +++ b/xen/include/xen/xsplice_patch.h @@ -56,4 +56,40 @@ typedef void (*xsplice_unloadcall_t)(void); #define XSPLICE_UNLOAD_HOOK(_fn) \ xsplice_unloadcall_t __attribute__((weak)) xsplice_unload_data __section(".xsplice.hooks.unload") = _fn; + +/* + * The following definitions are to be used in patches. They are taken + * from kpatch. + */ + +/* + * xsplice shadow variables + * + * These functions can be used to add new "shadow" fields to existing data + * structures. For example, to allocate a "newpid" variable associated with an + * instance of task_struct, and assign it a value of 1000: + * + * struct task_struct *tsk = current; + * int *newpid; + * newpid = xsplice_shadow_alloc(tsk, "newpid", sizeof(int)); + * if (newpid) + * *newpid = 1000; + * + * To retrieve a pointer to the variable: + * + * struct task_struct *tsk = current; + * int *newpid; + * newpid = xsplice_shadow_get(tsk, "newpid"); + * if (newpid) + * printk("task newpid = %d\n", *newpid); // prints "task newpid = 1000" + * + * To free it: + * + * xsplice_shadow_free(tsk, "newpid"); + */ + +void *xsplice_shadow_alloc(const void *obj, const char *var, size_t size); +void xsplice_shadow_free(const void *obj, const char *var); +void *xsplice_shadow_get(const void *obj, const char *var); + #endif /* __XEN_XSPLICE_PATCH_H__ */