linux-kernel - Re: [RFC][PATCH] ftrace/selftests: Add binary test to verify ring buffer timestamps

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:   Wed, 9 Dec 2020 08:39:54 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Shuah Khan <skhan@...uxfoundation.org>,
        Linux Trace Devel <linux-trace-devel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: [RFC][PATCH] ftrace/selftests: Add binary test to verify ring
 buffer timestamps

On Tue, 8 Dec 2020 10:00:24 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:

> On Tue, 8 Dec 2020 21:14:11 +0900
> Masami Hiramatsu <mhiramat@...nel.org> wrote:
> 
> > On Tue, 1 Dec 2020 16:06:56 -0500
> > Steven Rostedt <rostedt@...dmis.org> wrote:
> > 
> > > From: Steven Rostedt (VMware) <rostedt@...dmis.org>
> > > 
> > > A bug was reported about the ftrace ring buffer going backwards:
> > > 
> > > Link: https://lore.kernel.org/r/20201124223917.795844-1-elavila@google.com
> > > 
> > > In debugging this code, I wrote a C program that uses libtracefs to enable
> > > all events and function tracing (if it exits), and then read the raw ring
> > > buffer binary data and make sure that all the events never go backwards. If
> > > they do, then it does a dump of the ring buffer sub buffer page and shows
> > > the layout of the events and their deltas.
> > > 
> > > This was a very useful tool, and can be used to make sure that the ring
> > > buffer's timestamps are consistently monotonic.  
> > 
> > Yeah, this sounds good to me.
> > 
> > > 
> > > Adding this to the ftrace selftests seems to be a way that this can be
> > > tested more often. But this would introduce the first binary code to the
> > > ftracetests.  
> > 
> > No problem. I think it is better to be placed under ftracetest/bin/ and
> > add a PATH before running testcases.
> 
> OK.
> 
> > 
> > > To make sure that the tests still work on embedded devices (where a
> > > compiler may not even exist), and also since this binary incorporates the
> > > yet-to-be-released libtracefs library, if the make fails, the test exits
> > > with UNTESTED. The UNTESTED is documented as being a place holder which
> > > this would be if the make does not work.  
> > 
> > Hmm, in some embedded environment, we may not have make nor gcc.
> > So it would be better to be build in the kselftest build process as similar
> > to the other kselftests.
> > 
> > What about putting the source code under ftracetest/src/. For the embedded
> > devices, it can be built with cross-tools (and make it static binary if
> > needed) and install it under ftracetest/bin. If we have no cross-build
> > tool etc. we just skip building the binary under ftracetest/bin. And if 
> > the testcase finds there is no binary, it just returns UNRESOLVED or UNTESTED.
> > 
> 
> OK. I'll look at how to make this for both cases (embedded and not).
> Because, my current case is to copy the selftests to the machine and run
> them there. So my use case requires the build to happen at test time. But I
> can make it where it wont build if the binary already exists.

For that case, what about just "make clean" before copy, then the binary
will be removed? 

> 
> > (currently I returns UNRESOLVED when the test target kmodule is not found)
> 
> I used UNTESTED for a couple of reasons. I figured "UNRESOLVED" was for
> lack of kernel features or modules. But this is not a lack of the kernel,
> but a lack of user space. If something in user space is lacking (a tool,
> library, or binary), then I thought UNTESTED would be a better option. But
> if you have a strong opinion on it, I'll change it to UNRESOLVED, otherwise
> I'll keep UNTESTED.

The idea of UNTESTED/UNRESOLVED (and UNSUPPORTED) came from POSIX standard,
it is expained in dejagnu's manual:

https://www.gnu.org/software/dejagnu/manual/A-POSIX-Conforming-Test-Framework.html

In this case, user can build the user space binary for the environment but
does't, so I think UNRESOLVED will fit to that case.

So strictly speaking, UNTESTED is just a placeholder which will be implemented
in the future. (hmm, it will be a good chance to write a document for it)

Thank you,


-- 
Masami Hiramatsu <mhiramat@...nel.org>