All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Dario Petrillo <dario.pk1@gmail.com>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Mark Rutland <mark.rutland@arm.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	stable@kernel.org
Subject: [PATCH 5.15 19/28] perf annotate: Avoid TUI crash when navigating in the annotation of recursive functions
Date: Tue, 18 Jan 2022 17:06:05 +0100	[thread overview]
Message-ID: <20220118160452.511243269@linuxfoundation.org> (raw)
In-Reply-To: <20220118160451.879092022@linuxfoundation.org>

From: Dario Petrillo <dario.pk1@gmail.com>

commit d5962fb7d69073bf68fb647531cfd4f0adf84be3 upstream.

In 'perf report', entering a recursive function from inside of itself
(either directly of indirectly through some other function) results in
calling symbol__annotate2 multiple() times, and freeing the whole
disassembly when exiting from the innermost instance.

The first issue causes the function's disassembly to be duplicated, and
the latter a heap use-after-free (and crash) when trying to access the
disassembly again.

I reproduced the bug on perf 5.11.22 (Ubuntu 20.04.3 LTS) and 5.16.rc8
with the following testcase (compile with gcc recursive.c -o recursive).
To reproduce:

- perf record ./recursive
- perf report
- enter fibonacci and annotate it
- move the cursor on one of the "callq fibonacci" instructions and press enter
  - at this point there will be two copies of the function in the disassembly
- go back by pressing q, and perf will crash

  #include <stdio.h>

  int fibonacci(int n)
  {
      if(n <= 2) return 1;
      return fibonacci(n-1) + fibonacci(n-2);
  }

  int main()
  {
      printf("%d\n", fibonacci(40));
  }

This patch addresses the issue by annotating a function and freeing the
associated memory on exit only if no annotation is already present, so
that a recursive function is only annotated on entry.

Signed-off-by: Dario Petrillo <dario.pk1@gmail.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: stable@kernel.org
Link: http://lore.kernel.org/lkml/20220109234441.325106-1-dario.pk1@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 tools/perf/ui/browsers/annotate.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -966,6 +966,7 @@ int symbol__tui_annotate(struct map_symb
 		.opts = opts,
 	};
 	int ret = -1, err;
+	int not_annotated = list_empty(&notes->src->source);
 
 	if (sym == NULL)
 		return -1;
@@ -973,13 +974,15 @@ int symbol__tui_annotate(struct map_symb
 	if (ms->map->dso->annotate_warned)
 		return -1;
 
-	err = symbol__annotate2(ms, evsel, opts, &browser.arch);
-	if (err) {
-		char msg[BUFSIZ];
-		ms->map->dso->annotate_warned = true;
-		symbol__strerror_disassemble(ms, err, msg, sizeof(msg));
-		ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
-		goto out_free_offsets;
+	if (not_annotated) {
+		err = symbol__annotate2(ms, evsel, opts, &browser.arch);
+		if (err) {
+			char msg[BUFSIZ];
+			ms->map->dso->annotate_warned = true;
+			symbol__strerror_disassemble(ms, err, msg, sizeof(msg));
+			ui__error("Couldn't annotate %s:\n%s", sym->name, msg);
+			goto out_free_offsets;
+		}
 	}
 
 	ui_helpline__push("Press ESC to exit");
@@ -994,9 +997,11 @@ int symbol__tui_annotate(struct map_symb
 
 	ret = annotate_browser__run(&browser, evsel, hbt);
 
-	annotated_source__purge(notes->src);
+	if(not_annotated)
+		annotated_source__purge(notes->src);
 
 out_free_offsets:
-	zfree(&notes->offsets);
+	if(not_annotated)
+		zfree(&notes->offsets);
 	return ret;
 }



  parent reply	other threads:[~2022-01-18 16:10 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-18 16:05 [PATCH 5.15 00/28] 5.15.16-rc1 review Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 01/28] devtmpfs regression fix: reconfigure on each mount Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 02/28] drm/amd/display: explicitly set is_dsc_supported to false before use Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 03/28] orangefs: Fix the size of a memory allocation in orangefs_bufmap_alloc() Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 04/28] remoteproc: qcom: pil_info: Dont memcpy_toio more than is provided Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 05/28] vfs: fs_context: fix up param length parsing in legacy_parse_param Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 06/28] perf: Protect perf_guest_cbs with RCU Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 07/28] KVM: x86: Register perf callbacks after calling vendors hardware_setup() Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 08/28] KVM: x86: Register Processor Trace interrupt hook iff PT enabled in guest Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 09/28] KVM: x86: dont print when fail to read/write pv eoi memory Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 10/28] KVM: s390: Clarify SIGP orders versus STOP/RESTART Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 11/28] remoteproc: qcom: pas: Add missing power-domain "mxc" for CDSP Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 12/28] 9p: only copy valid iattrs in 9P2000.L setattr implementation Greg Kroah-Hartman
2022-01-18 16:05 ` [PATCH 5.15 13/28] video: vga16fb: Only probe for EGA and VGA 16 color graphic cards Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 14/28] media: uvcvideo: fix division by zero at stream start Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 15/28] rtlwifi: rtl8192cu: Fix WARNING when calling local_irq_restore() with interrupts enabled Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 16/28] firmware: qemu_fw_cfg: fix sysfs information leak Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 17/28] firmware: qemu_fw_cfg: fix NULL-pointer deref on duplicate entries Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 18/28] firmware: qemu_fw_cfg: fix kobject leak in probe error path Greg Kroah-Hartman
2022-01-18 16:06 ` Greg Kroah-Hartman [this message]
2022-01-18 16:06 ` [PATCH 5.15 20/28] KVM: x86: remove PMU FIXED_CTR3 from msrs_to_save_all Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 21/28] ALSA: hda/realtek: Add speaker fixup for some Yoga 15ITL5 devices Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 22/28] ALSA: hda/realtek: Use ALC285_FIXUP_HP_GPIO_LED on another HP laptop Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 23/28] ALSA: hda/realtek - Fix silent output on Gigabyte X570 Aorus Master after reboot from Windows Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 24/28] ALSA: hda: ALC287: Add Lenovo IdeaPad Slim 9i 14ITL5 speaker quirk Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 25/28] ALSA: hda/tegra: Fix Tegra194 HDA reset failure Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 26/28] ALSA: hda/realtek: Add quirk for Legion Y9000X 2020 Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 27/28] ALSA: hda/realtek: Re-order quirk entries for Lenovo Greg Kroah-Hartman
2022-01-18 16:06 ` [PATCH 5.15 28/28] mtd: fixup CFI on ixp4xx Greg Kroah-Hartman
2022-01-18 19:39 ` [PATCH 5.15 00/28] 5.15.16-rc1 review Florian Fainelli
2022-01-18 20:49 ` Jon Hunter
2022-01-18 22:22 ` Shuah Khan
2022-01-19  2:16 ` Ron Economos
2022-01-19 10:21 ` Naresh Kamboju
2022-01-20  1:48 ` Guenter Roeck
2022-01-22  0:29 ` Allen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220118160452.511243269@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dario.pk1@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.