[DTrace-devel] [PATCH 1/5] uprobes, libproc: handle uprobes to procs in different fs namespaces
Nick Alcock
nick.alcock at oracle.com
Mon Apr 24 19:08:12 UTC 2023
The specification for a uprobe includes a *mapping name*, which is
documented as the "path to [the] executable or library" where the probe
is placed. Unstated in the documentation is what filesystem namespace
this path is relative to, because -- obviously enough -- it's relative
to the task placing the uprobe.
Unfortunately we get our mapping names, ultimately, from the names in
/proc/$pid/maps, and those names are relative to the fs namespace of the
process being queried. So we cannot just use that name unmodified: the
file might not be there, or indeed anywhere accessible to the
prober. Worse yet, if we're really unlucky some *other* process might be
there and will have a probe stuffed in it at a thoroughly unsuitable
location. One can imagine an attacker setting up a mount namespace with
a /bin/su of their own in it, and putting a DTrace probe in it such that
we stick uprobes in the *real* /bin/su and oh dear. This can probably
only cause a crash, but it still feels very wrong.
Worse yet, when started by systemd dtprobed runs in a non-default
filesystem namespace in which /home is inaccessible: put all this
together and you can't create DTrace probes in binaries you compile
yourself under /home, unless you restart dtprobed yourself outside
systemd's aegis (or hack the service file to turn off ProtectHome).
So fix this by using the map names from /proc/$pid/map_files, which we
already acquire for the purposes of opening the ELF executable in
invasive mode. For that, we can fall back to the old GETMAPFD approach,
but given that that is an out-of-tree patch and was dead, and we're not
even looking for an fd here but rather a filename, and because the
conseqeuences of just using a filename out of /proc/$pid/maps are so
bad, we'll simply not implement a fallback here. USDT requires
map_files built into the kernel and that's all.
This makes uprobe_events less informative (no filenames any more), but
so be it. We can always add a filename as another parameter or something
if we want to.
Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
libcommon/uprobes.c | 21 ++++++++++++++++++---
libproc/Psymtab.c | 27 ++++++++++++++++++++++++++-
libproc/libproc.h | 3 ++-
3 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/libcommon/uprobes.c b/libcommon/uprobes.c
index 8c3cc6280e9a1..396c12e493d10 100644
--- a/libcommon/uprobes.c
+++ b/libcommon/uprobes.c
@@ -27,6 +27,7 @@ uprobe_spec_by_addr(pid_t pid, ps_prochandle *P, uint64_t addr,
int perr = 0;
char *spec = NULL;
const prmap_t *mapp, *first_mapp;
+ char *mapfile_name = NULL;
if (!P) {
P = Pgrab(pid, 2, 0, NULL, &perr);
@@ -41,17 +42,31 @@ uprobe_spec_by_addr(pid_t pid, ps_prochandle *P, uint64_t addr,
first_mapp = mapp->pr_file->first_segment;
+ /*
+ * Use a name in /proc/$pid/map_file: this will work even if the
+ * destination is in a different filesystem namespace. Never use the
+ * absolute path: not only might this not exist, but an *entirely
+ * different file* might be found there in the namespace in which we
+ * are running: prf_mapname is derived from /proc/$pid/maps, and the
+ * names in there are not relative to the namespace of the reader
+ * at all.
+ */
+ mapfile_name = Pmap_mapfile_name(P, mapp);
+ if (!mapfile_name)
+ goto out;
+
/*
* No need for error-checking here: we do the same on error
* and success.
*/
- asprintf(&spec, "%s:0x%lx", mapp->pr_file->prf_mapname,
- addr - first_mapp->pr_vaddr);
+ asprintf(&spec, "%s:0x%lx", mapfile_name, addr - first_mapp->pr_vaddr);
if (mapp_)
memcpy(mapp_, mapp, sizeof(prmap_t));
out:
+ free(mapfile_name);
+
if (free_p) {
/*
* Some things in the prmap aren't valid once the prochandle is
@@ -323,7 +338,7 @@ uprobe_create_from_addr(pid_t pid, uint64_t addr, int is_enabled, const char *pr
}
/*
- * Destroy a uprobe for a given device, address, and spec.
+ * Destroy a uprobe for a given device and address.
*/
int
uprobe_delete(dev_t dev, ino_t ino, uint64_t addr, int isret, int is_enabled)
diff --git a/libproc/Psymtab.c b/libproc/Psymtab.c
index fbfd5fe122cff..cd55b8cf1678a 100644
--- a/libproc/Psymtab.c
+++ b/libproc/Psymtab.c
@@ -1,6 +1,6 @@
/*
* Oracle Linux DTrace.
- * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
@@ -925,6 +925,31 @@ Pinode_to_file_map(struct ps_prochandle *P, dev_t dev, ino_t inum)
return NULL;
}
+
+/*
+ * Return the full path to a given mapping in /proc/$pid/map_files, as a new
+ * dynamically-allocated string, or NULL if none.
+ */
+char *
+Pmap_mapfile_name(struct ps_prochandle *P, const prmap_t *mapp)
+{
+ char *fn;
+
+ if (P->state == PS_DEAD)
+ return NULL;
+
+ Pupdate_maps(P);
+
+ if (mapp->pr_mapaddrname == NULL)
+ return NULL;
+
+ if (asprintf(&fn, "%s/%d/map_files/%s", procfs_path,
+ (int)P->pid, mapp->pr_mapaddrname) < 0)
+ return NULL;
+
+ return fn;
+}
+
/*
* We wouldn't need these if qsort(3C) took an argument for the callback...
*/
diff --git a/libproc/libproc.h b/libproc/libproc.h
index 30d4bd8add5e9..9f434fcaab22c 100644
--- a/libproc/libproc.h
+++ b/libproc/libproc.h
@@ -1,6 +1,6 @@
/*
* Oracle Linux DTrace.
- * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved.
* Licensed under the Universal Permissive License v 1.0 as shown at
* http://oss.oracle.com/licenses/upl.
*/
@@ -245,6 +245,7 @@ extern const prmap_t *Plmid_to_map(struct ps_prochandle *,
Lmid_t, const char *);
extern const prmap_file_t *Pinode_to_file_map(struct ps_prochandle *,
dev_t, ino_t);
+extern char *Pmap_mapfile_name(struct ps_prochandle *P, const prmap_t *mapp);
extern char *Pobjname(struct ps_prochandle *, uintptr_t, char *, size_t);
extern int Plmid(struct ps_prochandle *, uintptr_t, Lmid_t *);
--
2.39.1.268.g9de2f9a303
More information about the DTrace-devel
mailing list