linux-kernel - Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in echo

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, 11 May 2020 18:27:52 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Xiao Yang <yangx.jy@...fujitsu.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Shuah Khan <shuah@...nel.org>,
        <linux-kselftest@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Tom Zanussi <tom.zanussi@...ux.intel.com>,
        Li Philip <philip.li@...el.com>,
        Liu Yiding <yidingx.liu@...el.com>
Subject: Re: [PATCH 3/3] selftests/ftrace: Use /bin/echo instead of built-in
 echo

On Mon, 11 May 2020 15:22:25 +0800
Xiao Yang <yangx.jy@...fujitsu.com> wrote:

> On 2020/5/7 17:15, Masami Hiramatsu wrote:
> > On Thu, 7 May 2020 14:45:16 +0800
> > Xiao Yang<yangx.jy@...fujitsu.com>  wrote:
> >
> >> On 2020/5/1 21:38, Masami Hiramatsu wrote:
> >>> Since the built-in echo has different behavior in POSIX shell
> >>> (dash) and bash, we forcibly use /bin/echo -E (not interpret
> >>> backslash escapes) by default.
> >>>
> >>> This also fixes some test cases which expects built-in
> >>> echo command.
> >>>
> >>> Reported-by: Liu Yiding<yidingx.liu@...el.com>
> >>> Signed-off-by: Masami Hiramatsu<mhiramat@...nel.org>
> >>> ---
> >>>    tools/testing/selftests/ftrace/test.d/functions    |    3 +++
> >>>    .../test.d/trigger/trigger-trace-marker-hist.tc    |    2 +-
> >>>    .../trigger-trace-marker-synthetic-kernel.tc       |    4 ++++
> >>>    .../trigger/trigger-trace-marker-synthetic.tc      |    4 ++--
> >>>    4 files changed, 10 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> >>> index 5d4550591ff9..ea59b6ea2c3e 100644
> >>> --- a/tools/testing/selftests/ftrace/test.d/functions
> >>> +++ b/tools/testing/selftests/ftrace/test.d/functions
> >>> @@ -1,3 +1,6 @@
> >>> +# Since the built-in echo has different behavior in POSIX shell (dash) and
> >>> +# bash, we forcibly use /bin/echo -E (not interpret backslash escapes).
> >>> +alias echo="/bin/echo -E"
> >> Hi Masami, Steven
> >>
> >> It seems that only kprobe_syntax_errors.tc is impacted by the issue
> >> currently.  Is it necessary for all tests to use /bin/echo and could we
> >> just make kprobe_syntax_errors.tc use /bin/echo?
> >
> > Yes, I would like to unify the "echo"'s behavior among the testcases
> > instead of patching each failure in the future.
> > Or would you have any concern on it?
> Hi Masami,
> 
> Very sorry for the late reply.
> 
> We may not avoid fixing related failures after your change:
> 1) We have to reuse built-in echo (do alias echo=echo) if we want to 
> test common_pid for histogram.
> 2) We have to reuse built-in echo if some new tests want to interpret 
> backslash escapes in future.

1) yes, that's what I need to do for avoiding "pid" key histogram
  (but I think we should have better way to test it)
2) No, in that case you should use "/bin/echo -e" explicitly.
   dash's built-in echo doesn't support it.

> Is it simple to provide two implementations of echo?(built-in echo and 
> echo command?) and then just apply echo command for kprobe_syntax_errors.tc?

Hmm, OK, there might be another reason we reconsider this patch.

- Alisasing echo (this patch) can avoid dash related issues but
  this also makes "echo" running in another process implicitly.

- Using /bin/echo for backslash explicitly will be missed unless
  user runs it on dash, but it will keep "echo" in same process.

So both have pros/cons, but your idea will be locally effected.
OK, I'll retry it.

Thank you,

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