linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kprobes: Print an error if probe is rejected
@ 2021-06-10  8:56 Naveen N. Rao
  2021-06-10 10:16 ` Masami Hiramatsu
  0 siblings, 1 reply; 13+ messages in thread
From: Naveen N. Rao @ 2021-06-10  8:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Masami Hiramatsu, Peter Zijlstra, Steven Rostedt, Aneesh Kumar K.V

When probing at different locations in the kernel, it is not always
evident if the location can be probed or not. As an example:

    $ perf probe __radix__flush_tlb_range:35
    Failed to write event: Invalid argument
      Error: Failed to add events.

The probed line above is:
     35         if (!mmu_has_feature(MMU_FTR_GTSE) && type == FLUSH_TYPE_GLOBAL) {

This ends up trying to probe on BUILD_BUG_ON(), which is rejected.
However, the user receives no indication at all as to why the probe
failed. Print an error in such cases so that it is clear that the probe
was rejected.

Reported-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/kprobes.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index e41385afe79dc5..7c8929165924ed 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1542,8 +1542,10 @@ static int check_kprobe_address_safe(struct kprobe *p,
 	int ret;
 
 	ret = arch_check_ftrace_location(p);
-	if (ret)
+	if (ret) {
+		pr_err("kprobes: can't probe at the provided ftrace location\n");
 		return ret;
+	}
 	jump_label_lock();
 	preempt_disable();
 
@@ -1552,6 +1554,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
 	    within_kprobe_blacklist((unsigned long) p->addr) ||
 	    jump_label_text_reserved(p->addr, p->addr) ||
 	    find_bug((unsigned long)p->addr)) {
+		pr_err("kprobes: can't probe at address in reject list\n");
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1976,8 +1979,10 @@ int register_kretprobe(struct kretprobe *rp)
 	void *addr;
 
 	ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
-	if (ret)
+	if (ret) {
+		pr_err("kretprobes: can't probe at address outside function entry\n");
 		return ret;
+	}
 
 	/* If only rp->kp.addr is specified, check reregistering kprobes */
 	if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
@@ -1989,8 +1994,10 @@ int register_kretprobe(struct kretprobe *rp)
 			return PTR_ERR(addr);
 
 		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
-			if (kretprobe_blacklist[i].addr == addr)
+			if (kretprobe_blacklist[i].addr == addr) {
+				pr_err("kretprobes: can't probe at address in reject list\n");
 				return -EINVAL;
+			}
 		}
 	}
 

base-commit: 2e38eb04c95e5546b71bb86ee699a891c7d212b5
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-10  8:56 [PATCH] kprobes: Print an error if probe is rejected Naveen N. Rao
@ 2021-06-10 10:16 ` Masami Hiramatsu
  2021-06-10 17:33   ` Steven Rostedt
  2021-06-11 13:55   ` Naveen N. Rao
  0 siblings, 2 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2021-06-10 10:16 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: linux-kernel, Masami Hiramatsu, Peter Zijlstra, Steven Rostedt,
	Aneesh Kumar K.V

Hi Naveen,

