All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jianping Liu" <frankljpliu@gmail.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf annotate: Fix NULL pointer dereference in loongarch_call__parse
Date: Thu, 23 Apr 2026 05:54:37 +0000	[thread overview]
Message-ID: <20260423055438.4599FC2BCB2@smtp.kernel.org> (raw)
In-Reply-To: <20260423053102.3717015-1-frankljpliu@gmail.com>

Sashiko AI review found 1 potential issue(s):
- [Medium] The fix correctly prevents a NULL pointer dereference, but improperly aborts instruction parsing by returning `-1`. This strips the `is_call` attribute and degrades `perf annotate` functionality, whereas it should bypass name extraction and resolve the target using the already extracted address.
--

commit 807b803f1f78132524dc6117283161821fab508a
Author: Jianping Liu <frankljpliu@gmail.com>

perf annotate: Fix NULL pointer dereference in loongarch_call__parse

This commit prevents a segmentation fault in loongarch_call__parse()
by adding a NULL check for the return value of strchr() when parsing
objdump output. It handles cases where a branch-and-link instruction
lacks a symbol name.

> Add a NULL check for the return value of strchr() before dereferencing
> it, consistent with s390_call__parse() which handles the same case
> correctly.

Does s390_call__parse() actually handle this correctly without regressions?

Returning -1 causes disasm_line__init_ins() to receive a negative return
value and set dl->ins.ops = NULL. This makes ins__is_call() evaluate to
false, meaning the instruction completely loses its call semantics in the TUI.

> diff --git a/tools/perf/util/annotate-arch/annotate-loongarch.c b/tools/perf/util/annotate-arch/annotate-loongarch.c
> --- a/tools/perf/util/annotate-arch/annotate-loongarch.c
> +++ b/tools/perf/util/annotate-arch/annotate-loongarch.c
> @@ -29,6 +29,9 @@ static int loongarch_call__parse(const struct arch *arch, struct ins_operands *o
>  	ops->target.addr = strtoull(c, &endptr, 16);
>  
>  	name = strchr(endptr, '<');
> +	if (name == NULL)
> +		return -1;
> +
>  	name++;

Could this code bypass the name extraction and proceed directly to symbol
resolution using the already extracted target address?

Since ops->target.addr is successfully parsed right above this check,
skipping the name parsing and returning 0 instead of aborting with -1 would
preserve the is_call attribute. This would allow the TUI to still draw
branch arrows and navigate the call.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423053102.3717015-1-frankljpliu@gmail.com?part=1

      reply	other threads:[~2026-04-23  5:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-23  5:31 [PATCH] perf annotate: Fix NULL pointer dereference in loongarch_call__parse Jianping Liu
2026-04-23  5:54 ` sashiko-bot [this message]

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=20260423055438.4599FC2BCB2@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=frankljpliu@gmail.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko@lists.linux.dev \
    /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.