linux-kernel - Re: [PATCH] perf tools: Add missing else to cmd

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]

Message-ID: <CAM9d7chMGzEJ3GKyxXD-RwUpjje7u3Cp810TRD+ZGgCT+mFrEQ@mail.gmail.com>
Date:   Mon, 26 Jun 2023 13:58:48 -0700
From:   Namhyung Kim <namhyung@...nel.org>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Ingo Molnar <mingo@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        linux-perf-users@...r.kernel.org
Subject: Re: [PATCH] perf tools: Add missing else to cmd_daemon subcommand condition

Hi Jiri,

On Mon, Jun 26, 2023 at 1:16 PM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Namhyung reported segfault in perf daemon start command.
>
> It's caused by extra check on argv[0] which is set to NULL by previous
> __cmd_start call. Adding missing else to skip the extra check.
>
> Fixes: 92294b906e6c ("perf daemon: Dynamically allocate path to perf")
> Reported-by: Namhyung Kim <namhyung@...nel.org>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>

Thanks for the fix.  Now it runs ok.

Before:
  $ sudo ./perf test -v daemon
   85: daemon operations                                               :
  --- start ---
  test child forked, pid 82420
  test daemon list
  ./tests/shell/daemon.sh: line 133: 82426 Segmentation fault
perf daemon start --config ${config}
  test daemon reconfig
  ./tests/shell/daemon.sh: line 133: 82520 Segmentation fault
perf daemon start --config ${config}
  test daemon stop
  ./tests/shell/daemon.sh: line 133: 82636 Segmentation fault
perf daemon start --config ${config}
  test daemon signal
  ./tests/shell/daemon.sh: line 133: 82674 Segmentation fault
perf daemon start --config ${config}
  signal 12 sent to session 'test [82676]'
  signal 12 sent to session 'test [82676]'
  test daemon ping
  ./tests/shell/daemon.sh: line 133: 82702 Segmentation fault
perf daemon start --config ${config}
  test daemon lock
  ./tests/shell/daemon.sh: line 133: 82734 Segmentation fault
perf daemon start --config ${config}
  test child finished with 0
  ---- end ----
  daemon operations: Ok

Maybe we need to investigate more why it was ok..
But at least I don't see segfaults anymore


After:
   85: daemon operations                                               :
  --- start ---
  test child forked, pid 80752
  test daemon list
  test daemon reconfig
  test daemon stop
  test daemon signal
  signal 12 sent to session 'test [81022]'
  signal 12 sent to session 'test [81022]'
  test daemon ping
  test daemon lock
  test child finished with 0
  ---- end ----
  daemon operations: Ok


Tested-by: Namhyung Kim <namhyung@...nel.org>

Thanks,
Namhyung



> ---
>  tools/perf/builtin-daemon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index f5674d824a40..83954af36753 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -1524,7 +1524,7 @@ int cmd_daemon(int argc, const char **argv)
>         if (argc) {
>                 if (!strcmp(argv[0], "start"))
>                         ret = __cmd_start(&__daemon, daemon_options, argc, argv);
> -               if (!strcmp(argv[0], "signal"))
> +               else if (!strcmp(argv[0], "signal"))
>                         ret = __cmd_signal(&__daemon, daemon_options, argc, argv);
>                 else if (!strcmp(argv[0], "stop"))
>                         ret = __cmd_stop(&__daemon, daemon_options, argc, argv);
> --
> 2.41.0
>