All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Thomas Richter" <tmricht@linux.ibm.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH ping2 linux-next] perf test: probe_vfs_getname add do_getname() probing fallback
Date: Tue, 19 May 2026 11:25:57 +0000	[thread overview]
Message-ID: <20260519112558.5F588C2BCB3@smtp.kernel.org> (raw)
In-Reply-To: <20260519110643.1424411-1-tmricht@linux.ibm.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The fallback perf probe uses `result->iname:string` instead of `result->name:string`, causing out-of-bounds kernel memory reads for long paths.
- [Medium] The `do_getname` probe registration drops the reliable userspace string fallback (`pathname=filename:ustring`).
--

commit eda643d967c95ab86fa0ef1c44c37f4ff13cc45a
Author: Thomas Richter <tmricht@linux.ibm.com>

perf test: probe_vfs_getname add do_getname() probing fallback

The perf vfs_getname test skips on some platforms because getname_flags()
no longer matches the expected perf probe format after kernel changes. This
commit falls back to probing do_getname() if the initial getname_flags()
lookup fails.

> diff --git a/tools/perf/tests/shell/lib/probe_vfs_getname.sh b/tools/perf/tests/shell/lib/probe_vfs_getname.sh
> index 88cd0e26d5f67..9156a79212b9e 100644
> --- a/tools/perf/tests/shell/lib/probe_vfs_getname.sh
> +++ b/tools/perf/tests/shell/lib/probe_vfs_getname.sh

[ ... ]

> @@ -28,12 +30,24 @@ add_probe_vfs_getname() {
>  			line=$(perf probe -L getname_flags 2>&1 | grep -E "$result_aname_re" | sed -r "s/$result_aname_re/\1/")
>  		fi
>  
> +		if [ -z "$line" ] ; then
> +			do_getname=1
> +			result_iname_re="[[:space:]]+([[:digit:]]+)[[:space:]]+initname\(result\);"
> +			line=$(perf probe -L do_getname 2>&1 | grep -E "$result_iname_re" | sed -r "s/$result_iname_re/\1/")
> +		fi
> +
>  		if [ -z "$line" ] ; then
>  			echo "Could not find probeable line"
>  			return 2
>  		fi
>  
> -		perf probe -q       "vfs_getname=getname_flags:${line} pathname=result->name:string" || \
> +		if [ "$do_getname" -eq 1 ]
> +		then
> +			param="vfs_getname=do_getname:${line} pathname=result->iname:string"

Does probing result->iname:string read out-of-bounds kernel memory when paths
are longer than EMBEDDED_NAME_MAX (192 bytes)?

In fs/namei.c, struct filename uses the flexible array iname for short paths,
but dynamically allocates a separate buffer for longer ones. When paths are
long, iname is filled without a null terminator, and result->name points to
the newly allocated buffer.

Reading iname as a string might record garbage data or cause a kprobe fault if
it reads past the end of the slab allocation.

Would it be better to use pathname=result->name:string here, mirroring the
getname_flags behavior?

> +			perf probe -q "$param" || perf probe $add_probe_verbose "$param" || return 1

Does this logic inadvertently drop the userspace string fallback
pathname=filename:ustring?

The original probe logic gracefully falls back to probing the userspace
pointer directly if the compiler optimizes out the result pointer at the
probed line.

Since do_getname() receives const char __user *filename as its first argument
exactly like getname_flags(), could we retain the pathname=filename:ustring
fallback here to improve test reliability across different compiler
environments?

> +			return 0
> +		fi
> +		perf probe -q "vfs_getname=getname_flags:${line} pathname=result->name:string" || \
>  		perf probe $add_probe_verbose "vfs_getname=getname_flags:${line} pathname=filename:ustring" || return 1
>  	fi
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519110643.1424411-1-tmricht@linux.ibm.com?part=1

      reply	other threads:[~2026-05-19 11:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19 11:06 [PATCH ping2 linux-next] perf test: probe_vfs_getname add do_getname() probing fallback Thomas Richter
2026-05-19 11:25 ` 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=20260519112558.5F588C2BCB3@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=tmricht@linux.ibm.com \
    /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.