linux-kernel - Re: [PATCH 1/9] perf/jit: improve error messages from JVMTI

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, 17 Oct 2016 06:52:36 -0700
From:   Stephane Eranian <eranian@...gle.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Nilay Vaish <nilayvaish@...il.com>,
        Linux Kernel list <linux-kernel@...r.kernel.org>,
        Jiri Olsa <jolsa@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Anton Blanchard <anton@...abs.org>,
        Namhyung Kim <namhyung@...nel.org>
Subject: Re: [PATCH 1/9] perf/jit: improve error messages from JVMTI

On Fri, Oct 14, 2016 at 8:20 AM, Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
> Em Fri, Oct 14, 2016 at 05:57:25AM -0700, Stephane Eranian escreveu:
>> On Fri, Oct 14, 2016 at 4:13 AM, Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
>> > Em Thu, Oct 13, 2016 at 03:05:40PM -0500, Nilay Vaish escreveu:
>> >> Do we not need to release the memory for err_msg if the condition for
>> >> the 'if' statement evaluates to false?  Is it that we are going to
>> >> kill the process, so no need to release the memory?
>
>> > I guess that print_error() is called only when an error was returned
>> > somewhere, that ret parameter, then if there was no error
>> > (JVMTI_ERROR_NONE) in translating that numeric code to an string,
>> > err_msg, it can then be used with warnx() (the main purpose of
>> > print_error()) and then deallocated.
>> >
>> > For err != JVMTI_ERROR_NONE it silently goes back to the caller that
>> > expected it to print something.
>> >
>> > I.e. probably it should have an else clause, something like:
>> >
>> >         if (err == JVMTI_ERROR_NONE) {
>> >                 warnx("%s failed with %s", msg, err_msg);
>> >                 (*jvmti)->Deallocate(jvmti, (unsigned char *)err_msg);
>> >         } else {
>> >                 warnx("%s failed with an unknown error %d", msg, (int)ret);
>> >         }
>> >
>> > Stephane?
>> I will fix all of the comments over the week-end. I am away from office today.
>
> I have almost all of them fixed, will fix this one too, if you agree
> with the analysis.
>
Ok, the proposed fix is correct.
Thanks for fixing issues.

Now, I am tracking down another issue with the injection of jitdump.
It seems something goes wrong with ordering and timestamps and some
valid jitdump samples are not symbolized. It is not clear what is
causing this yet. It may be the issue raised by Adrian a while ago
about the finished_round processing.


> - Arnaldo