On Thu, 10 Jun 2021 14:26:17 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> When probing at different locations in the kernel, it is not always
> evident if the location can be probed or not. As an example:
> 
>     $ perf probe __radix__flush_tlb_range:35
>     Failed to write event: Invalid argument
>       Error: Failed to add events.
> 
> The probed line above is:
>      35         if (!mmu_has_feature(MMU_FTR_GTSE) && type == FLUSH_TYPE_GLOBAL) {
> 
> This ends up trying to probe on BUILD_BUG_ON(), which is rejected.
> However, the user receives no indication at all as to why the probe
> failed. Print an error in such cases so that it is clear that the probe
> was rejected.

Hmm, Nack for this way, but I understand that is a problem.
If you got the error in perf probe, which uses ftrace dynamic-event interface.
In that case, the errors should not be output in the dmesg, but are reported
via error_log in tracefs.
And kprobes itself should return different error code to the caller, instead
of printing error in dmesg. See below.

[...]
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1542,8 +1542,10 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  	int ret;
>  
>  	ret = arch_check_ftrace_location(p);
> -	if (ret)
> +	if (ret) {
> +		pr_err("kprobes: can't probe at the provided ftrace location\n");
>  		return ret;

This must be -EBUSY. (or arch depend return value)

> +	}
>  	jump_label_lock();
>  	preempt_disable();
>  
> @@ -1552,6 +1554,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
>  	    within_kprobe_blacklist((unsigned long) p->addr) ||
>  	    jump_label_text_reserved(p->addr, p->addr) ||
>  	    find_bug((unsigned long)p->addr)) {
> +		pr_err("kprobes: can't probe at address in reject list\n");
>  		ret = -EINVAL;

This maybe -EACCESS.

>  		goto out;
>  	}
> @@ -1976,8 +1979,10 @@ int register_kretprobe(struct kretprobe *rp)
>  	void *addr;
>  
>  	ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
> -	if (ret)
> +	if (ret) {
> +		pr_err("kretprobes: can't probe at address outside function entry\n");
>  		return ret;

return -ERANGE.

> +	}
>  
>  	/* If only rp->kp.addr is specified, check reregistering kprobes */
>  	if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
> @@ -1989,8 +1994,10 @@ int register_kretprobe(struct kretprobe *rp)
>  			return PTR_ERR(addr);
>  
>  		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
> -			if (kretprobe_blacklist[i].addr == addr)
> +			if (kretprobe_blacklist[i].addr == addr) {
> +				pr_err("kretprobes: can't probe at address in reject list\n");
>  				return -EINVAL;

return -EACCESS too.

Thank you,

> +			}
>  		}
>  	}
>  
> 
> base-commit: 2e38eb04c95e5546b71bb86ee699a891c7d212b5
> -- 
> 2.31.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-10 10:16 ` Masami Hiramatsu
@ 2021-06-10 17:33   ` Steven Rostedt
  2021-06-11  1:50     ` Masami Hiramatsu
  2021-06-11 13:55   ` Naveen N. Rao
  1 sibling, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2021-06-10 17:33 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Naveen N. Rao, linux-kernel, Peter Zijlstra, Aneesh Kumar K.V

On Thu, 10 Jun 2021 19:16:43 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hmm, Nack for this way, but I understand that is a problem.
> If you got the error in perf probe, which uses ftrace dynamic-event interface.
> In that case, the errors should not be output in the dmesg, but are reported
> via error_log in tracefs.
> And kprobes itself should return different error code to the caller, instead
> of printing error in dmesg. See below.

We should update perf to use libtracefs that also has an interface to
read the error_log file.

-- Steve

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-10 17:33   ` Steven Rostedt
@ 2021-06-11  1:50     ` Masami Hiramatsu
  0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2021-06-11  1:50 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Naveen N. Rao, linux-kernel, Peter Zijlstra, Aneesh Kumar K.V

On Thu, 10 Jun 2021 13:33:46 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 10 Jun 2021 19:16:43 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > Hmm, Nack for this way, but I understand that is a problem.
> > If you got the error in perf probe, which uses ftrace dynamic-event interface.
> > In that case, the errors should not be output in the dmesg, but are reported
> > via error_log in tracefs.
> > And kprobes itself should return different error code to the caller, instead
> > of printing error in dmesg. See below.
> 
> We should update perf to use libtracefs that also has an interface to
> read the error_log file.

Hmm, it seems that libtracefs has no parser for the error_log file.

What we need is to parse error_log and find appropriate entry of
the error log. Thus, the interface will be;

int tracefs_add_dynamic_event(const char *command, char **error_log);

And the usage will be;

ret = tracefs_add_dynamic_event(command, &error_buf);
if (ret < 0) {
	pr_error("Failed to add dynamic event: %d\n", ret);
	pr_error("Error log:\n%s\n", error_buf);
	free(error_buf);
	return ret;
}
...
This is because error_log file keeps some previous error logs and
you need to find appropriate one from the actual command.

Maybe a part of perf probe (util/probe-file.c) should be shared with
libtracefs and finally it should moved on the libtracefs.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-10 10:16 ` Masami Hiramatsu
  2021-06-10 17:33   ` Steven Rostedt
