| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <54DA4DA7.1040902@suse.cz>
Date: Tue, 10 Feb 2015 19:27:51 +0100
From: Jiri Slaby <jslaby@...e.cz>
To: Josh Poimboeuf <jpoimboe@...hat.com>,
Seth Jennings <sjenning@...hat.com>,
Jiri Kosina <jkosina@...e.cz>, Vojtech Pavlik <vojtech@...e.cz>
CC: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 3/9] livepatch: move patching functions into patch.c
On 02/09/2015, 06:31 PM, Josh Poimboeuf wrote:
> Move functions related to the actual patching of functions and objects
> into a new patch.c file.
>
> The only functional change is to remove the unnecessary
> WARN_ON(!klp_is_object_loaded()) check from klp_patch_object().
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -24,29 +24,10 @@
> #include <linux/kernel.h>
> #include <linux/mutex.h>
> #include <linux/slab.h>
> -#include <linux/ftrace.h>
> #include <linux/list.h>
> #include <linux/kallsyms.h>
> -#include <linux/livepatch.h>
I don't understand, you define some functions declared there and you
remove the include? patch.h below is not enough. When somebody shuffles
with the files again, we would have to fix this.
>
> -/**
> - * struct klp_ops - structure for tracking registered ftrace ops structs
> - *
> - * A single ftrace_ops is shared between all enabled replacement functions
> - * (klp_func structs) which have the same old_addr. This allows the switch
> - * between function versions to happen instantaneously by updating the klp_ops
> - * struct's func_stack list. The winner is the klp_func at the top of the
> - * func_stack (front of the list).
> - *
> - * @node: node for the global klp_ops list
> - * @func_stack: list head for the stack of klp_func's (active func is on top)
> - * @fops: registered ftrace ops struct
> - */
> -struct klp_ops {
> - struct list_head node;
> - struct list_head func_stack;
> - struct ftrace_ops fops;
> -};
> +#include "patch.h"
...
> --- /dev/null
> +++ b/kernel/livepatch/patch.c
> @@ -0,0 +1,176 @@
> +/*
> + * patch.c - Kernel Live Patching patching functions
...
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/slab.h>
> +
> +#include "patch.h"
> +
> +static LIST_HEAD(klp_ops);
list.h should be included.
> +static void notrace klp_ftrace_handler(unsigned long ip,
> + unsigned long parent_ip,
> + struct ftrace_ops *fops,
ftrace.h should be included.
> + struct pt_regs *regs)
> +{
> + struct klp_ops *ops;
> + struct klp_func *func;
> +
> + ops = container_of(fops, struct klp_ops, fops);
> +
> + rcu_read_lock();
> + func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
> + stack_node);
rculist.h & perhaps rcupdate.h?
> + rcu_read_unlock();
> +
> + if (WARN_ON_ONCE(!func))
> + return;
> +
> + klp_arch_set_pc(regs, (unsigned long)func->new_func);
> +}
...
> +static void klp_unpatch_func(struct klp_func *func)
> +{
> + struct klp_ops *ops;
> +
> + WARN_ON(!func->patched);
> + WARN_ON(!func->old_addr);
bug.h
> +
> + ops = klp_find_ops(func->old_addr);
> + if (WARN_ON(!ops))
> + return;
> +
> + if (list_is_singular(&ops->func_stack)) {
> + WARN_ON(unregister_ftrace_function(&ops->fops));
> + WARN_ON(ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0));
> +
> + list_del_rcu(&func->stack_node);
> + list_del(&ops->node);
> + kfree(ops);
> + } else {
> + list_del_rcu(&func->stack_node);
> + }
> +
> + func->patched = 0;
> +}
> +
> +static int klp_patch_func(struct klp_func *func)
> +{
> + struct klp_ops *ops;
> + int ret;
> +
> + if (WARN_ON(!func->old_addr))
> + return -EINVAL;
> +
> + if (WARN_ON(func->patched))
> + return -EINVAL;
> +
> + ops = klp_find_ops(func->old_addr);
> + if (!ops) {
> + ops = kzalloc(sizeof(*ops), GFP_KERNEL);
> + if (!ops)
> + return -ENOMEM;
> +
> + ops->fops.func = klp_ftrace_handler;
> + ops->fops.flags = FTRACE_OPS_FL_SAVE_REGS |
> + FTRACE_OPS_FL_DYNAMIC |
> + FTRACE_OPS_FL_IPMODIFY;
> +
> + list_add(&ops->node, &klp_ops);
> +
> + INIT_LIST_HEAD(&ops->func_stack);
> + list_add_rcu(&func->stack_node, &ops->func_stack);
> +
> + ret = ftrace_set_filter_ip(&ops->fops, func->old_addr, 0, 0);
> + if (ret) {
> + pr_err("failed to set ftrace filter for function '%s' (%d)\n",
> + func->old_name, ret);
printk.h
> + goto err;
> + }
> +
> + ret = register_ftrace_function(&ops->fops);
> + if (ret) {
> + pr_err("failed to register ftrace handler for function '%s' (%d)\n",
> + func->old_name, ret);
> + ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0);
> + goto err;
> + }
> + } else {
> + list_add_rcu(&func->stack_node, &ops->func_stack);
> + }
> +
> + func->patched = 1;
> +
> + return 0;
> +
> +err:
> + list_del_rcu(&func->stack_node);
> + list_del(&ops->node);
> + kfree(ops);
> + return ret;
> +}
...
> --- /dev/null
> +++ b/kernel/livepatch/patch.h
> @@ -0,0 +1,25 @@
This is not a correct header. Double-inclusion protection is missing.
> +#include <linux/livepatch.h>
> +
> +/**
> + * struct klp_ops - structure for tracking registered ftrace ops structs
> + *
> + * A single ftrace_ops is shared between all enabled replacement functions
> + * (klp_func structs) which have the same old_addr. This allows the switch
> + * between function versions to happen instantaneously by updating the klp_ops
> + * struct's func_stack list. The winner is the klp_func at the top of the
> + * func_stack (front of the list).
> + *
> + * @node: node for the global klp_ops list
> + * @func_stack: list head for the stack of klp_func's (active func is on top)
> + * @fops: registered ftrace ops struct
> + */
> +struct klp_ops {
> + struct list_head node;
> + struct list_head func_stack;
> + struct ftrace_ops fops;
This header obviously needs list.h and ftrace.h.
> +};
> +
> +struct klp_ops *klp_find_ops(unsigned long old_addr);
> +
> +extern int klp_patch_object(struct klp_object *obj);
> +extern void klp_unpatch_object(struct klp_object *obj);
>
regards,
--
js
suse labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists