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

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 17 Jun 2014 11:04:42 +0000
Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> 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@hitachi.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Josh Poimboeuf <jpoimboe@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
>  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;
>  }
>  
>  /*
>