@ 2021-06-11 13:55   ` Naveen N. Rao
  2021-06-11 19:40     ` Steven Rostedt
  2021-06-12  1:13     ` Masami Hiramatsu
  1 sibling, 2 replies; 13+ messages in thread
From: Naveen N. Rao @ 2021-06-11 13:55 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Aneesh Kumar K.V, linux-kernel, Peter Zijlstra, Steven Rostedt

Hi Masami,
Thanks for the review.


Masami Hiramatsu wrote:
> Hi Naveen,
> 
> On Thu, 10 Jun 2021 14:26:17 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> When probing at different locations in the kernel, it is not always
>> evident if the location can be probed or not. As an example:
>> 
>>     $ perf probe __radix__flush_tlb_range:35
>>     Failed to write event: Invalid argument
>>       Error: Failed to add events.
>> 
>> The probed line above is:
>>      35         if (!mmu_has_feature(MMU_FTR_GTSE) && type == FLUSH_TYPE_GLOBAL) {
>> 
>> This ends up trying to probe on BUILD_BUG_ON(), which is rejected.
>> However, the user receives no indication at all as to why the probe
>> failed. Print an error in such cases so that it is clear that the probe
>> was rejected.
> 
> Hmm, Nack for this way, but I understand that is a problem.
> If you got the error in perf probe, which uses ftrace dynamic-event interface.
> In that case, the errors should not be output in the dmesg, but are reported
> via error_log in tracefs.

That would be a nice thing to add to perf, but I don't see why this 
should be a either/or. I still think it is good to have the core kprobe 
infrastructure print such errors in the kernel log. It is easier to look 
up such error strings in the kernel source to understand why a probe was 
rejected.

We also have perf_event_open() as an interface to add probes, and I 
don't think it would be helpful to require all tools to utilize the 
error log from tracefs for this purpose.


- Naveen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-11 13:55   ` Naveen N. Rao
@ 2021-06-11 19:40     ` Steven Rostedt
  2021-06-14 15:30       ` Naveen N. Rao
  2021-06-12  1:13     ` Masami Hiramatsu
  1 sibling, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2021-06-11 19:40 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Masami Hiramatsu, Aneesh Kumar K.V, linux-kernel, Peter Zijlstra

On Fri, 11 Jun 2021 19:25:38 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> We also have perf_event_open() as an interface to add probes, and I 
> don't think it would be helpful to require all tools to utilize the 
> error log from tracefs for this purpose.

The there should be a perf interface to read the errors. I agree with
Masami. Let's not have console logs for probe errors.

-- Steve

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-11 13:55   ` Naveen N. Rao
  2021-06-11 19:40     ` Steven Rostedt
@ 2021-06-12  1:13     ` Masami Hiramatsu
  2021-06-14 15:37       ` Naveen N. Rao
  1 sibling, 1 reply; 13+ messages in thread
From: Masami Hiramatsu @ 2021-06-12  1:13 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Aneesh Kumar K.V, linux-kernel, Peter Zijlstra, Steven Rostedt

Hi Naveen,

On Fri, 11 Jun 2021 19:25:38 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Hi Masami,
> Thanks for the review.
> 
> 
> Masami Hiramatsu wrote:
> > Hi Naveen,
> > 
> > On Thu, 10 Jun 2021 14:26:17 +0530
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> > 
> >> When probing at different locations in the kernel, it is not always
> >> evident if the location can be probed or not. As an example:
> >> 
> >>     $ perf probe __radix__flush_tlb_range:35
> >>     Failed to write event: Invalid argument
> >>       Error: Failed to add events.
> >> 
> >> The probed line above is:
> >>      35         if (!mmu_has_feature(MMU_FTR_GTSE) && type == FLUSH_TYPE_GLOBAL) {
> >> 
> >> This ends up trying to probe on BUILD_BUG_ON(), which is rejected.
> >> However, the user receives no indication at all as to why the probe
> >> failed. Print an error in such cases so that it is clear that the probe
> >> was rejected.
> > 
> > Hmm, Nack for this way, but I understand that is a problem.
> > If you got the error in perf probe, which uses ftrace dynamic-event interface.
> > In that case, the errors should not be output in the dmesg, but are reported
> > via error_log in tracefs.
> 
> That would be a nice thing to add to perf, but I don't see why this 
> should be a either/or. I still think it is good to have the core kprobe 
> infrastructure print such errors in the kernel log.

Yes, but that is only when if there is any unexpected errors.

For the expected error (e.g. rejecting user input), the design policy is
- kprobes API should return correct error code.
- kprobe tracefs I/F should return correct error code and put a human
  readable error mesage in the error_log.
Thus, the perf probe should decode the error code or reuse the error_log.

> It is easier to look 
> up such error strings in the kernel source to understand why a probe was 
> rejected.

I don't like to put a log message for rejecting user input on dmesg anymore.


> We also have perf_event_open() as an interface to add probes, and I 
> don't think it would be helpful to require all tools to utilize the 
> error log from tracefs for this purpose.

No, perf probe doesn't use perf-event interface to add probes. It uses
the tracefs for adding probes.

Thank you,


> 
> 
> - Naveen
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-11 19:40     ` Steven Rostedt
@ 2021-06-14 15:30       ` Naveen N. Rao
  2021-06-18 16:15         ` Masami Hiramatsu
  0 siblings, 1 reply; 13+ messages in thread
From: Naveen N. Rao @ 2021-06-14 15:30 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Aneesh Kumar K.V, linux-kernel, Masami Hiramatsu, Peter Zijlstra

