| 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: <aQ1WiC3Id82pTHAI@google.com>
Date: Thu, 6 Nov 2025 18:16:40 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ilya Leoshkevich <iii@...ux.ibm.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
Heiko Carstens <hca@...ux.ibm.com>,
Vasily Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev <agordeev@...ux.ibm.com>
Subject: Re: [PATCH 1/5] perf jitdump: Fix PID namespace detection
On Wed, Nov 05, 2025 at 08:10:24PM +0100, Ilya Leoshkevich wrote:
> perf inject fails to detect jitdump file produced by a process
> running in a different PID namespace if this process has not exited
> yet.
>
> The PID namespace heuristic in jit_detect() compares two PIDs:
>
> * pid: outermost NStgid of mmap(jitdump) caller from perf's PoV.
> * nsinfo__nstgid(nsi): innermost NStgid of mmap(jitdump) caller from
> perf's PoV.
>
> The semantics of the in_pidns variable can be seen in, e.g.,
> nsinfo__get_nspid(): it's true if and only if perf and the profiled
> process are in different PID namespaces.
>
> The current logic is clearly inverted: if pid and nsinfo__nstgid(nsi)
> are different, then the profiled process must be in a different PID
> namespace. This, of course, ignores that fact that they may end up
> being equal by accident, but that's not the point here.
>
> Fix by flipping the comparison.
>
> Changing just that, however, breaks the case when the process has
> exited. Add explicit support for that by adding "synthesized" field to
> nsinfo, which tracks whether NStgid was obtained from a running
> process (ignoring considerations of PID reuse or running inject on
> a different machine). When the namespace information is synthesized,
> assume the process ran in a different PID namespace.
I'm not sure I'm following. It'd be great if anyone understand the
topic well could review.
>
> Signed-off-by: Ilya Leoshkevich <iii@...ux.ibm.com>
> ---
> tools/perf/util/jitdump.c | 27 +++++++++++++++++++++------
> tools/perf/util/namespaces.c | 9 +++++++++
> tools/perf/util/namespaces.h | 2 ++
> 3 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
> index b062b1f234b62..19e4bc139935b 100644
> --- a/tools/perf/util/jitdump.c
> +++ b/tools/perf/util/jitdump.c
> @@ -788,15 +788,30 @@ jit_detect(const char *mmap_name, pid_t pid, struct nsinfo *nsi, bool *in_pidns)
> if (!end)
> return -1;
>
> - *in_pidns = pid == nsinfo__nstgid(nsi);
> /*
> - * pid does not match mmap pid
> - * pid==0 in system-wide mode (synthesized)
> + * Determine whether the process ran inside a container, and whether it
> + * mapped jit.dump for profiling purposes or by accident. Record this
> + * for further use in jit_inject(). The kernel does not provide PID
> + * namespace information, so we have to resort to guesswork.
> *
> - * If the pid in the file name is equal to the nstgid, then
> - * the agent ran inside a container and perf outside the
> - * container, so record it for further use in jit_inject().
> + * If the process exited and perf had to synthesize the namespace
> + * information, then it's not possible to have any certainty; be
> + * aggressive and assume that the process ran inside a container. This
> + * allows the user to proceed with injection at the cost of a small
> + * probability of injecting irrelevant data.
> + *
> + * If the process' NStgid as observed by perf is different from its
> + * innermost NStgid, then it must have run inside a container. There is
> + * a very small probability that NStgids randomly happenned to be the
> + * same; ignore it.
> + *
> + * pid == 0 means system-wide mode, just proceed.
> + *
> + * Finally, the most straightforward case: if the PID in the file name
> + * is equal to the process' NStgid as observed by perf, then it must be
> + * a match.
> */
> + *in_pidns = nsinfo__synthesized(nsi) || pid != nsinfo__nstgid(nsi);
> if (pid && !(pid2 == pid || *in_pidns))
> return -1;
> /*
> diff --git a/tools/perf/util/namespaces.c b/tools/perf/util/namespaces.c
> index 01502570b32d0..7de5d62e271c4 100644
> --- a/tools/perf/util/namespaces.c
> +++ b/tools/perf/util/namespaces.c
> @@ -132,6 +132,8 @@ int nsinfo__init(struct nsinfo *nsi)
>
> rv = nsinfo__get_nspid(&RC_CHK_ACCESS(nsi)->tgid, &RC_CHK_ACCESS(nsi)->nstgid,
> &RC_CHK_ACCESS(nsi)->in_pidns, spath);
> + if (rv == 0)
> + RC_CHK_ACCESS(nsi)->synthesized = false;
>
> out:
> free(newns);
> @@ -166,6 +168,7 @@ struct nsinfo *nsinfo__new(pid_t pid)
> RC_CHK_ACCESS(nsi)->nstgid = pid;
> nsinfo__clear_need_setns(nsi);
> RC_CHK_ACCESS(nsi)->in_pidns = false;
> + RC_CHK_ACCESS(nsi)->synthesized = true;
> /* Init may fail if the process exits while we're trying to look at its
> * proc information. In that case, save the pid but don't try to enter
> * the namespace.
> @@ -197,6 +200,7 @@ struct nsinfo *nsinfo__copy(const struct nsinfo *nsi)
> RC_CHK_ACCESS(nnsi)->nstgid = nsinfo__nstgid(nsi);
> RC_CHK_ACCESS(nnsi)->need_setns = nsinfo__need_setns(nsi);
> RC_CHK_ACCESS(nnsi)->in_pidns = nsinfo__in_pidns(nsi);
> + RC_CHK_ACCESS(nnsi)->synthesized = nsinfo__synthesized(nsi);
> if (nsinfo__mntns_path(nsi)) {
> RC_CHK_ACCESS(nnsi)->mntns_path = strdup(nsinfo__mntns_path(nsi));
> if (!RC_CHK_ACCESS(nnsi)->mntns_path) {
> @@ -275,6 +279,11 @@ void nsinfo__set_in_pidns(struct nsinfo *nsi)
> RC_CHK_ACCESS(nsi)->in_pidns = true;
> }
>
> +bool nsinfo__synthesized(const struct nsinfo *nsi)
> +{
> + return RC_CHK_ACCESS(nsi)->synthesized;
> +}
> +
> void nsinfo__mountns_enter(struct nsinfo *nsi,
> struct nscookie *nc)
> {
> diff --git a/tools/perf/util/namespaces.h b/tools/perf/util/namespaces.h
> index e95c79b80e27c..41ba2ea8137e5 100644
> --- a/tools/perf/util/namespaces.h
> +++ b/tools/perf/util/namespaces.h
> @@ -38,6 +38,7 @@ DECLARE_RC_STRUCT(nsinfo) {
> bool in_pidns;
> char *mntns_path;
> refcount_t refcnt;
> + bool synthesized;
It'd be nice if you can put this along with other bool fields.
Thanks,
Namhyung
> };
>
> struct nscookie {
> @@ -60,6 +61,7 @@ pid_t nsinfo__nstgid(const struct nsinfo *nsi);
> pid_t nsinfo__pid(const struct nsinfo *nsi);
> bool nsinfo__in_pidns(const struct nsinfo *nsi);
> void nsinfo__set_in_pidns(struct nsinfo *nsi);
> +bool nsinfo__synthesized(const struct nsinfo *nsi);
>
> void nsinfo__mountns_enter(struct nsinfo *nsi, struct nscookie *nc);
> void nsinfo__mountns_exit(struct nscookie *nc);
> --
> 2.51.1
>
Powered by blists - more mailing lists