Messages in this thread | | | Date | Mon, 14 Jun 2021 21:07:40 +0530 | From | "Naveen N. Rao" <> | Subject | Re: [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
| |