fix

Author: olsajiri

kernel/bpf/trampoline.c

@@ -176,7 +176,6 @@ static int direct_ops_get(struct bpf_trampoline *tr)
 	tr->fops = &direct_ops;
 	return 0;
 }
-static void direct_ops_clear(struct bpf_trampoline *tr) { }
 static void direct_ops_free(struct bpf_trampoline *tr) { }
 #else
 static int direct_ops_get(struct bpf_trampoline *tr)
@@ -189,12 +188,6 @@ static int direct_ops_get(struct bpf_trampoline *tr)
 	return 0;
 }
 
-static void direct_ops_clear(struct bpf_trampoline *tr)
-{
-	tr->fops->func = NULL;
-	tr->fops->trampoline = 0;
-}
-
 static void direct_ops_free(struct bpf_trampoline *tr)
 {
 	if (tr->fops) {
@@ -548,9 +541,6 @@ static int bpf_trampoline_update(struct bpf_trampoline *tr, bool lock_direct_mut
 		 * BPF_TRAMP_F_SHARE_IPMODIFY is set, we can generate the
 		 * trampoline again, and retry register.
 		 */
-		/* reset fops->func and fops->trampoline for re-register */
-		direct_ops_clear(tr);
-
 		/* free im memory and reallocate later */
 		bpf_tramp_image_free(im);
 		goto again;

kernel/trace/ftrace.c

@@ -6121,21 +6121,10 @@ static struct ftrace_hash *hash_add(struct ftrace_hash *a, struct ftrace_hash *b
 	return NULL;
 }
 
-static void reset_direct_hash(struct ftrace_ops *ops, struct ftrace_hash **hashp)
-{
-	struct ftrace_hash *free_hash = direct_functions;
-
-	rcu_assign_pointer(direct_functions, *hashp);
-	*hashp = free_hash;
-
-	/* cleanup for possible another register call */
-	ops->func = NULL;
-	ops->trampoline = 0;
-}
-
 int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
 {
-	struct ftrace_hash *filter_hash = NULL, *new_hash = NULL, *free_hash = NULL;
+	struct ftrace_hash *old_direct_functions = NULL, *new_direct_functions = NULL;
+	struct ftrace_hash *old_filter_hash = NULL, *new_filter_hash = NULL;
 	struct ftrace_func_entry *entry;
 	int i, size, err = -EINVAL;
 	bool reg;
@@ -6154,10 +6143,11 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
 		}
 	}
 
-	filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+	old_filter_hash = ops->func_hash ? ops->func_hash->filter_hash : NULL;
+	old_direct_functions = direct_functions;
 
 	/* If there's nothing in filter_hash we need to register the ops. */
-	reg = hash_count(filter_hash) == 0;
+	reg = hash_count(old_filter_hash) == 0;
 	if (reg) {
 		if (ops->func || ops->trampoline)
 			goto out_unlock;
@@ -6166,40 +6156,54 @@ int update_ftrace_direct_add(struct ftrace_ops *ops, struct ftrace_hash *hash)
 	}
 
 	err = -ENOMEM;
-	filter_hash = hash_add(filter_hash, hash);
-	if (!filter_hash)
+	new_filter_hash = hash_add(old_filter_hash, hash);
+	if (!new_filter_hash)
 		goto out_unlock;
 
-	new_hash = hash_add(direct_functions, hash);
-	if (!new_hash)
+	new_direct_functions = hash_add(old_direct_functions, hash);
+	if (!new_direct_functions)
 		goto out_unlock;
 
-	free_hash = direct_functions;
-	rcu_assign_pointer(direct_functions, new_hash);
+	rcu_assign_pointer(direct_functions, new_direct_functions);
 
 	if (reg) {
 		ops->func = call_direct_funcs;
-		ops->flags = MULTI_FLAGS;
+		ops->flags |= MULTI_FLAGS;
 		ops->trampoline = FTRACE_REGS_ADDR;
-		ops->local_hash.filter_hash = filter_hash;
+		ops->local_hash.filter_hash = new_filter_hash;
 
 		err = register_ftrace_function_nolock(ops);
-		if (!err)
-			filter_hash = NULL;
+		if (err) {
+			/* restore old filter on error */
+			ops->local_hash.filter_hash = old_filter_hash;
+			old_filter_hash = new_filter_hash;
+
+			/* cleanup for possible another register call */
+			ops->func = NULL;
+			ops->trampoline = 0;
+		}
 	} else {
-		err = ftrace_update_ops(ops, filter_hash, EMPTY_HASH);
+		err = ftrace_update_ops(ops, new_filter_hash, EMPTY_HASH);
+		/*
+		 * new_filter_hash is dup-ed, so we need to release it anyway,
+		 * old_filter_hash either stays on error or is released already
+		 */
+		old_filter_hash = new_filter_hash;
 	}
 
-	if (err)
-		reset_direct_hash(ops, &free_hash);
+	if (err) {
+		/* reset direct_functions and free the new one */
+		rcu_assign_pointer(direct_functions, old_direct_functions);
+		old_direct_functions = new_direct_functions;
+	}
 
  out_unlock:
 	mutex_unlock(&direct_mutex);
 
-	if (free_hash && free_hash != EMPTY_HASH)
-		call_rcu_tasks(&free_hash->rcu, register_ftrace_direct_cb);
-	if (filter_hash)
-		free_ftrace_hash(filter_hash);
+	if (old_direct_functions && old_direct_functions != EMPTY_HASH)
+		call_rcu_tasks(&old_direct_functions->rcu, register_ftrace_direct_cb);
+	if (old_filter_hash)
+		free_ftrace_hash(old_filter_hash);
 
 	return err;
 }