Steven Rostedt wrote:
> On Fri, 11 Jun 2021 19:25:38 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> We also have perf_event_open() as an interface to add probes, and I 
>> don't think it would be helpful to require all tools to utilize the 
>> error log from tracefs for this purpose.
> 
> The there should be a perf interface to read the errors. I agree with
> Masami. Let's not have console logs for probe errors.

Ok, understood.


Thanks,
Naveen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-12  1:13     ` Masami Hiramatsu
@ 2021-06-14 15:37       ` Naveen N. Rao
  2021-06-15  5:42         ` Masami Hiramatsu
  0 siblings, 1 reply; 13+ messages in thread
From: Naveen N. Rao @ 2021-06-14 15:37 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Aneesh Kumar K.V, linux-kernel, Peter Zijlstra, Steven Rostedt

Masami Hiramatsu wrote:
> Hi Naveen,
> 
> On Fri, 11 Jun 2021 19:25:38 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> Hi Masami,
>> Thanks for the review.
>> 
>> 
>> Masami Hiramatsu wrote:
>> > Hi Naveen,
>> > 
>> > On Thu, 10 Jun 2021 14:26:17 +0530
>> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>> > 
>> >> When probing at different locations in the kernel, it is not always
>> >> evident if the location can be probed or not. As an example:
>> >> 
>> >>     $ perf probe __radix__flush_tlb_range:35
>> >>     Failed to write event: Invalid argument
>> >>       Error: Failed to add events.
>> >> 
>> >> The probed line above is:
>> >>      35         if (!mmu_has_feature(MMU_FTR_GTSE) && type == FLUSH_TYPE_GLOBAL) {
>> >> 
>> >> This ends up trying to probe on BUILD_BUG_ON(), which is rejected.
>> >> However, the user receives no indication at all as to why the probe
>> >> failed. Print an error in such cases so that it is clear that the probe
>> >> was rejected.
>> > 
>> > Hmm, Nack for this way, but I understand that is a problem.
>> > If you got the error in perf probe, which uses ftrace dynamic-event interface.
>> > In that case, the errors should not be output in the dmesg, but are reported
>> > via error_log in tracefs.
>> 
>> That would be a nice thing to add to perf, but I don't see why this 
>> should be a either/or. I still think it is good to have the core kprobe 
>> infrastructure print such errors in the kernel log.
> 
> Yes, but that is only when if there is any unexpected errors.
> 
> For the expected error (e.g. rejecting user input), the design policy is
> - kprobes API should return correct error code.
> - kprobe tracefs I/F should return correct error code and put a human
>   readable error mesage in the error_log.
> Thus, the perf probe should decode the error code or reuse the error_log.
> 
>> It is easier to look 
>> up such error strings in the kernel source to understand why a probe was 
>> rejected.
> 
> I don't like to put a log message for rejecting user input on dmesg anymore.

Understood.

> 
> 
>> We also have perf_event_open() as an interface to add probes, and I 
>> don't think it would be helpful to require all tools to utilize the 
>> error log from tracefs for this purpose.
> 
> No, perf probe doesn't use perf-event interface to add probes. It uses
> the tracefs for adding probes.

Yes, but I was referring to some of the bpf tools (bcc) that now use 
perf_event_open() interface.


Thanks,
Naveen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-14 15:37       ` Naveen N. Rao
@ 2021-06-15  5:42         ` Masami Hiramatsu
  0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2021-06-15  5:42 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Aneesh Kumar K.V, linux-kernel, Peter Zijlstra, Steven Rostedt

Hi Naveen,

On Mon, 14 Jun 2021 21:07:40 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> > 
> >> We also have perf_event_open() as an interface to add probes, and I 
> >> don't think it would be helpful to require all tools to utilize the 
> >> error log from tracefs for this purpose.
> > 
> > No, perf probe doesn't use perf-event interface to add probes. It uses
> > the tracefs for adding probes.
> 
> Yes, but I was referring to some of the bpf tools (bcc) that now use 
> perf_event_open() interface.

Yes, bpf chooses to use a special temporary perf_event, something like a
performance counter event. Those are hidden from tracefs and perf-tool.
The perf probe is the frontend of the trace event and perf event, which
will add dynamic events in tracefs, and perf will use it as same as
the other static events.

Thank you,




-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-14 15:30       ` Naveen N. Rao
@ 2021-06-18 16:15         ` Masami Hiramatsu
  2021-06-21  9:36           ` Naveen N. Rao
  0 siblings, 1 reply; 13+ messages in thread
