On Fri, 28 Jan 2022 23:33:15 +0900
Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> Add exit_handler to fprobe. fprobe + rethook allows us to hook the kernel
> function return. The rethook will be enabled only if the
> fprobe::exit_handler is set.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> ---
> Changes in v6:
> - Update according to the fprobe update.
> Changes in v5:
> - Add dependency for HAVE_RETHOOK.
> Changes in v4:
> - Check fprobe is disabled in the exit handler.
> Changes in v3:
> - Make sure to clear rethook->data before free.
> - Handler checks the data is not NULL.
> - Free rethook only if the rethook is using.
> ---
> include/linux/fprobe.h | 6 ++
> kernel/trace/Kconfig | 2 +
> kernel/trace/fprobe.c | 127 +++++++++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 123 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/fprobe.h b/include/linux/fprobe.h
> index b920dc1b2969..acfdcc37acf6 100644
> --- a/include/linux/fprobe.h
> +++ b/include/linux/fprobe.h
> @@ -5,19 +5,25 @@
>
> #include <linux/compiler.h>
> #include <linux/ftrace.h>
> +#include <linux/rethook.h>
>
> /**
> * struct fprobe - ftrace based probe.
> * @ops: The ftrace_ops.
> * @nmissed: The counter for missing events.
> * @flags: The status flag.
> + * @rethook: The rethook data structure. (internal data)
> * @entry_handler: The callback function for function entry.
> + * @exit_handler: The callback function for function exit.
> */
> struct fprobe {
> struct ftrace_ops ops;
> unsigned long nmissed;
> unsigned int flags;
> + struct rethook *rethook;
> +
> void (*entry_handler)(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs);
> + void (*exit_handler)(struct fprobe *fp, unsigned long entry_ip, struct pt_regs *regs);
> };
>
> #define FPROBE_FL_DISABLED 1
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 9e66fd29d94e..3c1f808969f1 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -245,6 +245,8 @@ config FPROBE
> bool "Kernel Function Probe (fprobe)"
> depends on FUNCTION_TRACER
> depends on DYNAMIC_FTRACE_WITH_REGS
> + depends on HAVE_RETHOOK
> + select RETHOOK
> default n
> help
> This option enables kernel function probe (fprobe) based on ftrace,
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 081aef6bf531..408dcb6503fe 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -8,12 +8,22 @@
> #include <linux/fprobe.h>
> #include <linux/kallsyms.h>
> #include <linux/kprobes.h>
> +#include <linux/rethook.h>
> #include <linux/slab.h>
> #include <linux/sort.h>
>
> +#include "trace.h"
> +
> +struct fprobe_rethook_node {
> + struct rethook_node node;
> + unsigned long entry_ip;
> +};
> +
> static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *ops, struct ftrace_regs *fregs)
> {
> + struct fprobe_rethook_node *fpr;
> + struct rethook_node *rh;
> struct fprobe *fp;
> int bit;
>
> @@ -30,10 +40,37 @@ static void fprobe_handler(unsigned long ip, unsigned long parent_ip,
> if (fp->entry_handler)
> fp->entry_handler(fp, ip, ftrace_get_regs(fregs));
>
> + if (fp->exit_handler) {
> + rh = rethook_try_get(fp->rethook);
> + if (!rh) {
> + fp->nmissed++;
> + goto out;
> + }
> + fpr = container_of(rh, struct fprobe_rethook_node, node);
> + fpr->entry_ip = ip;
> + rethook_hook(rh, ftrace_get_regs(fregs));
> + }
> +
> +out:
> ftrace_test_recursion_unlock(bit);
> }
> NOKPROBE_SYMBOL(fprobe_handler);
>
> +static void fprobe_exit_handler(struct rethook_node *rh, void *data,
> + struct pt_regs *regs)
> +{
> + struct fprobe *fp = (struct fprobe *)data;
> + struct fprobe_rethook_node *fpr;
> +
> + if (!fp || fprobe_disabled(fp))
> + return;
> +
> + fpr = container_of(rh, struct fprobe_rethook_node, node);
> +
> + fp->exit_handler(fp, fpr->entry_ip, regs);
> +}
> +NOKPROBE_SYMBOL(fprobe_exit_handler);
> +
> /* Convert ftrace location address from symbols */
> static unsigned long *get_ftrace_locations(const char **syms, int num)
> {
> @@ -76,6 +113,48 @@ static void fprobe_init(struct fprobe *fp)
> fp->ops.flags |= FTRACE_OPS_FL_SAVE_REGS;
> }
>
> +static int fprobe_init_rethook(struct fprobe *fp, int num)
> +{
> + int i, size;
> +
> + if (num < 0)
> + return -EINVAL;
> +
> + if (!fp->exit_handler) {
> + fp->rethook = NULL;
> + return 0;
> + }
> +
> + /* Initialize rethook if needed */
> + size = num * num_possible_cpus() * 2;
> + if (size < 0)
> + return -E2BIG;
> +
> + fp->rethook = rethook_alloc((void *)fp, fprobe_exit_handler);
> + for (i = 0; i < size; i++) {
> + struct rethook_node *node;
> +
> + node = kzalloc(sizeof(struct fprobe_rethook_node), GFP_KERNEL);
> + if (!node) {
> + rethook_free(fp->rethook);
> + fp->rethook = NULL;
> + return -ENOMEM;
> + }
> + rethook_add_node(fp->rethook, node);
> + }
> + return 0;
> +}
> +
> +static void fprobe_fail_cleanup(struct fprobe *fp)
> +{
> + if (fp->rethook) {
> + /* Don't need to cleanup rethook->handler because this is not used. */
> + rethook_free(fp->rethook);
> + fp->rethook = NULL;
> + }
> + ftrace_free_filter(&fp->ops);
> +}
> +
> /**
> * register_fprobe() - Register fprobe to ftrace by pattern.
> * @fp: A fprobe data structure to be registered.
> @@ -89,6 +168,7 @@ static void fprobe_init(struct fprobe *fp)
> */
> int register_fprobe(struct fprobe *fp, const char *filter, const char *notfilter)
> {
> + struct ftrace_hash *hash;
> unsigned char *str;
> int ret, len;
>
> @@ -113,10 +193,21 @@ int register_fprobe(struct fprobe *fp, const char *filter, const char *notfilter
> goto out;
> }
>
> - ret = register_ftrace_function(&fp->ops);
> + /* TODO:
> + * correctly calculate the total number of filtered symbols
> + * from both filter and notfilter.
> + */
> + hash = fp->ops.local_hash.filter_hash;
> + if (WARN_ON_ONCE(!hash))
> + goto out;
> +
> + ret = fprobe_init_rethook(fp, (int)hash->count);
> + if (!ret)
> + ret = register_ftrace_function(&fp->ops);
> +
> out:
> if (ret)
> - ftrace_free_filter(&fp->ops);
> + fprobe_fail_cleanup(fp);
> return ret;
> }
> EXPORT_SYMBOL_GPL(register_fprobe);
> @@ -144,12 +235,15 @@ int register_fprobe_ips(struct fprobe *fp, unsigned long *addrs, int num)
> fprobe_init(fp);
>
> ret = ftrace_set_filter_ips(&fp->ops, addrs, num, 0, 0);
> + if (ret)
> + return ret;
> +
> + ret = fprobe_init_rethook(fp, num);
> if (!ret)
> ret = register_ftrace_function(&fp->ops);
>
> if (ret)
> - ftrace_free_filter(&fp->ops);
> -
> + fprobe_fail_cleanup(fp);
> return ret;
> }
> EXPORT_SYMBOL_GPL(register_fprobe_ips);
> @@ -179,14 +273,16 @@ int register_fprobe_syms(struct fprobe *fp, const char **syms, int num)
> return PTR_ERR(addrs);
>
> ret = ftrace_set_filter_ips(&fp->ops, addrs, num, 0, 0);
> + kfree(addrs);
> if (ret)
> - goto out;
> - ret = register_ftrace_function(&fp->ops);
> - if (ret)
> - ftrace_free_filter(&fp->ops);
> + return ret;
>
> -out:
> - kfree(addrs);
> + ret = fprobe_init_rethook(fp, num);
> + if (!ret)
> + ret = register_ftrace_function(&fp->ops);
> +
> + if (ret)
> + fprobe_fail_cleanup(fp);
>
> return ret;
> }
> @@ -210,9 +306,16 @@ int unregister_fprobe(struct fprobe *fp)
> return -EINVAL;
>
> ret = unregister_ftrace_function(&fp->ops);
> + if (ret < 0)
> + return ret;
>
> - if (!ret)
> - ftrace_free_filter(&fp->ops);
> + if (fp->rethook) {
> + /* Make sure to clear rethook->data before freeing. */
> + WRITE_ONCE(fp->rethook->data, NULL);
> + barrier();
> + rethook_free(fp->rethook);
> + }
Oops, I felt uncomfortable on this part, and I found that this rethook_free()
must be called before unregister_ftrace_function() so that no more rethook will
be used, and also need to wait for rcu before returning (this is also done
in unregister_ftrace_function().)
Let me update this part.
Thank you,
> + ftrace_free_filter(&fp->ops);
>
> return ret;
> }
>
--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>
