[PATCH] kprobes: Fix build errors with CONFIG_KRETPROBES=n

Masami Hiramatsu posted 1 patch 4 years ago
include/linux/kprobes.h |    2 -
kernel/kprobes.c        |  144 +++++++++++++++++++++++------------------------
2 files changed, 72 insertions(+), 74 deletions(-)
[PATCH] kprobes: Fix build errors with CONFIG_KRETPROBES=n
Posted by Masami Hiramatsu 4 years ago
Max Filippov reported:

When building kernel with CONFIG_KRETPROBES=n kernel/kprobes.c
compilation fails with the following messages:

  kernel/kprobes.c: In function ‘recycle_rp_inst’:
  kernel/kprobes.c:1273:32: error: implicit declaration of function
                                   ‘get_kretprobe’

  kernel/kprobes.c: In function ‘kprobe_flush_task’:
  kernel/kprobes.c:1299:35: error: ‘struct task_struct’ has no member
                                   named ‘kretprobe_instances’

This came from the commit d741bf41d7c7 ("kprobes: Remove
kretprobe hash") which introduced get_kretprobe() and
kretprobe_instances member in task_struct when CONFIG_KRETPROBES=y,
but did not make recycle_rp_inst() and kprobe_flush_task()
depending on CONFIG_KRETPORBES.

Since those functions are only used for kretprobe, move those
functions into #ifdef CONFIG_KRETPROBE area.

Reported-by: Max Filippov <jcmvbkbc@gmail.com>
Fixes: d741bf41d7c7 ("kprobes: Remove kretprobe hash")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 include/linux/kprobes.h |    2 -
 kernel/kprobes.c        |  144 +++++++++++++++++++++++------------------------
 2 files changed, 72 insertions(+), 74 deletions(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 157168769fc2..55041d2f884d 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -424,7 +424,7 @@ void unregister_kretprobe(struct kretprobe *rp);
 int register_kretprobes(struct kretprobe **rps, int num);
 void unregister_kretprobes(struct kretprobe **rps, int num);
 
-#ifdef CONFIG_KRETPROBE_ON_RETHOOK
+#if defined(CONFIG_KRETPROBE_ON_RETHOOK) || !defined(CONFIG_KRETPROBES)
 #define kprobe_flush_task(tk)	do {} while (0)
 #else
 void kprobe_flush_task(struct task_struct *tk);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index dd58c0be9ce2..f214f8c088ed 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1257,79 +1257,6 @@ void kprobe_busy_end(void)
 	preempt_enable();
 }
 
-#if !defined(CONFIG_KRETPROBE_ON_RETHOOK)
-static void free_rp_inst_rcu(struct rcu_head *head)
-{
-	struct kretprobe_instance *ri = container_of(head, struct kretprobe_instance, rcu);
-
-	if (refcount_dec_and_test(&ri->rph->ref))
-		kfree(ri->rph);
-	kfree(ri);
-}
-NOKPROBE_SYMBOL(free_rp_inst_rcu);
-
-static void recycle_rp_inst(struct kretprobe_instance *ri)
-{
-	struct kretprobe *rp = get_kretprobe(ri);
-
-	if (likely(rp))
-		freelist_add(&ri->freelist, &rp->freelist);
-	else
-		call_rcu(&ri->rcu, free_rp_inst_rcu);
-}
-NOKPROBE_SYMBOL(recycle_rp_inst);
-
-/*
- * This function is called from delayed_put_task_struct() when a task is
- * dead and cleaned up to recycle any kretprobe instances associated with
- * this task. These left over instances represent probed functions that
- * have been called but will never return.
- */
-void kprobe_flush_task(struct task_struct *tk)
-{
-	struct kretprobe_instance *ri;
-	struct llist_node *node;
-
-	/* Early boot, not yet initialized. */
-	if (unlikely(!kprobes_initialized))
-		return;
-
-	kprobe_busy_begin();
-
-	node = __llist_del_all(&tk->kretprobe_instances);
-	while (node) {
-		ri = container_of(node, struct kretprobe_instance, llist);
-		node = node->next;
-
-		recycle_rp_inst(ri);
-	}
-
-	kprobe_busy_end();
-}
-NOKPROBE_SYMBOL(kprobe_flush_task);
-
-static inline void free_rp_inst(struct kretprobe *rp)
-{
-	struct kretprobe_instance *ri;
-	struct freelist_node *node;
-	int count = 0;
-
-	node = rp->freelist.head;
-	while (node) {
-		ri = container_of(node, struct kretprobe_instance, freelist);
-		node = node->next;
-
-		kfree(ri);
-		count++;
-	}
-
-	if (refcount_sub_and_test(count, &rp->rph->ref)) {
-		kfree(rp->rph);
-		rp->rph = NULL;
-	}
-}
-#endif	/* !CONFIG_KRETPROBE_ON_RETHOOK */
-
 /* Add the new probe to 'ap->list'. */
 static int add_new_kprobe(struct kprobe *ap, struct kprobe *p)
 {
@@ -1928,6 +1855,77 @@ static struct notifier_block kprobe_exceptions_nb = {
 #ifdef CONFIG_KRETPROBES
 
 #if !defined(CONFIG_KRETPROBE_ON_RETHOOK)
+static void free_rp_inst_rcu(struct rcu_head *head)
+{
+	struct kretprobe_instance *ri = container_of(head, struct kretprobe_instance, rcu);
+
+	if (refcount_dec_and_test(&ri->rph->ref))
+		kfree(ri->rph);
+	kfree(ri);
+}
+NOKPROBE_SYMBOL(free_rp_inst_rcu);
+
+static void recycle_rp_inst(struct kretprobe_instance *ri)
+{
+	struct kretprobe *rp = get_kretprobe(ri);
+
+	if (likely(rp))
+		freelist_add(&ri->freelist, &rp->freelist);
+	else
+		call_rcu(&ri->rcu, free_rp_inst_rcu);
+}
+NOKPROBE_SYMBOL(recycle_rp_inst);
+
+/*
+ * This function is called from delayed_put_task_struct() when a task is
+ * dead and cleaned up to recycle any kretprobe instances associated with
+ * this task. These left over instances represent probed functions that
+ * have been called but will never return.
+ */
+void kprobe_flush_task(struct task_struct *tk)
+{
+	struct kretprobe_instance *ri;
+	struct llist_node *node;
+
+	/* Early boot, not yet initialized. */
+	if (unlikely(!kprobes_initialized))
+		return;
+
+	kprobe_busy_begin();
+
+	node = __llist_del_all(&tk->kretprobe_instances);
+	while (node) {
+		ri = container_of(node, struct kretprobe_instance, llist);
+		node = node->next;
+
+		recycle_rp_inst(ri);
+	}
+
+	kprobe_busy_end();
+}
+NOKPROBE_SYMBOL(kprobe_flush_task);
+
+static inline void free_rp_inst(struct kretprobe *rp)
+{
+	struct kretprobe_instance *ri;
+	struct freelist_node *node;
+	int count = 0;
+
+	node = rp->freelist.head;
+	while (node) {
+		ri = container_of(node, struct kretprobe_instance, freelist);
+		node = node->next;
+
+		kfree(ri);
+		count++;
+	}
+
+	if (refcount_sub_and_test(count, &rp->rph->ref)) {
+		kfree(rp->rph);
+		rp->rph = NULL;
+	}
+}
+
 /* This assumes the 'tsk' is the current task or the is not running. */
 static kprobe_opcode_t *__kretprobe_find_ret_addr(struct task_struct *tsk,
 						  struct llist_node **cur)

Re: [PATCH] kprobes: Fix build errors with CONFIG_KRETPROBES=n
Posted by Max Filippov 4 years ago
On Tue, May 3, 2022 at 8:36 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Max Filippov reported:
>
> When building kernel with CONFIG_KRETPROBES=n kernel/kprobes.c
> compilation fails with the following messages:
>
>   kernel/kprobes.c: In function ‘recycle_rp_inst’:
>   kernel/kprobes.c:1273:32: error: implicit declaration of function
>                                    ‘get_kretprobe’
>
>   kernel/kprobes.c: In function ‘kprobe_flush_task’:
>   kernel/kprobes.c:1299:35: error: ‘struct task_struct’ has no member
>                                    named ‘kretprobe_instances’
>
> This came from the commit d741bf41d7c7 ("kprobes: Remove
> kretprobe hash") which introduced get_kretprobe() and
> kretprobe_instances member in task_struct when CONFIG_KRETPROBES=y,
> but did not make recycle_rp_inst() and kprobe_flush_task()
> depending on CONFIG_KRETPORBES.
>
> Since those functions are only used for kretprobe, move those
> functions into #ifdef CONFIG_KRETPROBE area.
>
> Reported-by: Max Filippov <jcmvbkbc@gmail.com>
> Fixes: d741bf41d7c7 ("kprobes: Remove kretprobe hash")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  include/linux/kprobes.h |    2 -
>  kernel/kprobes.c        |  144 +++++++++++++++++++++++------------------------
>  2 files changed, 72 insertions(+), 74 deletions(-)

Tested-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max
Re: [PATCH] kprobes: Fix build errors with CONFIG_KRETPROBES=n
Posted by Masami Hiramatsu 4 years ago
On Wed, 4 May 2022 10:53:42 -0700
Max Filippov <jcmvbkbc@gmail.com> wrote:

> On Tue, May 3, 2022 at 8:36 PM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Max Filippov reported:
> >
> > When building kernel with CONFIG_KRETPROBES=n kernel/kprobes.c
> > compilation fails with the following messages:
> >
> >   kernel/kprobes.c: In function ‘recycle_rp_inst’:
> >   kernel/kprobes.c:1273:32: error: implicit declaration of function
> >                                    ‘get_kretprobe’
> >
> >   kernel/kprobes.c: In function ‘kprobe_flush_task’:
> >   kernel/kprobes.c:1299:35: error: ‘struct task_struct’ has no member
> >                                    named ‘kretprobe_instances’
> >
> > This came from the commit d741bf41d7c7 ("kprobes: Remove
> > kretprobe hash") which introduced get_kretprobe() and
> > kretprobe_instances member in task_struct when CONFIG_KRETPROBES=y,
> > but did not make recycle_rp_inst() and kprobe_flush_task()
> > depending on CONFIG_KRETPORBES.
> >
> > Since those functions are only used for kretprobe, move those
> > functions into #ifdef CONFIG_KRETPROBE area.
> >
> > Reported-by: Max Filippov <jcmvbkbc@gmail.com>
> > Fixes: d741bf41d7c7 ("kprobes: Remove kretprobe hash")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  include/linux/kprobes.h |    2 -
> >  kernel/kprobes.c        |  144 +++++++++++++++++++++++------------------------
> >  2 files changed, 72 insertions(+), 74 deletions(-)
> 
> Tested-by: Max Filippov <jcmvbkbc@gmail.com>

Thank you!

Steve, can you pick this fix?

Thanks,

-- 
Masami Hiramatsu <mhiramat@kernel.org>
Re: [PATCH] kprobes: Fix build errors with CONFIG_KRETPROBES=n
Posted by Steven Rostedt 4 years ago
On Thu, 5 May 2022 09:31:40 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Steve, can you pick this fix?

Hi Masami,

When I get back home from traveling I'll pull this in.

-- Steve
Re: [PATCH] kprobes: Fix build errors with CONFIG_KRETPROBES=n
Posted by Steven Rostedt 3 years, 11 months ago
On Sat, 7 May 2022 09:57:23 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 5 May 2022 09:31:40 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Steve, can you pick this fix?  
> 
> Hi Masami,
> 
> When I get back home from traveling I'll pull this in.
>

Just added it to my queue. I'll be running tests when I've finished with
all the patches in my backlog.

-- Steve
Re: [PATCH] kprobes: Fix build errors with CONFIG_KRETPROBES=n
Posted by Masami Hiramatsu 4 years ago
Hello Max,

Can you test this patch? Since the other archs already implemented
kretprobe, I couldn't test it without modifying kernel.

Thank you,

On Wed,  4 May 2022 12:36:31 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Max Filippov reported:
> 
> When building kernel with CONFIG_KRETPROBES=n kernel/kprobes.c
> compilation fails with the following messages:
> 
>   kernel/kprobes.c: In function ‘recycle_rp_inst’:
>   kernel/kprobes.c:1273:32: error: implicit declaration of function
>                                    ‘get_kretprobe’
> 
>   kernel/kprobes.c: In function ‘kprobe_flush_task’:
>   kernel/kprobes.c:1299:35: error: ‘struct task_struct’ has no member
>                                    named ‘kretprobe_instances’
> 
> This came from the commit d741bf41d7c7 ("kprobes: Remove
> kretprobe hash") which introduced get_kretprobe() and
> kretprobe_instances member in task_struct when CONFIG_KRETPROBES=y,
> but did not make recycle_rp_inst() and kprobe_flush_task()
> depending on CONFIG_KRETPORBES.
> 
> Since those functions are only used for kretprobe, move those
> functions into #ifdef CONFIG_KRETPROBE area.
> 
> Reported-by: Max Filippov <jcmvbkbc@gmail.com>
> Fixes: d741bf41d7c7 ("kprobes: Remove kretprobe hash")
> Cc: stable@vger.kernel.org
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  include/linux/kprobes.h |    2 -
>  kernel/kprobes.c        |  144 +++++++++++++++++++++++------------------------
>  2 files changed, 72 insertions(+), 74 deletions(-)
> 
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 157168769fc2..55041d2f884d 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -424,7 +424,7 @@ void unregister_kretprobe(struct kretprobe *rp);
>  int register_kretprobes(struct kretprobe **rps, int num);
>  void unregister_kretprobes(struct kretprobe **rps, int num);
>  
> -#ifdef CONFIG_KRETPROBE_ON_RETHOOK
> +#if defined(CONFIG_KRETPROBE_ON_RETHOOK) || !defined(CONFIG_KRETPROBES)
>  #define kprobe_flush_task(tk)	do {} while (0)
>  #else
>  void kprobe_flush_task(struct task_struct *tk);
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index dd58c0be9ce2..f214f8c088ed 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1257,79 +1257,6 @@ void kprobe_busy_end(void)
>  	preempt_enable();
>  }
>  
> -#if !defined(CONFIG_KRETPROBE_ON_RETHOOK)
> -static void free_rp_inst_rcu(struct rcu_head *head)
> -{
> -	struct kretprobe_instance *ri = container_of(head, struct kretprobe_instance, rcu);
> -
> -	if (refcount_dec_and_test(&ri->rph->ref))
> -		kfree(ri->rph);
> -	kfree(ri);
> -}
> -NOKPROBE_SYMBOL(free_rp_inst_rcu);
> -
> -static void recycle_rp_inst(struct kretprobe_instance *ri)
> -{
> -	struct kretprobe *rp = get_kretprobe(ri);
> -
> -	if (likely(rp))
> -		freelist_add(&ri->freelist, &rp->freelist);
> -	else
> -		call_rcu(&ri->rcu, free_rp_inst_rcu);
> -}
> -NOKPROBE_SYMBOL(recycle_rp_inst);
> -
> -/*
> - * This function is called from delayed_put_task_struct() when a task is
> - * dead and cleaned up to recycle any kretprobe instances associated with
> - * this task. These left over instances represent probed functions that
> - * have been called but will never return.
> - */
> -void kprobe_flush_task(struct task_struct *tk)
> -{
> -	struct kretprobe_instance *ri;
> -	struct llist_node *node;
> -
> -	/* Early boot, not yet initialized. */
> -	if (unlikely(!kprobes_initialized))
> -		return;
> -
> -	kprobe_busy_begin();
> -
> -	node = __llist_del_all(&tk->kretprobe_instances);
> -	while (node) {
> -		ri = container_of(node, struct kretprobe_instance, llist);
> -		node = node->next;
> -
> -		recycle_rp_inst(ri);
> -	}
> -
> -	kprobe_busy_end();
> -}
> -NOKPROBE_SYMBOL(kprobe_flush_task);
> -
> -static inline void free_rp_inst(struct kretprobe *rp)
> -{
> -	struct kretprobe_instance *ri;
> -	struct freelist_node *node;
> -	int count = 0;
> -
> -	node = rp->freelist.head;
> -	while (node) {
> -		ri = container_of(node, struct kretprobe_instance, freelist);
> -		node = node->next;
> -
> -		kfree(ri);
> -		count++;
> -	}
> -
> -	if (refcount_sub_and_test(count, &rp->rph->ref)) {
> -		kfree(rp->rph);
> -		rp->rph = NULL;
> -	}
> -}
> -#endif	/* !CONFIG_KRETPROBE_ON_RETHOOK */
> -
>  /* Add the new probe to 'ap->list'. */
>  static int add_new_kprobe(struct kprobe *ap, struct kprobe *p)
>  {
> @@ -1928,6 +1855,77 @@ static struct notifier_block kprobe_exceptions_nb = {
>  #ifdef CONFIG_KRETPROBES
>  
>  #if !defined(CONFIG_KRETPROBE_ON_RETHOOK)
> +static void free_rp_inst_rcu(struct rcu_head *head)
> +{
> +	struct kretprobe_instance *ri = container_of(head, struct kretprobe_instance, rcu);
> +
> +	if (refcount_dec_and_test(&ri->rph->ref))
> +		kfree(ri->rph);
> +	kfree(ri);
> +}
> +NOKPROBE_SYMBOL(free_rp_inst_rcu);
> +
> +static void recycle_rp_inst(struct kretprobe_instance *ri)
> +{
> +	struct kretprobe *rp = get_kretprobe(ri);
> +
> +	if (likely(rp))
> +		freelist_add(&ri->freelist, &rp->freelist);
> +	else
> +		call_rcu(&ri->rcu, free_rp_inst_rcu);
> +}
> +NOKPROBE_SYMBOL(recycle_rp_inst);
> +
> +/*
> + * This function is called from delayed_put_task_struct() when a task is
> + * dead and cleaned up to recycle any kretprobe instances associated with
> + * this task. These left over instances represent probed functions that
> + * have been called but will never return.
> + */
> +void kprobe_flush_task(struct task_struct *tk)
> +{
> +	struct kretprobe_instance *ri;
> +	struct llist_node *node;
> +
> +	/* Early boot, not yet initialized. */
> +	if (unlikely(!kprobes_initialized))
> +		return;
> +
> +	kprobe_busy_begin();
> +
> +	node = __llist_del_all(&tk->kretprobe_instances);
> +	while (node) {
> +		ri = container_of(node, struct kretprobe_instance, llist);
> +		node = node->next;
> +
> +		recycle_rp_inst(ri);
> +	}
> +
> +	kprobe_busy_end();
> +}
> +NOKPROBE_SYMBOL(kprobe_flush_task);
> +
> +static inline void free_rp_inst(struct kretprobe *rp)
> +{
> +	struct kretprobe_instance *ri;
> +	struct freelist_node *node;
> +	int count = 0;
> +
> +	node = rp->freelist.head;
> +	while (node) {
> +		ri = container_of(node, struct kretprobe_instance, freelist);
> +		node = node->next;
> +
> +		kfree(ri);
> +		count++;
> +	}
> +
> +	if (refcount_sub_and_test(count, &rp->rph->ref)) {
> +		kfree(rp->rph);
> +		rp->rph = NULL;
> +	}
> +}
> +
>  /* This assumes the 'tsk' is the current task or the is not running. */
>  static kprobe_opcode_t *__kretprobe_find_ret_addr(struct task_struct *tsk,
>  						  struct llist_node **cur)
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>