linux-kernel - Re: [PATCH] kernel/trace: Add TRACING_ALLOW

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: <20200630083912.40a6c50d@oasis.local.home>
Date:   Tue, 30 Jun 2020 08:39:11 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Nicolas Boichat <drinkcat@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Peter Zijlstra <peterz@...radead.org>,
        Vinod Koul <vkoul@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Thomas Gleixner <tglx@...utronix.de>,
        "Guilherme G . Piccoli" <gpiccoli@...onical.com>,
        Will Deacon <will@...nel.org>,
        Douglas Anderson <dianders@...omium.org>,
        Guenter Roeck <groeck@...omium.org>, bpf@...r.kernel.org
Subject: Re: [PATCH] kernel/trace: Add TRACING_ALLOW_PRINTK config option

On Mon, 29 Jun 2020 22:16:59 -0700
Alexei Starovoitov <alexei.starovoitov@...il.com> wrote:

> > 
> > Warning, not even compiled tested.  
> 
> Thanks! I see what you mean now.

Great! :-)

> 
> > 
> > -- Steve
> > 
> > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
> > index 6575bb0a0434..aeba5ee7325a 100644
> > --- a/kernel/trace/Makefile
> > +++ b/kernel/trace/Makefile
> > @@ -31,6 +31,8 @@ ifdef CONFIG_GCOV_PROFILE_FTRACE
> >  GCOV_PROFILE := y
> >  endif
> >  
> > +CFLAGS_bpf_trace.o := -I$(src)  
> 
> not following. why this is needed?

It's required in order to have the TRACE_EVENT macro magic work. More
info about it here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/trace_events/Makefile


> 
> > +
> >  CFLAGS_trace_benchmark.o := -I$(src)
> >  CFLAGS_trace_events_filter.o := -I$(src)
> >  
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index dc05626979b8..01bedf335b2e 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -19,6 +19,9 @@
> >  #include "trace_probe.h"
> >  #include "trace.h"
> >  
> > +#define CREATE_TRACE_EVENTS  
> 
> CREATE_TRACE_POINTS ?


Doh, yeah. I did say it wasn't even compiled tested ;-)

> 
> > +#include "bpf_trace.h"
> > +
> >  #define bpf_event_rcu_dereference(p)					\
> >  	rcu_dereference_protected(p, lockdep_is_held(&bpf_event_mutex))
> >  
> > @@ -473,13 +476,29 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> >  		fmt_cnt++;
> >  	}
> >  
> > +static DEFINE_SPINLOCK(trace_printk_lock);
> > +#define BPF_TRACE_PRINTK_SIZE	1024
> > +
> > +static inline void do_trace_printk(const char *fmt, ...)
> > +{
> > +	static char buf[BPF_TRACE_PRINT_SIZE];
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&trace_printk_lock, flags);
> > +	va_start(ap, fmt);
> > +	vsnprintf(buf, BPF_TRACE_PRINT_SIZE, fmt, ap);
> > +	va_end(ap);
> > +
> > +	trace_bpf_trace_printk(buf);
> > +	spin_unlock_irqrestore(&trace_printk_lock, flags);  
> 
> interesting. I don't think anyone would care about spin_lock overhead.
> It's better because 'trace_bpf_trace_printk' would be a separate event
> that can be individually enabled/disabled?
> I guess it can work.
> Thanks!

I hope this does everything you need for bpf_trace_printk. If there's
something  that's not working for you, PLEASE reach out to me and ask
what you need.

Cheers!

-- Steve