lkml.org 
[lkml]   [2021]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] kprobes: Print an error if probe is rejected
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

\
 
 \ /
  Last update: 2021-06-14 17:39    [W:0.255 / U:1.800 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site