From: Masami Hiramatsu @ 2021-06-18 16:15 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Steven Rostedt, Aneesh Kumar K.V, linux-kernel, Masami Hiramatsu,
	Peter Zijlstra

Hi Naveen,

On Mon, 14 Jun 2021 21:00:52 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Steven Rostedt wrote:
> > On Fri, 11 Jun 2021 19:25:38 +0530
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> > 
> >> We also have perf_event_open() as an interface to add probes, and I 
> >> don't think it would be helpful to require all tools to utilize the 
> >> error log from tracefs for this purpose.
> > 
> > The there should be a perf interface to read the errors. I agree with
> > Masami. Let's not have console logs for probe errors.
> 
> Ok, understood.

Will you update this?

Thank you,

> 
> 
> Thanks,
> Naveen
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-18 16:15         ` Masami Hiramatsu
@ 2021-06-21  9:36           ` Naveen N. Rao
  2021-06-21 12:54             ` Masami Hiramatsu
  0 siblings, 1 reply; 13+ messages in thread
From: Naveen N. Rao @ 2021-06-21  9:36 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Aneesh Kumar K.V, linux-kernel, Peter Zijlstra, Steven Rostedt, acme

Hi Masami,

Masami Hiramatsu wrote:
> Hi Naveen,
> 
> On Mon, 14 Jun 2021 21:00:52 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
>> Steven Rostedt wrote:
>> > On Fri, 11 Jun 2021 19:25:38 +0530
>> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>> > 
>> >> We also have perf_event_open() as an interface to add probes, and I 
>> >> don't think it would be helpful to require all tools to utilize the 
>> >> error log from tracefs for this purpose.
>> > 
>> > The there should be a perf interface to read the errors. I agree with
>> > Masami. Let's not have console logs for probe errors.
>> 
>> Ok, understood.
> 
> Will you update this?

Not sure if you are asking about error logging with perf_event_open(), 
or about reading error_log from tracefs.

For perf_event_open(), I think we will just need to return the error 
code, and interpret the return value in userspace. I don't think there 
is any other way to pass error log or strings back from the kernel.

I will update this series to return the correct return value as 
suggested.


Thanks,
Naveen


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] kprobes: Print an error if probe is rejected
  2021-06-21  9:36           ` Naveen N. Rao
@ 2021-06-21 12:54             ` Masami Hiramatsu
  0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2021-06-21 12:54 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Aneesh Kumar K.V, linux-kernel, Peter Zijlstra, Steven Rostedt, acme

On Mon, 21 Jun 2021 15:06:24 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Hi Masami,
> 
> Masami Hiramatsu wrote:
> > Hi Naveen,
> > 
> > On Mon, 14 Jun 2021 21:00:52 +0530
> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> > 
> >> Steven Rostedt wrote:
> >> > On Fri, 11 Jun 2021 19:25:38 +0530
> >> > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> >> > 
> >> >> We also have perf_event_open() as an interface to add probes, and I 
> >> >> don't think it would be helpful to require all tools to utilize the 
> >> >> error log from tracefs for this purpose.
> >> > 
> >> > The there should be a perf interface to read the errors. I agree with
> >> > Masami. Let's not have console logs for probe errors.
> >> 
> >> Ok, understood.
> > 
> > Will you update this?
> 
> Not sure if you are asking about error logging with perf_event_open(), 
> or about reading error_log from tracefs.
> 
> For perf_event_open(), I think we will just need to return the error 
> code, and interpret the return value in userspace. I don't think there 
> is any other way to pass error log or strings back from the kernel.

Yeah, I think that's good idea.

> 
> I will update this series to return the correct return value as 
> suggested.

OK, then I will update tracefs to interpret the code and put appropriate
error message on error_log.

Thank you!

> 
> 
> Thanks,
> Naveen
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-06-21 12:54 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-10  8:56 [PATCH] kprobes: Print an error if probe is rejected Naveen N. Rao
2021-06-10 10:16 ` Masami Hiramatsu
2021-06-10 17:33   ` Steven Rostedt
2021-06-11  1:50     ` Masami Hiramatsu
2021-06-11 13:55   ` Naveen N. Rao
2021-06-11 19:40     ` Steven Rostedt
2021-06-14 15:30       ` Naveen N. Rao
2021-06-18 16:15         ` Masami Hiramatsu
2021-06-21  9:36           ` Naveen N. Rao
2021-06-21 12:54             ` Masami Hiramatsu
2021-06-12  1:13     ` Masami Hiramatsu
2021-06-14 15:37       ` Naveen N. Rao
2021-06-15  5:42         ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).