netdev - Re: [PATCH v2 bpf-next 1/5] ftrace: allow customized flags for ftrace_direct_multi 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-prev] [day] [month] [year] [list]

Message-ID: <20220714092215.149d4823@gandalf.local.home>
Date:   Thu, 14 Jul 2022 09:22:15 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Song Liu <songliubraving@...com>
Cc:     Song Liu <song@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "andrii@...nel.org" <andrii@...nel.org>,
        Kernel Team <Kernel-team@...com>,
        "jolsa@...nel.org" <jolsa@...nel.org>,
        "mhiramat@...nel.org" <mhiramat@...nel.org>
Subject: Re: [PATCH v2 bpf-next 1/5] ftrace: allow customized flags for
 ftrace_direct_multi ftrace_ops

On Thu, 14 Jul 2022 04:37:43 +0000
Song Liu <songliubraving@...com> wrote:

> >   
> >> 
> >> non-direct ops without IPMODIFY can already share with IPMODIFY ops.  
> > 
> > It can? ftrace sets IPMODIFY for all DIRECT callers to prevent that. Except
> > for this patch that removes that restriction (which I believe is broken).  
> 
> I mean "non-direct" ftrace ops, not direct ftrace ops. 

Ah, sorry misunderstood that.


> > Let me start from the beginning.  
> 
> I got your point now. We replace the flag on direct trampoline with a 
> callback check. So yes, this works. 

I'm glad we are on the same page :-)


> > 9. ftrace sees the lkp IPMODIFY ops has SHARED_IPMODIFY on it, and knows
> >   that there's a direct call here too. It removes the IPMODIFY ops, and
> >   then calls the direct ops->ops_func(STOP_SHARE_WITH_IPMODIFY) to let the
> >   direct code know that it is no longer sharing with an IPMODIFY such that
> >   it can change to call the function directly and not use the stack.  
> 
> I wonder whether we still need this flag. Alternatively, we can always
> find direct calls on the function and calls ops_func(STOP_SHARE_WITH_IPMODIFY). 

Actually we don't need the new flag and we don't need to always search. When
a direct is attached to the function then the rec->flags will have
FTRACE_FL_DIRECT attached to it.

Then if an IPMODIFY is being removed and the rec->flags has
FTRACE_FL_DIRECT set, then we know to search the ops for the one that has a
DIRECT flag attached and we can call the ops_func() on that one.

We should also add a FTRACE_WARN_ON() if a direct is not found but the flag
was set.

> 
> What do you think about this? 
>

I think this works.

Also, on the patch that implements this in the next version, please add to
the change log:

Link: https://lore.kernel.org/all/20220602193706.2607681-2-song@kernel.org/

so that we have a link to this discussion.

Thanks,

-- Steve