[DTrace-devel] [PATCH 1/5] uprobes, libproc: handle uprobes to procs in different fs namespaces

Kris Van Hees kris.van.hees at oracle.com
Sat May 13 05:39:39 UTC 2023


On Mon, Apr 24, 2023 at 08:08:12PM +0100, Nick Alcock via DTrace-devel wrote:
> 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.

So the end of this paragraph begs the question: in what circumstances (if
any) is map_files *not* built into the kernel?  Is it is matter of how old
the kernel is?  How old are we talking about?  (Since I do not see any
config option that enables/disables it.)
> 
> 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>

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com

... with comments/corrections as listed

> ---
>  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

map_file -> map_files

> +	 * 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
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel



More information about the DTrace-devel mailing list