Re: [PATCH -tip v2 1/3] ftrace: Simplify ftrace_hash_disable/enable path in ftrace_hash_move

From: Steven Rostedt
Date: Thu Jun 19 2014 - 22:08:11 EST


On Tue, 17 Jun 2014 11:04:42 +0000
Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx> wrote:

> Simplify ftrace_hash_disable/enable path in ftrace_hash_move
> for hardening the process if the memory allocation failed.
>

Regardless of what we do with IPMODIFY, I pulled this into my 3.17
queue. You don't need to resend it with any new versions of the patches.

-- Steve

> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> kernel/trace/ftrace.c | 33 +++++++++++----------------------
> 1 file changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5b372e3..d65719d 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1304,25 +1304,15 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
> struct ftrace_hash *new_hash;
> int size = src->count;
> int bits = 0;
> - int ret;
> int i;
>
> /*
> - * Remove the current set, update the hash and add
> - * them back.
> - */
> - ftrace_hash_rec_disable(ops, enable);
> -
> - /*
> * If the new source is empty, just free dst and assign it
> * the empty_hash.
> */
> if (!src->count) {
> - free_ftrace_hash_rcu(*dst);
> - rcu_assign_pointer(*dst, EMPTY_HASH);
> - /* still need to update the function records */
> - ret = 0;
> - goto out;
> + new_hash = EMPTY_HASH;
> + goto update;
> }
>
> /*
> @@ -1335,10 +1325,9 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
> if (bits > FTRACE_HASH_MAX_BITS)
> bits = FTRACE_HASH_MAX_BITS;
>
> - ret = -ENOMEM;
> new_hash = alloc_ftrace_hash(bits);
> if (!new_hash)
> - goto out;
> + return -ENOMEM;
>
> size = 1 << src->size_bits;
> for (i = 0; i < size; i++) {
> @@ -1349,20 +1338,20 @@ ftrace_hash_move(struct ftrace_ops *ops, int enable,
> }
> }
>
> +update:
> + /*
> + * Remove the current set, update the hash and add
> + * them back.
> + */
> + ftrace_hash_rec_disable(ops, enable);
> +
> old_hash = *dst;
> rcu_assign_pointer(*dst, new_hash);
> free_ftrace_hash_rcu(old_hash);
>
> - ret = 0;
> - out:
> - /*
> - * Enable regardless of ret:
> - * On success, we enable the new hash.
> - * On failure, we re-enable the original hash.
> - */
> ftrace_hash_rec_enable(ops, enable);
>
> - return ret;
> + return 0;
> }
>
> /*
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/