| 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
| ||
|
Message-ID: <9003cbc9-bc53-4fb7-a35e-d416c22df4ca@linaro.org>
Date: Wed, 11 Dec 2024 08:39:55 +0000
From: James Clark <james.clark@...aro.org>
To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
Masami Hiramatsu <mhiramat@...nel.org>
Cc: linux-perf-users <linux-perf-users@...r.kernel.org>,
Namhyung Kim <namhyung@...nel.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>,
"Liang, Kan" <kan.liang@...ux.intel.com>, Leo Yan <leo.yan@....com>,
Dima Kogan <dima@...retsauce.net>, "Dr. David Alan Gilbert"
<linux@...blig.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] perf probe: Fix uninitialized variable
On 11/12/2024 12:43 am, Arnaldo Carvalho de Melo wrote:
> On Tue, Dec 10, 2024, 7:55 PM Masami Hiramatsu <mhiramat@...nel.org> wrote:
>
>> Hi,
>>
>> On Mon, 9 Dec 2024 17:12:21 +0000
>> James Clark <james.clark@...aro.org> wrote:
>>
>>> Since the linked fixes: commit, err is returned uninitialized due to the
>>> removal of "return 0". Initialize err to fix it, and rename err to out
>>> to avoid confusion because buf is still supposed to be freed in non
>>> error cases.
>>>
>>> This fixes the following intermittent test failure on release builds:
>>>
>>> $ perf test "testsuite_probe"
>>> ...
>>> -- [ FAIL ] -- perf_probe :: test_invalid_options :: mutually exclusive
>> options :: -L foo -V bar (output regexp parsing)
>>> Regexp not found: \"Error: switch .+ cannot be used with switch .+\"
>>> ...
>>>
>>> Fixes: 080e47b2a237 ("perf probe: Introduce quotation marks support")
>>> Signed-off-by: James Clark <james.clark@...aro.org>
>>> ---
>>> tools/perf/util/probe-event.c | 20 ++++++++++----------
>>> 1 file changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/tools/perf/util/probe-event.c
>> b/tools/perf/util/probe-event.c
>>> index 6d51a4c98ad7..35af6570cf9b 100644
>>> --- a/tools/perf/util/probe-event.c
>>> +++ b/tools/perf/util/probe-event.c
>>> @@ -1370,7 +1370,7 @@ int parse_line_range_desc(const char *arg, struct
>> line_range *lr)
>>> {
>>> char *buf = strdup(arg);
>>> char *p;
>>> - int err;
>>> + int err = 0;
>>
>> I think only this is required, and others are just cleanups by renaming
>> err -> out (usually for-next).
>> But Arnaldo is OK to combine these changes, I'm OK too.
>>
>> Acked-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
>>
>
> I agree, almost mentioned that, but since you mentioned this, yes, I'd
> prefer to have as small as possible a patch.
>
> - Arnaldo
>
>
Sure, I'll split it
>> Thank you,
>>
>>>
>>> if (!buf)
>>> return -ENOMEM;
>>> @@ -1383,20 +1383,20 @@ int parse_line_range_desc(const char *arg,
>> struct line_range *lr)
>>> if (p == buf) {
>>> semantic_error("No file/function name in '%s'.\n",
>> p);
>>> err = -EINVAL;
>>> - goto err;
>>> + goto out;
>>> }
>>> *(p++) = '\0';
>>>
>>> err = parse_line_num(&p, &lr->start, "start line");
>>> if (err)
>>> - goto err;
>>> + goto out;
>>>
>>> if (*p == '+' || *p == '-') {
>>> const char c = *(p++);
>>>
>>> err = parse_line_num(&p, &lr->end, "end line");
>>> if (err)
>>> - goto err;
>>> + goto out;
>>>
>>> if (c == '+') {
>>> lr->end += lr->start;
>>> @@ -1416,11 +1416,11 @@ int parse_line_range_desc(const char *arg,
>> struct line_range *lr)
>>> if (lr->start > lr->end) {
>>> semantic_error("Start line must be smaller"
>>> " than end line.\n");
>>> - goto err;
>>> + goto out;
>>> }
>>> if (*p != '\0') {
>>> semantic_error("Tailing with invalid str '%s'.\n",
>> p);
>>> - goto err;
>>> + goto out;
>>> }
>>> }
>>>
>>> @@ -1431,7 +1431,7 @@ int parse_line_range_desc(const char *arg, struct
>> line_range *lr)
>>> lr->file = strdup_esq(p);
>>> if (lr->file == NULL) {
>>> err = -ENOMEM;
>>> - goto err;
>>> + goto out;
>>> }
>>> }
>>> if (*buf != '\0')
>>> @@ -1439,7 +1439,7 @@ int parse_line_range_desc(const char *arg, struct
>> line_range *lr)
>>> if (!lr->function && !lr->file) {
>>> semantic_error("Only '@*' is not allowed.\n");
>>> err = -EINVAL;
>>> - goto err;
>>> + goto out;
>>> }
>>> } else if (strpbrk_esq(buf, "/."))
>>> lr->file = strdup_esq(buf);
>>> @@ -1448,10 +1448,10 @@ int parse_line_range_desc(const char *arg,
>> struct line_range *lr)
>>> else { /* Invalid name */
>>> semantic_error("'%s' is not a valid function name.\n",
>> buf);
>>> err = -EINVAL;
>>> - goto err;
>>> + goto out;
>>> }
>>>
>>> -err:
>>> +out:
>>> free(buf);
>>> return err;
>>> }
>>> --
>>> 2.34.1
>>>
>>
>>
>> --
>> Masami Hiramatsu (Google) <mhiramat@...nel.org>
>>
>
Powered by blists - more mailing lists