linux-kernel - Re: [PATCH] kprobes: Print an error if probe is rejected

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 Jun 2021 21:07:40 +0530
From:   "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>,
        linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Steven Rostedt <rostedt@...dmis.org>
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@...ux.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@...ux.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