On Thu 2017-06-01 14:25:26, Joe Lawrence wrote:
> Modify the sample livepatch to demonstrate the shadow variable API.
>
> Signed-off-by: Joe Lawrence <joe.lawre...@redhat.com>
> ---
> samples/livepatch/livepatch-sample.c | 39
> +++++++++++++++++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/samples/livepatch/livepatch-sample.c
> b/samples/livepatch/livepatch-sample.c
> index 84795223f15f..e0236750cefb 100644
> --- a/samples/livepatch/livepatch-sample.c
> +++ b/samples/livepatch/livepatch-sample.c
> @@ -25,26 +25,57 @@
>
> /*
> * This (dumb) live patch overrides the function that prints the
> - * kernel boot cmdline when /proc/cmdline is read.
> + * kernel boot cmdline when /proc/cmdline is read. It also
> + * demonstrates a contrived shadow-variable usage.
> *
> * Example:
> *
> * $ cat /proc/cmdline
> * <your cmdline>
> + * current=<current task pointer> count=<shadow variable counter>
> *
> * $ insmod livepatch-sample.ko
> * $ cat /proc/cmdline
> * this has been live patched
> + * current=ffff8800331c9540 count=1
> + * $ cat /proc/cmdline
> + * this has been live patched
> + * current=ffff8800331c9540 count=2
> *
> * $ echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled
> * $ cat /proc/cmdline
> * <your cmdline>
> */
>
> +static LIST_HEAD(shadow_list);
> +
> +struct task_ctr {
> + struct list_head list;
> + int count;
> +};
> +
> #include <linux/seq_file.h>
> +#include <linux/slab.h>
> static int livepatch_cmdline_proc_show(struct seq_file *m, void *v)
> {
> + struct task_ctr *nd;
> +
> + nd = klp_shadow_get(current, "task_ctr");
> + if (!nd) {
> + nd = kzalloc(sizeof(*nd), GFP_KERNEL);
> + if (nd) {
> + list_add(&nd->list, &shadow_list);
> + klp_shadow_attach(current, "task_ctr", GFP_KERNEL, nd);
> + }
> + }
Hmm, this looks racy:
CPU0 CPU1
klp_shadow_get() klp_shadow_get()
# returns NULL # returns NULL
if (!nd) { if (!nd) {
nd = kzalloc(); nd = kzalloc();
list_add(&nd->list, list_add(&nd->list,
&shadow_list); &shadow_list);
BANG: This is one race. We add items into the shared shadow_list
in parallel. The question is if we need it. IMHO, the API
could help here, see the comments for livepatch_exit() below.
klp_shadow_attach(); klp_shadow_attach();
WARNING: This is fine because the shadow objects are for
different objects. I mean that "current" must point
to different processes on the two CPUs.
But it is racy in general. The question is if the API
could help here. A possibility might be to allow to
define a callback function that would create the shadow
structure when it does not exist. I mean something like
typedef void (*klp_shadow_create_obj_func_t)(void * obj);
void *klp_shadow_get_or_create(void *obj, int key, gfp_t gfp,
klp_shadow_create_obj_fun_t *create)
{
struct klp_shadow *shadow;
shadow = klp_shadow_get(obj, key);
if (!shadow && create) {
void *shadow_obj;
spin_lock_irqsave(&klp_shadow_lock, flags);
shadow = klp_shadow_get(obj, key);
if (shadow)
goto out;
shadow_obj = create(obj);
shadow = __klp_shadow_attach(obj, key, gfp,
shadow_obj);
out:
spin_unlock_irqrestore(&klp_shadow_lock, flags);
}
return shadow;
}
I do not know. Maybe it is too ugly. Or will it safe a duplicated code
in many cases?
> seq_printf(m, "%s\n", "this has been live patched");
> +
> + if (nd) {
> + nd->count++;
> + seq_printf(m, "current=%p count=%d\n", current, nd->count);
> + }
> +
> return 0;
> }
>
> @@ -99,6 +130,12 @@ static int livepatch_init(void)
>
> static void livepatch_exit(void)
> {
> + struct task_ctr *nd, *tmp;
> +
> + list_for_each_entry_safe(nd, tmp, &shadow_list, list) {
> + list_del(&nd->list);
> + kfree(nd);
I think that this will be a rather common operation. Do we need
the extra list? It might be enough to delete all entries
in the hash table with a given key. Well, we might need
a callback again:
typedef void (*klp_shadow_free_obj_func_t)(void *shadow_obj);
void klp_shadow_free_all(int key, klp_shadow_free_obj_fun_t *shadow_free)
{
int i;
struct klp_shadow *shadow;
spin_lock_irqsave(&klp_shadow_lock, flags);
hash_for_each(klp_shadow_hash, i, shadow, node) {
hash_del_rcu(&shadow->node);
/* patch and shadow data are not longer used. */
shadow_free(shadow->shadow_obj);
/*
* shadow structure might still be traversed
* by someone
*/
kfree_rcu(shadow, rcu_head);
}
}
spin_unlocklock_irqrestore(&klp_shadow_lock, flags);
}
Best Regards,
Petr
