netdev - Re: [RFC][PATCH 1/2] ftrace: Add modify_ftrace

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-next>] [day] [month] [year] [list]

Date:   Mon, 18 Nov 2019 22:04:28 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>, bpf <bpf@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [RFC][PATCH 1/2] ftrace: Add modify_ftrace_direct()

On Sun, Nov 17, 2019 at 2:18 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Fri, 15 Nov 2019 13:51:26 -0800
> Alexei Starovoitov <alexei.starovoitov@...il.com> wrote:
>
> > Thanks a lot for implementing it.
> > Switching to iterator just to modify the call.. hmm.
> > So "call direct_bpf_A" gets replaced to "call ftrace_stub" to do the iterator
> > only to patch "call direct_bpf_B" later. I'm struggling to see why do that when
> > arch can provide call to call rewrite easily. x86 and others have such ability
> > already. I don't understand why you want to sacrifice simplicity here.
> > Anyway with all 3 apis (register, modify, unreg) it looks complete.
> > I'll start playing with it on Monday.
>
> Now if you take my latest for-next branch, and add the patch below,

I took your for-next without the extra patch and used it from bpf trampoline.
It's looking good so far. Passed basic testing. Will add more stress tests.

Do you mind doing:
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 73eb2e93593f..6ddb203ca550 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -256,16 +256,16 @@ struct ftrace_direct_func
*ftrace_find_direct_func(unsigned long addr);
 # define ftrace_direct_func_count 0
 static inline int register_ftrace_direct(unsigned long ip, unsigned long addr)
 {
-       return -ENODEV;
+       return -ENOTSUPP;
 }
 static inline int unregister_ftrace_direct(unsigned long ip, unsigned
long addr)
 {
-       return -ENODEV;
+       return -ENOTSUPP;
 }
 static inline int modify_ftrace_direct(unsigned long ip,
                                       unsigned long old_addr,
unsigned long new_addr)
 {
-       return -ENODEV;
+       return -ENOTSUPP;
 }

otherwise ENODEV is a valid error when ip is incorrect which is
indistinguishable from ftrace not compiled in.