linux-kernel - Re: [PATCH] perf: support running perf binaries with a dash in their name

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 Sep 2017 07:08:18 -0700
From:   David Ahern <dsahern@...il.com>
To:     Milian Wolff <milian.wolff@...b.com>, acme@...nel.org
Cc:     Linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Yao Jin <yao.jin@...ux.intel.com>
Subject: Re: [PATCH] perf: support running perf binaries with a dash in their
 name

On 9/11/17 4:14 AM, Milian Wolff wrote:
> Previously the part behind "perf-" was interpreted as an internal
> perf command. If the suffix could not be handled, the execution
> was stopped. This makes it impossible to launch perf binaries that
> got renamed to have the `perf-` prefix. This is e.g. the case for
> appimages (e.g. "perf-x86_64.AppImage"), but would also apply to
> all other scenarios where users symlink or rename perf themselves:
> 
...
> ~~~~~
> $ ./perf-custom-suffix list
> 
> List of pre-defined events (to be used in -e):
> ...
> ~~~~~
> 
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Yao Jin <yao.jin@...ux.intel.com>
> Signed-off-by: Milian Wolff <milian.wolff@...b.com>
> ---
>  tools/perf/perf.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index e0279babe0c0..7a3a39014446 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -467,15 +467,19 @@ int main(int argc, const char **argv)
>  	 *  - cannot execute it externally (since it would just do
>  	 *    the same thing over again)
>  	 *
> -	 * So we just directly call the internal command handler, and
> -	 * die if that one cannot handle it.
> +	 * So we just directly call the internal command handler. If that one
> +	 * fails to handle this, then maybe we just run a renamed perf binary
> +	 * that contains a dash in its name. To handle this scenario, we just
> +	 * fall through and ignore the "xxxx" part of the command string.
>  	 */
>  	if (strstarts(cmd, "perf-")) {
>  		cmd += 5;
>  		argv[0] = cmd;
>  		handle_internal_command(argc, argv);
> -		fprintf(stderr, "cannot handle %s internally", cmd);
> -		goto out;
> +		// if the command is handled, the above function does not return
> +		// undo changes and fall through in such a case

Those should be /* */ not //

> +		cmd -= 5;
> +		argv[0] = cmd;
>  	}
>  	if (strstarts(cmd, "trace")) {
>  #ifdef HAVE_LIBAUDIT_SUPPORT
> 

other than that LGTM and long over due.

Acked-by: David Ahern <dsahern@...il.com>