[v5,27/28] xsplice: Add support for shadow variables.

Commit Message

Konrad Rzeszutek Wilk March 24, 2016, 8 p.m. UTC

From: Ross Lagerwall <[e-mail]>

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.

Martin Pohlack also mentions that using the SHADOW_SLOTS of small
prime numbers has the advantage of having a simple hash function.
Using round numbers (such as 256) will give "lot's of hash collisions
at the price of very fast hash computations as compilers, linkers, and
memory allocators tend to align starting addresses." (Martin)

Signed-off-by: Ross Lagerwall <[e-mail]>
---
Cc: Ian Jackson <[e-mail]>
Cc: Jan Beulich <[e-mail]>
Cc: Keir Fraser <[e-mail]>
Cc: Tim Deegan <[e-mail]>

v4: Add Copyright
v5: Change SHADOW_SLOTS to 257 per Martin's suggestion.
---
---
 xen/common/Makefile             |   1 +
 xen/common/xsplice_shadow.c     | 109 ++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/xsplice_patch.h |  36 +++++++++++++
 3 files changed, 146 insertions(+)
 create mode 100644 xen/common/xsplice_shadow.c

Comments

Jan Beulich April 4, 2016, 3:18 p.m. UTC | #1

>>> On 24.03.16 at 21:00, <[e-mail]> 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

Konrad Rzeszutek Wilk April 6, 2016, 2:26 a.m. UTC | #2

On Mon, Apr 04, 2016 at 09:18:34AM -0600, Jan Beulich wrote:
> >>> On 24.03.16 at 21:00, <[e-mail]> 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.

Ross Lagerwall April 8, 2016, 3:58 p.m. UTC | #3

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, <[e-mail]> 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.

Patch

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__ */